Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Enter node before scope for all types in semantic #4276

Closed
overlookmotel opened this issue Jul 15, 2024 · 3 comments · Fixed by #4347
Closed

Enter node before scope for all types in semantic #4276

overlookmotel opened this issue Jul 15, 2024 · 3 comments · Fixed by #4347
Labels
C-bug Category - Bug

Comments

@overlookmotel
Copy link
Collaborator

overlookmotel commented Jul 15, 2024 •
edited
Loading

We recently changed Visit to call enter_node before enter_scope (#4203).

This change is mostly also reflected in SemanticBuilder's Visit method overrides, but a few still have enter_scope before enter_node. These are:

  • visit_for_statement
  • visit_for_in_statement
  • visit_for_of_statement

We should alter them to align with Visit's default behavior, to avoid surprise.
@overlookmotel overlookmotel added the C-bug Category - Bug label Jul 15, 2024
@overlookmotel
Copy link
Collaborator Author

@Dunqing Do you think this would break anything?

@Dunqing
Copy link
Member

Dunqing commented Jul 17, 2024

@Dunqing Do you think this would break anything?

I don't think this will break anything, I just forgot about them. Could you leave it to me? I will finish this after #4253 is merged.

@Dunqing Dunqing mentioned this issue Jul 18, 2024
Merged
@overlookmotel
Copy link
Collaborator Author

Fixed in #4347.

overlookmotel pushed a commit that referenced this issue Jul 18, 2024
@Dunqing
refactor(ast): enter node before scope (#4347)
58f6ec2 
close: #4276
This was referenced Aug 7, 2024
Merged
Merged
Merged
Merged
Merged
overlookmotel pushed a commit that referenced this issue Aug 7, 2024
@rzvxa
feat(span): implement CloneIn for the AST-related items. (#4729)
2e63618 
Follow-on after #4276, related to #4284.
overlookmotel pushed a commit that referenced this issue Aug 7, 2024
@rzvxa
feat(syntax): derive CloneIn for the AST-related items. (#4730)
6a36616 
Follow-on after #4276, related to #4284.
overlookmotel pushed a commit that referenced this issue Aug 7, 2024
@rzvxa
feat(ast_codegen): support for generate_derive marker. (#4728)
2e91ad6 
Follow-on after #4276, related to #4284.
overlookmotel pushed a commit that referenced this issue Aug 7, 2024
@rzvxa
feat(ast_codegen): add derive_clone_in generator. (#4731)
ec0b4cb 
Follow-on after #4276, related to #4284.
overlookmotel pushed a commit that referenced this issue Aug 7, 2024
@rzvxa
feat(ast): derive CloneIn for AST types, using generate_derive. (#…
51c1ca0 
…4732)

Closes #4284 (follow-on after #4276).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-bug Category - Bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

refactor(ast): enter node before scope oxc-project/oxc
2 participants
@overlookmotel @Dunqing