dan/oxc
1
0
Fork 0
mirror of https://github.com/danbulant/oxc synced 2026-05-19 12:19:15 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

refactor(semantic): var hoisting (#4379)

Browse source 
close: #4323

This PR refactors the var hoisting logic to avoid inserting the same symbol into every scope that can be hosted.
This commit is contained in:
Dunqing 2024-07-25 00:55:02 +00:00
parent fd363d1c8b
commit 7cd53f3897
8 changed files with 97 additions and 90 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
crates/oxc_linter/src/rules/eslint/no_import_assign.rs
View file

@ -124,7 +124,7 @@ fn is_argument_of_well_known_mutation_function(node_id: AstNodeId, ctx: &LintCon
if ((ident.name == "Object" && OBJECT_MUTATION_METHODS.contains(property_name))
|| (ident.name == "Reflect" && REFLECT_MUTATION_METHODS.contains(property_name)))
&& !ctx.scopes().has_binding(current_node.scope_id(), &ident.name)
&& ident.reference_id.get().is_some_and(|id| !ctx.symbols().has_binding(id))
{
return expr
.arguments

16
crates/oxc_mangler/src/lib.rs
View file

@ -104,24 +104,8 @@ impl ManglerBuilder {
let mut slot = parent_max_slot;
if !bindings.is_empty() {
let mut parent_bindings = None;
// `bindings` are stored in order, traverse and increment slot
for symbol_id in bindings.values() {
// 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;
}
}
slots[*symbol_id] = slot;
slot += 1;
}

92
crates/oxc_semantic/src/binder.rs
View file

@ -12,14 +12,13 @@ use oxc_span::SourceType;
use crate::{scope::ScopeFlags, symbol::SymbolFlags, SemanticBuilder};
pub trait Binder {
pub trait Binder<'a> {
#[allow(unused_variables)]
fn bind(&self, builder: &mut SemanticBuilder) {}
fn bind(&self, builder: &mut SemanticBuilder<'a>) {}
}
impl<'a> Binder for VariableDeclarator<'a> {
fn bind(&self, builder: &mut SemanticBuilder) {
let current_scope_id = builder.current_scope_id;
impl<'a> Binder<'a> for VariableDeclarator<'a> {
fn bind(&self, builder: &mut SemanticBuilder<'a>) {
let (includes, excludes) = match self.kind {
VariableDeclarationKind::Const => (
SymbolFlags::BlockScopedVariable | SymbolFlags::ConstVariable,
@ -41,48 +40,63 @@ impl<'a> Binder for VariableDeclarator<'a> {
return;
}
// Logic for scope hoisting `var`
// ------------------ var hosting ------------------
let mut target_scope_id = builder.current_scope_id;
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) {
let flag = builder.scope.get_flags(scope_id);
// Skip the catch clause, the scope bindings have been cloned to the child block scope
if flag.is_catch_clause() {
continue;
}
var_scope_ids.push(scope_id);
if flag.is_var() {
break;
}
// Collect all scopes where variable hoisting can occur
for scope_id in builder.scope.ancestors(target_scope_id) {
let flag = builder.scope.get_flags(scope_id);
if flag.is_var() {
target_scope_id = scope_id;
break;
}
var_scope_ids.push(scope_id);
}
self.id.bound_names(&mut |ident| {
let span = ident.span;
let name = &ident.name;
let mut declared_symbol_id = None;
for scope_id in &var_scope_ids {
if let Some(symbol_id) =
builder.check_redeclaration(*scope_id, span, name, excludes, true)
{
ident.symbol_id.set(Some(symbol_id));
builder.add_redeclare_variable(symbol_id, ident.span);
return;
builder.add_redeclare_variable(symbol_id, span);
declared_symbol_id = Some(symbol_id);
let name = name.to_compact_str();
// remove current scope binding and add to target scope
// avoid same symbols appear in multi-scopes
builder.scope.remove_binding(*scope_id, &name);
builder.scope.add_binding(target_scope_id, name, symbol_id);
break;
}
}
let symbol_id =
builder.declare_symbol_on_scope(span, name, current_scope_id, includes, excludes);
// If a variable is already declared in the hoisted scopes,
// we don't need to create another symbol with the same name
// to make sure they point to the same symbol.
let symbol_id = declared_symbol_id.unwrap_or_else(|| {
builder.declare_symbol_on_scope(span, name, target_scope_id, includes, excludes)
});
ident.symbol_id.set(Some(symbol_id));
// Finally, add the variable to all hoisted scopes
// to support redeclaration checks when declaring variables with the same name later.
for scope_id in &var_scope_ids {
builder.scope.add_binding(*scope_id, name.to_compact_str(), symbol_id);
builder
.hoisting_variables
.entry(*scope_id)
.or_default()
.insert(name.clone(), symbol_id);
}
});
}
}
impl<'a> Binder for Class<'a> {
impl<'a> Binder<'a> for Class<'a> {
fn bind(&self, builder: &mut SemanticBuilder) {
if !self.declare {
let Some(ident) = &self.id else { return };
@ -109,7 +123,7 @@ fn function_as_var(flags: ScopeFlags, source_type: SourceType) -> bool {
flags.is_function() || (source_type.is_script() && flags.is_top())
}
impl<'a> Binder for Function<'a> {
impl<'a> Binder<'a> for Function<'a> {
fn bind(&self, builder: &mut SemanticBuilder) {
let current_scope_id = builder.current_scope_id;
let scope_flags = builder.current_scope_flags();
@ -170,7 +184,7 @@ impl<'a> Binder for Function<'a> {
}
}
impl<'a> Binder for BindingRestElement<'a> {
impl<'a> Binder<'a> for BindingRestElement<'a> {
// Binds the FormalParameters's rest of a function or method.
fn bind(&self, builder: &mut SemanticBuilder) {
let parent_kind = builder.nodes.parent_kind(builder.current_node_id).unwrap();
@ -192,7 +206,7 @@ impl<'a> Binder for BindingRestElement<'a> {
}
}
impl<'a> Binder for FormalParameter<'a> {
impl<'a> Binder<'a> for FormalParameter<'a> {
// Binds the FormalParameter of a function or method.
fn bind(&self, builder: &mut SemanticBuilder) {
let parent_kind = builder.nodes.parent_kind(builder.current_node_id).unwrap();
@ -231,7 +245,7 @@ impl<'a> Binder for FormalParameter<'a> {
}
}
impl<'a> Binder for CatchParameter<'a> {
impl<'a> Binder<'a> for CatchParameter<'a> {
fn bind(&self, builder: &mut SemanticBuilder) {
let current_scope_id = builder.current_scope_id;
// https://tc39.es/ecma262/#sec-variablestatements-in-catch-blocks
@ -279,31 +293,31 @@ fn declare_symbol_for_import_specifier(
ident.symbol_id.set(Some(symbol_id));
}
impl<'a> Binder for ImportSpecifier<'a> {
impl<'a> Binder<'a> for ImportSpecifier<'a> {
fn bind(&self, builder: &mut SemanticBuilder) {
declare_symbol_for_import_specifier(&self.local, self.import_kind.is_type(), builder);
}
}
impl<'a> Binder for ImportDefaultSpecifier<'a> {
impl<'a> Binder<'a> for ImportDefaultSpecifier<'a> {
fn bind(&self, builder: &mut SemanticBuilder) {
declare_symbol_for_import_specifier(&self.local, false, builder);
}
}
impl<'a> Binder for ImportNamespaceSpecifier<'a> {
impl<'a> Binder<'a> for ImportNamespaceSpecifier<'a> {
fn bind(&self, builder: &mut SemanticBuilder) {
declare_symbol_for_import_specifier(&self.local, false, builder);
}
}
impl<'a> Binder for TSImportEqualsDeclaration<'a> {
impl<'a> Binder<'a> for TSImportEqualsDeclaration<'a> {
fn bind(&self, builder: &mut SemanticBuilder) {
declare_symbol_for_import_specifier(&self.id, false, builder);
}
}
impl<'a> Binder for TSTypeAliasDeclaration<'a> {
impl<'a> Binder<'a> for TSTypeAliasDeclaration<'a> {
fn bind(&self, builder: &mut SemanticBuilder) {
let symbol_id = builder.declare_symbol(
self.id.span,
@ -315,7 +329,7 @@ impl<'a> Binder for TSTypeAliasDeclaration<'a> {
}
}
impl<'a> Binder for TSInterfaceDeclaration<'a> {
impl<'a> Binder<'a> for TSInterfaceDeclaration<'a> {
fn bind(&self, builder: &mut SemanticBuilder) {
let symbol_id = builder.declare_symbol(
self.id.span,
@ -327,7 +341,7 @@ impl<'a> Binder for TSInterfaceDeclaration<'a> {
}
}
impl<'a> Binder for TSEnumDeclaration<'a> {
impl<'a> Binder<'a> for TSEnumDeclaration<'a> {
fn bind(&self, builder: &mut SemanticBuilder) {
let is_const = self.r#const;
let includes = if is_const { SymbolFlags::ConstEnum } else { SymbolFlags::RegularEnum };
@ -341,7 +355,7 @@ impl<'a> Binder for TSEnumDeclaration<'a> {
}
}
impl<'a> Binder for TSEnumMember<'a> {
impl<'a> Binder<'a> for TSEnumMember<'a> {
fn bind(&self, builder: &mut SemanticBuilder) {
// TODO: Perf
if self.id.is_expression() {
@ -362,7 +376,7 @@ impl<'a> Binder for TSEnumMember<'a> {
}
}
impl<'a> Binder for TSModuleDeclaration<'a> {
impl<'a> Binder<'a> for TSModuleDeclaration<'a> {
fn bind(&self, builder: &mut SemanticBuilder) {
// At declaration time a module has no value declaration it is only when a value declaration
// is made inside a the scope of a module that the symbol is modified
@ -376,7 +390,7 @@ impl<'a> Binder for TSModuleDeclaration<'a> {
}
}
impl<'a> Binder for TSTypeParameter<'a> {
impl<'a> Binder<'a> for TSTypeParameter<'a> {
fn bind(&self, builder: &mut SemanticBuilder) {
let symbol_id = builder.declare_symbol(
self.name.span,

20
crates/oxc_semantic/src/builder.rs
View file

@ -15,6 +15,7 @@ use oxc_cfg::{
use oxc_diagnostics::OxcDiagnostic;
use oxc_span::{Atom, CompactStr, SourceType, Span};
use oxc_syntax::{module_record::ModuleRecord, operator::AssignmentOperator};
use rustc_hash::FxHashMap;
use crate::{
binder::Binder,
@ -27,7 +28,7 @@ use crate::{
module_record::ModuleRecordBuilder,
node::{AstNodeId, AstNodes, NodeFlags},
reference::{Reference, ReferenceFlag, ReferenceId},
scope::{ScopeFlags, ScopeId, ScopeTree},
scope::{Bindings, ScopeFlags, ScopeId, ScopeTree},
symbol::{SymbolFlags, SymbolId, SymbolTable},
unresolved_stack::UnresolvedReferencesStack,
JSDocFinder, Semantic,
@ -66,6 +67,7 @@ pub struct SemanticBuilder<'a> {
// to value like
pub namespace_stack: Vec<SymbolId>,
current_reference_flag: ReferenceFlag,
pub hoisting_variables: FxHashMap<ScopeId, FxHashMap<Atom<'a>, SymbolId>>,
// builders
pub nodes: AstNodes<'a>,
@ -114,6 +116,7 @@ impl<'a> SemanticBuilder<'a> {
function_stack: vec![],
namespace_stack: vec![],
nodes: AstNodes::default(),
hoisting_variables: FxHashMap::default(),
scope,
symbols: SymbolTable::default(),
unresolved_references: UnresolvedReferencesStack::new(),
@ -357,14 +360,16 @@ impl<'a> SemanticBuilder<'a> {
}
pub fn check_redeclaration(
&mut self,
&self,
scope_id: ScopeId,
span: Span,
name: &str,
excludes: SymbolFlags,
report_error: bool,
) -> Option<SymbolId> {
let symbol_id = self.scope.get_binding(scope_id, name)?;
let symbol_id = self.scope.get_binding(scope_id, name).or_else(|| {
self.hoisting_variables.get(&scope_id).and_then(|symbols| symbols.get(name).copied())
})?;
if report_error && self.symbols.get_flag(symbol_id).intersects(excludes) {
let symbol_span = self.symbols.get_span(symbol_id);
self.error(redeclaration(name, symbol_span, span));
@ -496,10 +501,11 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
scope_id.set(Some(self.current_scope_id));
if self.scope.get_flags(parent_scope_id).is_catch_clause() {
// Clone the `CatchClause` bindings and add them to the current scope.
// to make it easier to check redeclare errors.
let bindings = self.scope.get_bindings(parent_scope_id).clone();
self.scope.get_bindings_mut(self.current_scope_id).extend(bindings);
// Move all bindings from parent scope to current scope
// to make it easier to resole references and check redeclare errors.
let parent_bindings =
self.scope.get_bindings_mut(parent_scope_id).drain(..).collect::<Bindings>();
*self.scope.get_bindings_mut(self.current_scope_id) = parent_bindings;
}
self.unresolved_references.increment_scope_depth();

2
crates/oxc_semantic/src/scope.rs
View file

@ -11,7 +11,7 @@ use crate::{symbol::SymbolId, AstNodeId};
type FxIndexMap<K, V> = IndexMap<K, V, BuildHasherDefault<FxHasher>>;
type Bindings = FxIndexMap<CompactStr, SymbolId>;
pub(crate) type Bindings = FxIndexMap<CompactStr, SymbolId>;
pub(crate) type UnresolvedReference = (ReferenceId, ReferenceFlag);
pub type UnresolvedReferences = FxHashMap<CompactStr, Vec<UnresolvedReference>>;

10
crates/oxc_semantic/tests/fixtures/typescript-eslint/catch/inherited-scope.snap vendored
View file

@ -33,15 +33,7 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/catch/inherited
"flag": "ScopeFlags(StrictMode | CatchClause)",
"id": 2,
"node": "CatchClause",
"symbols": [
{
"flag": "SymbolFlags(FunctionScopedVariable | CatchVariable)",
"id": 1,
"name": "e",
"node": "CatchParameter",
"references": []
}
]
"symbols": []
}
],
"flag": "ScopeFlags(StrictMode | Top)",

17
crates/oxc_semantic/tests/fixtures/typescript-eslint/catch/scope.snap vendored
View file

@ -47,22 +47,7 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/catch/scope.ts
"flag": "ScopeFlags(StrictMode | CatchClause)",
"id": 2,
"node": "CatchClause",
"symbols": [
{
"flag": "SymbolFlags(FunctionScopedVariable | CatchVariable)",
"id": 0,
"name": "e",
"node": "CatchParameter",
"references": [
{
"flag": "ReferenceFlag(Read)",
"id": 0,
"name": "e",
"node_id": 8
}
]
}
]
"symbols": []
}
],
"flag": "ScopeFlags(StrictMode | Top)",

28
tasks/coverage/parser_typescript.snap
View file

@ -2,7 +2,7 @@ commit: d8086f14
parser_typescript Summary:
AST Parsed : 6444/6456 (99.81%)
Positive Passed: 6422/6456 (99.47%)
Positive Passed: 6421/6456 (99.46%)
Negative Passed: 1160/5653 (20.52%)
Expect Syntax Error: "compiler/ClassDeclaration10.ts"
Expect Syntax Error: "compiler/ClassDeclaration11.ts"
@ -4639,6 +4639,19 @@ Expect to Parse: "compiler/withStatementInternalComments.ts"
2 │ /*1*/ with /*2*/ ( /*3*/ false /*4*/ ) /*5*/ {}
· ────
╰────
Expect to Parse: "conformance/async/es6/asyncWithVarShadowing_es6.ts"
× Identifier `x` has already been declared
╭─[conformance/async/es6/asyncWithVarShadowing_es6.ts:130:14]
129 │ }
130 │ catch ({ x }) {
· ┬
· ╰── `x` has already been declared here
131 │ var x;
· ┬
· ╰── It can not be redeclared here
132 │ }
╰────
Expect to Parse: "conformance/classes/propertyMemberDeclarations/staticPropertyNameConflicts.ts"
× Classes may not have a static property named prototype
@ -6354,6 +6367,19 @@ Expect to Parse: "conformance/salsa/typeFromPropertyAssignmentWithExport.ts"
· ──
╰────
× Identifier `x` has already been declared
╭─[compiler/constDeclarationShadowedByVarDeclaration.ts:4:11]
3 │ {
4 │ const x = 0;
· ┬
· ╰── `x` has already been declared here
5 │
6 │ var x = 0;
· ┬
· ╰── It can not be redeclared here
7 │ }
╰────
× Identifier `y` has already been declared
╭─[compiler/constDeclarationShadowedByVarDeclaration.ts:12:11]
11 │ {