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.

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

semantic: var hoisting #4323

Closed
Boshen opened this issue Jul 17, 2024 · 0 comments · Fixed by #4379
Closed

semantic: var hoisting #4323

Boshen opened this issue Jul 17, 2024 · 0 comments · Fixed by #4379
Assignees
Labels
C-bug Category - Bug

Comments

@Boshen
Copy link
Member

Boshen commented Jul 17, 2024

Several attempts was made to pass all tests in test262 + babel + typescript regarding var hoisting in strict / non-strict mode.

Scope tree ended up with a var binding everywhere:

{
  {
    var x
  } 
}

x = 1

produces

Scope1 (ScopeFlags(StrictMode | Top)) {
  Bindings: {
    x (SymbolFlags(FunctionScopedVariable))
  }
  Scope2 (ScopeFlags(StrictMode)) {
    Bindings: {
      x (SymbolFlags(FunctionScopedVariable))
    }
    Scope3 (ScopeFlags(StrictMode)) {
      Bindings: {
        x (SymbolFlags(FunctionScopedVariable))
      }
    }
  }
}

The consequence of this is:

  • unintuitive
  • performance killer
  • breaks mangler
    // omit var hoisting because var symbols are added to every parent scope
    if symbol_table.get_flag(*symbol_id).is_function_scoped_declaration()
    && parent_bindings.is_none()
    {
    parent_bindings = scope_tree
    .get_parent_id(scope_id)
    .map(|parent_scope_id| scope_tree.get_bindings(parent_scope_id));
    }
    if let Some(parent_bindings) = &parent_bindings {
    if parent_bindings.values().contains(symbol_id) {
    continue;
    }
    }

To understand the test cases, try different strategies in

let mut var_scope_ids = vec![];
if !builder.current_scope_flags().is_var() {
for scope_id in builder.scope.ancestors(current_scope_id).skip(1) {
var_scope_ids.push(scope_id);
if builder.scope.get_flags(scope_id).is_var() {
break;
}
}
}

and run just c to see changes in snapshot.

@Boshen Boshen added the C-bug Category - Bug label Jul 17, 2024
Dunqing added a commit that referenced this issue Jul 18, 2024
Dunqing added a commit that referenced this issue Jul 25, 2024
close: #4323

This PR refactors the var hoisting logic to avoid inserting the same symbol into every scope that can be hosted.
@Dunqing Dunqing closed this as completed Jul 25, 2024
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 a pull request may close this issue.

2 participants