feat(semantic): add some scopes to align TypeScript's container #3969
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Your org has enabled the Graphite merge queue for merging into mainAdd the label “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. |
github-actions
bot
added
A-parser
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
CodSpeed Performance ReportMerging #3969 will not alter performanceComparing Summary
|
Dunqing
force-pushed
the
06-29-feat_semantic_add_some_scopes_to_align_typescript_s_container
branch
3 times, most recently
from
c673495 to
44587fa
Compare
Dunqing
commented
Jun 29, 2024
Dunqing
force-pushed
the
06-29-feat_semantic_add_some_scopes_to_align_typescript_s_container
branch
from
44587fa to
cbd672e
Compare
DonIsaac
reviewed
Jun 29, 2024
| @@ -237,6 +238,7 @@ pub struct TSConditionalType<'a> { | |||
| pub extends_type: TSType<'a>, | |||
| pub true_type: TSType<'a>, | |||
| pub false_type: TSType<'a>, | |||
| pub scope_id: Cell<Option<ScopeId>>, | |||
There was a problem hiding this comment.
Would Option<Cell<ScopeId>> be better or worse?
There was a problem hiding this comment.
It has to be Cell<Option<ScopeId>> because the Option value needs to be mutated from None to Some(...) with only an immutable &TSConditionalType (in Semantic).
Dunqing
force-pushed
the
06-29-feat_semantic_add_some_scopes_to_align_typescript_s_container
branch
from
cbd672e to
86b4b58
Compare
Dunqing
force-pushed
the
06-29-feat_semantic_add_some_scopes_to_align_typescript_s_container
branch
2 times, most recently
from
b6ef7ca to
53efc22
Compare
Dunqing
changed the base branch from
main
to
07-10-refactor_semantic_add_strict_mode_in_scope_flags_for_class_definitions
Dunqing
force-pushed
the
07-10-refactor_semantic_add_strict_mode_in_scope_flags_for_class_definitions
branch
from
df02c40 to
f676841
Compare
Dunqing
force-pushed
the
06-29-feat_semantic_add_some_scopes_to_align_typescript_s_container
branch
from
53efc22 to
c46b60d
Compare
Dunqing
force-pushed
the
07-10-refactor_semantic_add_strict_mode_in_scope_flags_for_class_definitions
branch
2 times, most recently
from
e88e957 to
e9192fb
Compare
This was referenced Jul 11, 2024
Dunqing
force-pushed
the
06-29-feat_semantic_add_some_scopes_to_align_typescript_s_container
branch
from
c46b60d to
1244aab
Compare
Dunqing
force-pushed
the
07-10-refactor_semantic_add_strict_mode_in_scope_flags_for_class_definitions
branch
from
e9192fb to
2f41660
Compare
Dunqing
force-pushed
the
06-29-feat_semantic_add_some_scopes_to_align_typescript_s_container
branch
from
1244aab to
5c75f05
Compare
Dunqing
force-pushed
the
07-10-refactor_semantic_add_strict_mode_in_scope_flags_for_class_definitions
branch
from
2f41660 to
f171c7e
Compare
Boshen
force-pushed
the
07-10-refactor_semantic_add_strict_mode_in_scope_flags_for_class_definitions
branch
from
f171c7e to
dc2b3c4
Compare
Boshen
changed the base branch from
07-10-refactor_semantic_add_strict_mode_in_scope_flags_for_class_definitions
to
main
|
Superseded by #4280 |
Boshen
pushed a commit
that referenced
this pull request
Jul 17, 2024
close: #3969 close: #2023 We need to add scope according to [this](https://github.com/microsoft/TypeScript/blob/d8086f14b6b97c0df34a0cc2f56d4b5926a0c299/src/compiler/binder.ts#L3883-L3962). There are still some ASTs that need to be added to the scope. --- Context from @Boshen: Before this whole journey of fixing symbols and scopes I asked @Dunqing to debug through binder.ts via a debugger to fully understand how it does resolution. We then agreed to align the implementation so that when a problem occurs, we can debug through both implementations and find where our problem is. tsc doesn't have a specification, so we need to align with the reference implementation instead.
Boshen
deleted the
06-29-feat_semantic_add_some_scopes_to_align_typescript_s_container
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pr is a prototype to fix #2023. We have added scope to some nodes