From 3ce41169303eceb47b4a5147e8fceeea23a145e8 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Thu, 17 Aug 2023 10:03:00 -0700 Subject: [PATCH] [rust][sema] Only create one scope per function/class Fixes up function and class handling to not create nested scopes in some cases. Doesn't affect name resolution otherwise. --- .../forget_semantic_analysis/src/analyzer.rs | 125 +++++++++------ ...s_test__fixtures@function-hoisting.js.snap | 12 +- ...sis_test__fixtures@simple-function.js.snap | 151 ++++++++---------- 3 files changed, 143 insertions(+), 145 deletions(-) diff --git a/compiler/forget/crates/forget_semantic_analysis/src/analyzer.rs b/compiler/forget/crates/forget_semantic_analysis/src/analyzer.rs index 75d8bf4204..c587a290f6 100644 --- a/compiler/forget/crates/forget_semantic_analysis/src/analyzer.rs +++ b/compiler/forget/crates/forget_semantic_analysis/src/analyzer.rs @@ -149,53 +149,49 @@ impl Analyzer { } fn visit_function(&mut self, node: &T) { + assert_eq!(self.manager.scope(self.current).kind, ScopeKind::Function); let function = node.function(); - let scope = self.enter(ScopeKind::Function, |visitor| { - for param in &function.params { - // `this` parameters don't declare variables, nor can they have - // default values - if let Pattern::Identifier(param) = param { - if ¶m.name == "this" { - continue; - } + for param in &function.params { + // `this` parameters don't declare variables, nor can they have + // default values + if let Pattern::Identifier(param) = param { + if ¶m.name == "this" { + continue; } - Analyzer::visit_declaration_pattern( - visitor, - param, - Some(DeclarationKind::Function), - ); } + Analyzer::visit_declaration_pattern(self, param, Some(DeclarationKind::Function)); + } - if let Some(body) = &function.body { - match body { - FunctionBody::BlockStatement(body) => { - // Skip calling visit_block_statement to avoid creating an extra - // block scope - for item in &body.body { - visitor.visit_statement(item); - } - } - FunctionBody::Expression(body) => { - visitor.visit_expression(body); + if let Some(body) = &function.body { + match body { + FunctionBody::BlockStatement(body) => { + // Skip calling visit_block_statement to avoid creating an extra + // block scope + for item in &body.body { + self.visit_statement(item); } } + FunctionBody::Expression(body) => { + self.visit_expression(body); + } } - }); + } self.manager .node_scopes - .insert(AstNode::from(function), scope); + .insert(AstNode::from(function), self.current); } fn visit_class(&mut self, node: &T) { + assert_eq!(self.manager.scope(self.current).kind, ScopeKind::Class); let class = node.class(); - let scope = self.enter(ScopeKind::Class, |visitor| { - if let Some(super_class) = &class.super_class { - visitor.visit_expression(super_class); - } + if let Some(super_class) = &class.super_class { + self.visit_expression(super_class); + } - visitor.visit_class_body(&class.body); - }); - self.manager.node_scopes.insert(AstNode::from(class), scope); + self.visit_class_body(&class.body); + self.manager + .node_scopes + .insert(AstNode::from(class), self.current); } fn visit_reference_identifier( @@ -361,7 +357,28 @@ impl Visitor for Analyzer { .node_declarations .insert(AstNode::from(id), declaration); } - Analyzer::visit_class(self, ast); + self.enter(ScopeKind::Class, |visitor| { + Analyzer::visit_class(visitor, ast); + }); + } + + fn visit_class_expression(&mut self, ast: &forget_estree::ClassExpression) { + self.enter(ScopeKind::Class, |visitor| { + if let Some(id) = &ast.class.id { + let declaration = visitor.manager.add_declaration( + visitor.current, + id.name.clone(), + DeclarationKind::Function, + id.range, + ); + visitor + .manager + .node_declarations + .insert(AstNode::from(id), declaration); + } + + Analyzer::visit_class(visitor, ast); + }); } fn visit_class_property(&mut self, ast: &forget_estree::ClassProperty) { @@ -419,32 +436,34 @@ impl Visitor for Analyzer { .node_declarations .insert(AstNode::from(id), declaration); } - Analyzer::visit_function(self, ast); + self.enter(ScopeKind::Function, |visitor| { + Analyzer::visit_function(visitor, ast); + }); } fn visit_function_expression(&mut self, ast: &forget_estree::FunctionExpression) { - let mut function_scope: Option = None; - if let Some(id) = &ast.function.id { - function_scope = Some(self.enter_scope(ScopeKind::Function)); - let declaration = self.manager.add_declaration( - self.current, - id.name.clone(), - DeclarationKind::Function, - id.range, - ); - self.manager - .node_declarations - .insert(AstNode::from(id), declaration); - } + self.enter(ScopeKind::Function, |visitor| { + if let Some(id) = &ast.function.id { + let declaration = visitor.manager.add_declaration( + visitor.current, + id.name.clone(), + DeclarationKind::Function, + id.range, + ); + visitor + .manager + .node_declarations + .insert(AstNode::from(id), declaration); + } - Analyzer::visit_function(self, ast); - if let Some(function_scope) = function_scope { - self.close_scope(function_scope); - } + Analyzer::visit_function(visitor, ast); + }); } fn visit_arrow_function_expression(&mut self, ast: &forget_estree::ArrowFunctionExpression) { - Analyzer::visit_function(self, ast); + self.enter(ScopeKind::Function, |visitor| { + Analyzer::visit_function(visitor, ast); + }); } fn visit_assignment_expression(&mut self, ast: &forget_estree::AssignmentExpression) { diff --git a/compiler/forget/crates/forget_semantic_analysis/tests/snapshots/analysis_test__fixtures@function-hoisting.js.snap b/compiler/forget/crates/forget_semantic_analysis/tests/snapshots/analysis_test__fixtures@function-hoisting.js.snap index 36b05b2b08..08af0368e2 100644 --- a/compiler/forget/crates/forget_semantic_analysis/tests/snapshots/analysis_test__fixtures@function-hoisting.js.snap +++ b/compiler/forget/crates/forget_semantic_analysis/tests/snapshots/analysis_test__fixtures@function-hoisting.js.snap @@ -255,17 +255,7 @@ ScopeManager { }, }, references: [], - children: [ - Scope { - id: ScopeId( - 10, - ), - kind: Function, - declarations: {}, - references: [], - children: [], - }, - ], + children: [], }, ], }, diff --git a/compiler/forget/crates/forget_semantic_analysis/tests/snapshots/analysis_test__fixtures@simple-function.js.snap b/compiler/forget/crates/forget_semantic_analysis/tests/snapshots/analysis_test__fixtures@simple-function.js.snap index ffefd1f4e6..8e19dd86f3 100644 --- a/compiler/forget/crates/forget_semantic_analysis/tests/snapshots/analysis_test__fixtures@simple-function.js.snap +++ b/compiler/forget/crates/forget_semantic_analysis/tests/snapshots/analysis_test__fixtures@simple-function.js.snap @@ -169,91 +169,80 @@ ScopeManager { 2, ), }, - }, - references: [], - children: [ - Scope { - id: ScopeId( - 3, + "c": Declaration { + id: DeclarationId( + 14, ), kind: Function, - declarations: { - "c": Declaration { - id: DeclarationId( - 14, - ), - kind: Function, - scope: ScopeId( - 3, - ), - }, - "d": Declaration { - id: DeclarationId( - 15, - ), - kind: Let, - scope: ScopeId( - 3, - ), - }, - }, - references: [ - Reference { - id: ReferenceId( - 1, - ), - kind: Read, - declaration: DeclarationId( - 10, - ), - declaration (name): "a", - scope: ScopeId( - 3, - ), - }, - Reference { - id: ReferenceId( - 2, - ), - kind: Read, - declaration: DeclarationId( - 11, - ), - declaration (name): "b", - scope: ScopeId( - 3, - ), - }, - Reference { - id: ReferenceId( - 3, - ), - kind: Read, - declaration: DeclarationId( - 14, - ), - declaration (name): "c", - scope: ScopeId( - 3, - ), - }, - Reference { - id: ReferenceId( - 4, - ), - kind: Read, - declaration: DeclarationId( - 15, - ), - declaration (name): "d", - scope: ScopeId( - 3, - ), - }, - ], - children: [], + scope: ScopeId( + 2, + ), + }, + "d": Declaration { + id: DeclarationId( + 15, + ), + kind: Let, + scope: ScopeId( + 2, + ), + }, + }, + references: [ + Reference { + id: ReferenceId( + 1, + ), + kind: Read, + declaration: DeclarationId( + 10, + ), + declaration (name): "a", + scope: ScopeId( + 2, + ), + }, + Reference { + id: ReferenceId( + 2, + ), + kind: Read, + declaration: DeclarationId( + 11, + ), + declaration (name): "b", + scope: ScopeId( + 2, + ), + }, + Reference { + id: ReferenceId( + 3, + ), + kind: Read, + declaration: DeclarationId( + 14, + ), + declaration (name): "c", + scope: ScopeId( + 2, + ), + }, + Reference { + id: ReferenceId( + 4, + ), + kind: Read, + declaration: DeclarationId( + 15, + ), + declaration (name): "d", + scope: ScopeId( + 2, + ), }, ], + children: [], }, ], },