Skip to content

Conversation

@ShuiRuTian
Copy link
Contributor

Function should be Value

@graphite-app
Copy link
Contributor

graphite-app bot commented Nov 21, 2024

Your org has enabled the Graphite merge queue for merging into main

Add the label “0-merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

@ShuiRuTian
Copy link
Contributor Author

And a stupid question, just out of curious...

The implementation seems to refer typescript's design. Then why it is missed?

@codspeed-hq
Copy link

codspeed-hq bot commented Nov 21, 2024

CodSpeed Performance Report

Merging #7402 will not alter performance

Comparing ShuiRuTian:function-is-value (266b1bb) with main (0918e52)

Summary

✅ 30 untouched benchmarks

@overlookmotel overlookmotel changed the title Fix(binder): function is value fix(semantic): function is value Nov 21, 2024
@github-actions github-actions bot added the C-bug Category - Bug label Nov 21, 2024
@Boshen
Copy link
Member

Boshen commented Nov 23, 2024

This breaks many tests 🤔

@Boshen Boshen marked this pull request as draft November 23, 2024 15:26
@ShuiRuTian
Copy link
Contributor Author

This breaks many tests 🤔

Yeah, strange, investigating...

@Dunqing
Copy link
Member

Dunqing commented Nov 26, 2024

The SymbolFlags is port from TypeScript and did some adjustments to support non-strict checks. I have changed this in #7479 as well

@Boshen
Copy link
Member

Boshen commented Nov 26, 2024

I'll follow this up in #7479

@Boshen Boshen closed this Nov 26, 2024
graphite-app bot pushed a commit that referenced this pull request Mar 24, 2025
#7479)

close #7402
related: #7470

The purpose of this PR is to keep the SymbolFlags of the function consistent.

Always use `SymbolFlags::Function` for the function id symbol, to avoid adding extra logic to find accurate `SymbolFlags` for the function in `transformer`. This change adds a fallback redeclaration check for case `async function foo() {}; var foo;` in `SemanticBuilder::check_redeclaraion`.

No performance difference here because redeclaration is always rare.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-bug Category - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants