From 011570a0785d4d2f0fd23dd5eae37ac797ce882b Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Mon, 14 Aug 2023 10:08:02 -0700 Subject: [PATCH] [rust][sema] Bubble unresolved references (for hoisting) This is a precursor to adding support for hoisting in semantic analysis. Previously when we encountered an unknown reference we immediately reported an error. But hoisted variables may be referenced before they're defined, so we don't know for sure when we see an unknown variable if its actually unbound or not. This PR adds the first part of hoistingn support: rather than immediately report an error when encountering an unbound variable we store it in a list of unresolved references on the current scope. As we close each scope we recheck and see if the variable can now be resolved. If yes we record that, otherwise we bubble up the unresolved reference to the parent scope (and try again there). The next PR(s) will handle hoisting of `var` and other syntax to the apropriate nearest scope boundary (function/module). --- .../forget_semantic_analysis/src/analyzer.rs | 59 +++++++-- .../src/scope_manager.rs | 44 ++++++- .../tests/fixtures/let-const-hoisting.js | 8 ++ ..._test__fixtures@let-const-hoisting.js.snap | 122 ++++++++++++++++++ 4 files changed, 221 insertions(+), 12 deletions(-) create mode 100644 compiler/forget/crates/forget_semantic_analysis/tests/fixtures/let-const-hoisting.js create mode 100644 compiler/forget/crates/forget_semantic_analysis/tests/snapshots/analysis_test__fixtures@let-const-hoisting.js.snap diff --git a/compiler/forget/crates/forget_semantic_analysis/src/analyzer.rs b/compiler/forget/crates/forget_semantic_analysis/src/analyzer.rs index c167c85d41..cb8129bd0d 100644 --- a/compiler/forget/crates/forget_semantic_analysis/src/analyzer.rs +++ b/compiler/forget/crates/forget_semantic_analysis/src/analyzer.rs @@ -86,10 +86,9 @@ impl Analyzer { where F: FnMut(&mut Self) -> (), { - let scope = self.manager.add_scope(self.current, kind); - let previous = std::mem::replace(&mut self.current, scope); + let scope = self.enter_scope(kind); f(self); - let scope = std::mem::replace(&mut self.current, previous); + self.close_scope(scope); scope } @@ -101,8 +100,26 @@ impl Analyzer { fn close_scope(&mut self, id: ScopeId) { assert_eq!(self.current, id, "Mismatched enter_scope/close_scope"); - let scope = self.manager.scope(self.current); - self.current = scope.parent.unwrap(); + let scope = self.manager.mut_scope(self.current); + let parent = scope.parent.unwrap(); + let unresolved = std::mem::take(&mut scope.unresolved); + drop(scope); + self.current = parent; + + // Lookup unresolved nodes from the child scope in the (now-current) parent scope + for reference in unresolved { + if let Some(declaration) = self + .manager + .lookup_declaration(reference.scope, &reference.name) + { + let id = + self.manager + .add_reference(reference.scope, reference.kind, declaration.id); + self.manager.node_references.insert(reference.ast, id); + } else { + self.manager.push_unresolved_reference(parent, reference); + } + } } fn visit_function(&mut self, node: &T) { @@ -157,10 +174,8 @@ impl Analyzer { .add_reference(self.current, kind, declaration.id); self.manager.node_references.insert(ast, id); } else { - // Oops, undefined variable self.manager - .diagnostics - .push(Diagnostic::invalid_syntax("Undefined variable", range)); + .add_unresolved_reference(self.current, ast, name.to_string(), kind, range); } } @@ -201,9 +216,13 @@ impl Analyzer { .node_references .insert(AstNode::from(ast), reference); } else { - self.manager - .diagnostics - .push(Diagnostic::invalid_syntax("Undefined variable", ast.range)); + self.manager.add_unresolved_reference( + self.current, + AstNode::from(ast), + ast.name.clone(), + ReferenceKind::ReadWrite, + ast.range, + ); } } } @@ -616,6 +635,24 @@ impl Visitor for Analyzer { self.visit_module_item(item); } } + let scope = self.manager.mut_scope(self.current); + let unresolved = std::mem::take(&mut scope.unresolved); + for reference in unresolved { + if let Some(declaration) = self + .manager + .lookup_declaration(reference.scope, &reference.name) + { + let id = + self.manager + .add_reference(reference.scope, reference.kind, declaration.id); + self.manager.node_references.insert(reference.ast, id); + } else { + self.manager.diagnostics.push(Diagnostic::invalid_syntax( + "Undefined variable", + reference.range, + )); + } + } } fn visit_property(&mut self, ast: &forget_estree::Property) { diff --git a/compiler/forget/crates/forget_semantic_analysis/src/scope_manager.rs b/compiler/forget/crates/forget_semantic_analysis/src/scope_manager.rs index d52ce3d850..e293059f09 100644 --- a/compiler/forget/crates/forget_semantic_analysis/src/scope_manager.rs +++ b/compiler/forget/crates/forget_semantic_analysis/src/scope_manager.rs @@ -1,6 +1,7 @@ use forget_diagnostics::Diagnostic; use forget_estree::{ - BreakStatement, ContinueStatement, ESTreeNode, LabeledStatement, VariableDeclarationKind, + BreakStatement, ContinueStatement, ESTreeNode, LabeledStatement, SourceRange, + VariableDeclarationKind, }; use forget_utils::PointerAddress; use indexmap::IndexMap; @@ -43,6 +44,7 @@ impl ScopeManager { declarations: Default::default(), references: Default::default(), children: Default::default(), + unresolved: Default::default(), }], labels: Default::default(), declarations: Default::default(), @@ -75,6 +77,10 @@ impl ScopeManager { &self.scopes[id.0] } + pub fn mut_scope(&mut self, id: ScopeId) -> &mut Scope { + &mut self.scopes[id.0] + } + pub fn is_descendant_of(&self, maybe_descendant: ScopeId, maybe_ancestor: ScopeId) -> bool { let mut current = maybe_descendant; loop { @@ -192,6 +198,7 @@ impl ScopeManager { declarations: Default::default(), references: Default::default(), children: Default::default(), + unresolved: Default::default(), }); self.scopes[parent.0].children.push(id); id @@ -252,6 +259,31 @@ impl ScopeManager { self.scopes[scope.0].references.push(id); id } + + pub(crate) fn push_unresolved_reference( + &mut self, + scope: ScopeId, + reference: UnresolvedReference, + ) { + self.scopes[scope.0].unresolved.push(reference); + } + + pub(crate) fn add_unresolved_reference( + &mut self, + scope: ScopeId, + ast: AstNode, + name: String, + kind: ReferenceKind, + range: Option, + ) { + self.scopes[scope.0].unresolved.push(UnresolvedReference { + ast, + scope, + name, + kind, + range, + }); + } } #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Copy, Clone)] @@ -286,6 +318,16 @@ pub struct Scope { pub declarations: IndexMap, pub references: Vec, pub children: Vec, + pub unresolved: Vec, +} + +#[derive(Debug, Clone)] +pub struct UnresolvedReference { + pub scope: ScopeId, + pub ast: AstNode, + pub name: String, + pub kind: ReferenceKind, + pub range: Option, } #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Copy, Clone)] diff --git a/compiler/forget/crates/forget_semantic_analysis/tests/fixtures/let-const-hoisting.js b/compiler/forget/crates/forget_semantic_analysis/tests/fixtures/let-const-hoisting.js new file mode 100644 index 0000000000..8f9c7e6b40 --- /dev/null +++ b/compiler/forget/crates/forget_semantic_analysis/tests/fixtures/let-const-hoisting.js @@ -0,0 +1,8 @@ +function Component() { + function foo() { + a; + b; + } + let a; + const b = true; +} diff --git a/compiler/forget/crates/forget_semantic_analysis/tests/snapshots/analysis_test__fixtures@let-const-hoisting.js.snap b/compiler/forget/crates/forget_semantic_analysis/tests/snapshots/analysis_test__fixtures@let-const-hoisting.js.snap new file mode 100644 index 0000000000..f96d37a004 --- /dev/null +++ b/compiler/forget/crates/forget_semantic_analysis/tests/snapshots/analysis_test__fixtures@let-const-hoisting.js.snap @@ -0,0 +1,122 @@ +--- +source: crates/forget_semantic_analysis/tests/analysis_test.rs +expression: "format!(\"Input:\\n{input}\\n\\nAnalysis:\\n{output}\")" +input_file: crates/forget_semantic_analysis/tests/fixtures/let-const-hoisting.js +--- +Input: +function Component() { + function foo() { + a; + b; + } + let a; + const b = true; +} + + +Analysis: +Scope { + id: ScopeId( + 0, + ), + kind: Global, + declarations: {}, + references: [], + children: [ + Scope { + id: ScopeId( + 1, + ), + kind: Module, + declarations: { + "Component": Declaration { + id: DeclarationId( + 0, + ), + kind: FunctionDeclaration, + scope: ScopeId( + 1, + ), + }, + }, + references: [], + children: [ + Scope { + id: ScopeId( + 2, + ), + kind: Function, + declarations: { + "foo": Declaration { + id: DeclarationId( + 1, + ), + kind: FunctionDeclaration, + scope: ScopeId( + 2, + ), + }, + "a": Declaration { + id: DeclarationId( + 2, + ), + kind: Let, + scope: ScopeId( + 2, + ), + }, + "b": Declaration { + id: DeclarationId( + 3, + ), + kind: Const, + scope: ScopeId( + 2, + ), + }, + }, + references: [], + children: [ + Scope { + id: ScopeId( + 3, + ), + kind: Function, + declarations: {}, + references: [ + Reference { + id: ReferenceId( + 0, + ), + kind: Read, + declaration: DeclarationId( + 2, + ), + declaration (name): "a", + scope: ScopeId( + 3, + ), + }, + Reference { + id: ReferenceId( + 1, + ), + kind: Read, + declaration: DeclarationId( + 3, + ), + declaration (name): "b", + scope: ScopeId( + 3, + ), + }, + ], + children: [], + }, + ], + }, + ], + }, + ], +} +