@samitouri / QOS-React-1 / commits / 2edf66ecdf

[rust][sema] Resolve all references after traversing program

Previously we attempted to resolve each reference at the close of its defining scope, and if it couldn't be resolved yet we bubbled the unresolved reference up to the parent scope. That approach isn't ideal for two reasons: * First, it's inefficient since we may have to make multiple attempts to resolve the same reference. * Second, it's incorrect. There can be cases where we think we can resolve a reference to a value defined in an outer scope, but there is a hoisted declaration from an intermediate scope that we haven't seen yet. The safest and most optimal thing is to just queue all references and resolve them at the end.

Joe Savona committed Aug 15, 2023 at 12:33 UTC 2edf66ecdfa31bbae9384b0f8993a0fbe6e0b025
5 files changed +174 -242
compiler/forget/crates/forget_semantic_analysis/src/analyzer.rs
+46 -51
@@ -14,13 +14,23 @@ use crate::{
14 pub fn analyze(ast: &Program) -> ScopeManager {
15 let mut analyzer = Analyzer::new(ast);
16 analyzer.visit_program(ast);
17 - analyzer.manager
17 + analyzer.complete()
18 }
19
20 struct Analyzer {
21 manager: ScopeManager,
22 labels: Vec<LabelId>,
23 current: ScopeId,
24 + unresolved: Vec<UnresolvedReference>,
25 +}
26 +
27 +#[derive(Debug, Clone)]
28 +pub struct UnresolvedReference {
29 + pub scope: ScopeId,
30 + pub ast: AstNode,
31 + pub name: String,
32 + pub kind: ReferenceKind,
33 + pub range: Option<SourceRange>,
34 }
35
36 impl Analyzer {
@@ -32,9 +42,30 @@ impl Analyzer {
42 manager,
43 labels,
44 current,
45 + unresolved: Default::default(),
46 }
47 }
48
49 + fn complete(mut self) -> ScopeManager {
50 + for reference in self.unresolved {
51 + if let Some(declaration) = self
52 + .manager
53 + .lookup_declaration(reference.scope, &reference.name)
54 + {
55 + let id =
56 + self.manager
57 + .add_reference(reference.scope, reference.kind, declaration.id);
58 + self.manager.node_references.insert(reference.ast, id);
59 + } else {
60 + self.manager.diagnostics.push(Diagnostic::invalid_syntax(
61 + "Undefined variable",
62 + reference.range,
63 + ));
64 + }
65 + }
66 + self.manager
67 + }
68 +
69 fn enter_label<F>(&mut self, id: LabelId, mut f: F)
70 where
71 F: FnMut(&mut Self) -> (),
@@ -103,24 +134,7 @@ impl Analyzer {
134 assert_eq!(self.current, id, "Mismatched enter_scope/close_scope");
135 let scope = self.manager.mut_scope(self.current);
136 let parent = scope.parent.unwrap();
106 - let unresolved = std::mem::take(&mut scope.unresolved);
107 - drop(scope);
137 self.current = parent;
109 -
110 - // Lookup unresolved nodes from the child scope in the (now-current) parent scope
111 - for reference in unresolved {
112 - if let Some(declaration) = self
113 - .manager
114 - .lookup_declaration(reference.scope, &reference.name)
115 - {
116 - let id =
117 - self.manager
118 - .add_reference(reference.scope, reference.kind, declaration.id);
119 - self.manager.node_references.insert(reference.ast, id);
120 - } else {
121 - self.manager.push_unresolved_reference(parent, reference);
122 - }
123 - }
138 }
139
140 fn visit_function<T: IntoFunction>(&mut self, node: &T) {
@@ -168,8 +182,13 @@ impl Analyzer {
182 kind: ReferenceKind,
183 range: Option<SourceRange>,
184 ) {
171 - self.manager
172 - .add_unresolved_reference(self.current, ast, name.to_string(), kind, range);
185 + self.unresolved.push(UnresolvedReference {
186 + scope: self.current,
187 + ast,
188 + name: name.to_string(),
189 + kind,
190 + range,
191 + });
192 }
193
194 fn visit_declaration_identifier(
@@ -199,13 +218,13 @@ impl Analyzer {
218 .insert(AstNode::from(ast), id);
219 } else {
220 // Re-assigning a variable
202 - self.manager.add_unresolved_reference(
203 - self.current,
204 - AstNode::from(ast),
205 - ast.name.to_string(),
206 - ReferenceKind::Write,
207 - ast.range,
208 - );
221 + self.unresolved.push(UnresolvedReference {
222 + scope: self.current,
223 + ast: AstNode::from(ast),
224 + name: ast.name.to_string(),
225 + kind: ReferenceKind::Write,
226 + range: ast.range,
227 + });
228 }
229 }
230
@@ -642,30 +661,6 @@ impl Visitor for Analyzer {
661 )
662 }
663
645 - fn visit_program(&mut self, ast: &forget_estree::Program) {
646 - for item in &ast.body {
647 - self.visit_module_item(item);
648 - }
649 - let scope = self.manager.mut_scope(self.current);
650 - let unresolved = std::mem::take(&mut scope.unresolved);
651 - for reference in unresolved {
652 - if let Some(declaration) = self
653 - .manager
654 - .lookup_declaration(reference.scope, &reference.name)
655 - {
656 - let id =
657 - self.manager
658 - .add_reference(reference.scope, reference.kind, declaration.id);
659 - self.manager.node_references.insert(reference.ast, id);
660 - } else {
661 - self.manager.diagnostics.push(Diagnostic::invalid_syntax(
662 - "Undefined variable",
663 - reference.range,
664 - ));
665 - }
666 - }
667 - }
668 -
664 fn visit_property(&mut self, ast: &forget_estree::Property) {
665 if ast.is_computed {
666 self.visit_expression(&ast.key);
compiler/forget/crates/forget_semantic_analysis/src/scope_manager.rs
+6 -58
@@ -1,6 +1,6 @@
1 use forget_diagnostics::Diagnostic;
2 use forget_estree::{
3 - BreakStatement, ContinueStatement, ESTreeNode, LabeledStatement, SourceRange, SourceType,
3 + BreakStatement, ContinueStatement, ESTreeNode, LabeledStatement, SourceType,
4 VariableDeclarationKind,
5 };
6 use forget_utils::PointerAddress;
@@ -48,7 +48,6 @@ impl ScopeManager {
48 declarations: Default::default(),
49 references: Default::default(),
50 children: Default::default(),
51 - unresolved: Default::default(),
51 }],
52 labels: Default::default(),
53 declarations: Default::default(),
@@ -202,7 +201,6 @@ impl ScopeManager {
201 declarations: Default::default(),
202 references: Default::default(),
203 children: Default::default(),
205 - unresolved: Default::default(),
204 });
205 self.scopes[parent.0].children.push(id);
206 id
@@ -256,31 +254,14 @@ impl ScopeManager {
254 | DeclarationKind::Const
255 | DeclarationKind::CatchClause
256 | DeclarationKind::For => scope,
259 - DeclarationKind::Var => {
260 - let mut current = scope;
261 - loop {
262 - let scope = self.scope(current);
263 - match scope.kind {
264 - ScopeKind::Function | ScopeKind::Global | ScopeKind::StaticBlock => {
265 - return current;
266 - }
267 - _ => { /* no-op */ }
268 - }
269 - if let Some(parent) = &scope.parent {
270 - current = *parent
271 - } else {
272 - unreachable!("Expected scope without a parent to be a Global scope");
273 - }
274 - }
275 - }
276 - DeclarationKind::FunctionDeclaration => {
257 + DeclarationKind::Var | DeclarationKind::FunctionDeclaration => {
258 let mut current = scope;
259 loop {
260 let scope = self.scope(current);
261 match scope.kind {
262 ScopeKind::Function
282 - | ScopeKind::Module
263 | ScopeKind::Global
264 + | ScopeKind::Module
265 | ScopeKind::StaticBlock => {
266 return current;
267 }
@@ -289,7 +270,9 @@ impl ScopeManager {
270 if let Some(parent) = &scope.parent {
271 current = *parent
272 } else {
292 - unreachable!("Expected scope without a parent to be a Global scope");
273 + unreachable!(
274 + "Expected scope without a parent to be a Global or Module scope"
275 + );
276 }
277 }
278 }
@@ -312,31 +295,6 @@ impl ScopeManager {
295 self.scopes[scope.0].references.push(id);
296 id
297 }
315 -
316 - pub(crate) fn push_unresolved_reference(
317 - &mut self,
318 - scope: ScopeId,
319 - reference: UnresolvedReference,
320 - ) {
321 - self.scopes[scope.0].unresolved.push(reference);
322 - }
323 -
324 - pub(crate) fn add_unresolved_reference(
325 - &mut self,
326 - scope: ScopeId,
327 - ast: AstNode,
328 - name: String,
329 - kind: ReferenceKind,
330 - range: Option<SourceRange>,
331 - ) {
332 - self.scopes[scope.0].unresolved.push(UnresolvedReference {
333 - ast,
334 - scope,
335 - name,
336 - kind,
337 - range,
338 - });
339 - }
298 }
299
300 #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Copy, Clone)]
@@ -372,16 +330,6 @@ pub struct Scope {
330 pub declarations: IndexMap<String, DeclarationId>,
331 pub references: Vec<ReferenceId>,
332 pub children: Vec<ScopeId>,
375 - pub unresolved: Vec<UnresolvedReference>,
376 -}
377 -
378 -#[derive(Debug, Clone)]
379 -pub struct UnresolvedReference {
380 - pub scope: ScopeId,
381 - pub ast: AstNode,
382 - pub name: String,
383 - pub kind: ReferenceKind,
384 - pub range: Option<SourceRange>,
333 }
334
335 #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Copy, Clone)]
compiler/forget/crates/forget_semantic_analysis/tests/snapshots/analysis_test__fixtures@function-hoisting.js.snap
+3 -3
@@ -75,7 +75,7 @@ Scope {
75 references: [
76 Reference {
77 id: ReferenceId(
78 - 1,
78 + 0,
79 ),
80 kind: Read,
81 declaration: DeclarationId(
@@ -88,7 +88,7 @@ Scope {
88 },
89 Reference {
90 id: ReferenceId(
91 - 2,
91 + 1,
92 ),
93 kind: Read,
94 declaration: DeclarationId(
@@ -150,7 +150,7 @@ Scope {
150 references: [
151 Reference {
152 id: ReferenceId(
153 - 0,
153 + 2,
154 ),
155 kind: Read,
156 declaration: DeclarationId(
compiler/forget/crates/forget_semantic_analysis/tests/snapshots/analysis_test__fixtures@labels.js.snap
+6 -6
@@ -98,7 +98,7 @@ Scope {
98 references: [
99 Reference {
100 id: ReferenceId(
101 - 4,
101 + 0,
102 ),
103 kind: Read,
104 declaration: DeclarationId(
@@ -111,7 +111,7 @@ Scope {
111 },
112 Reference {
113 id: ReferenceId(
114 - 5,
114 + 1,
115 ),
116 kind: Read,
117 declaration: DeclarationId(
@@ -133,7 +133,7 @@ Scope {
133 references: [
134 Reference {
135 id: ReferenceId(
136 - 0,
136 + 2,
137 ),
138 kind: Read,
139 declaration: DeclarationId(
@@ -146,7 +146,7 @@ Scope {
146 },
147 Reference {
148 id: ReferenceId(
149 - 1,
149 + 3,
150 ),
151 kind: Write,
152 declaration: DeclarationId(
@@ -159,7 +159,7 @@ Scope {
159 },
160 Reference {
161 id: ReferenceId(
162 - 2,
162 + 4,
163 ),
164 kind: Read,
165 declaration: DeclarationId(
@@ -172,7 +172,7 @@ Scope {
172 },
173 Reference {
174 id: ReferenceId(
175 - 3,
175 + 5,
176 ),
177 kind: Read,
178 declaration: DeclarationId(
compiler/forget/crates/forget_semantic_analysis/tests/snapshots/analysis_test__fixtures@var-hoisting.js.snap
+113 -124
@@ -30,8 +30,17 @@ Scope {
30 id: ScopeId(
31 0,
32 ),
33 - kind: Global,
33 + kind: Module,
34 declarations: {
35 + "Component": Declaration {
36 + id: DeclarationId(
37 + 0,
38 + ),
39 + kind: FunctionDeclaration,
40 + scope: ScopeId(
41 + 0,
42 + ),
43 + },
44 "baz": Declaration {
45 id: DeclarationId(
46 5,
@@ -48,19 +57,103 @@ Scope {
57 id: ScopeId(
58 1,
59 ),
51 - kind: Module,
60 + kind: Function,
61 declarations: {
53 - "Component": Declaration {
62 + "props": Declaration {
63 id: DeclarationId(
55 - 0,
64 + 1,
65 ),
66 kind: FunctionDeclaration,
67 scope: ScopeId(
68 1,
69 ),
70 },
71 + "foo": Declaration {
72 + id: DeclarationId(
73 + 2,
74 + ),
75 + kind: FunctionDeclaration,
76 + scope: ScopeId(
77 + 1,
78 + ),
79 + },
80 + "bar": Declaration {
81 + id: DeclarationId(
82 + 4,
83 + ),
84 + kind: Var,
85 + scope: ScopeId(
86 + 1,
87 + ),
88 + },
89 },
63 - references: [],
90 + references: [
91 + Reference {
92 + id: ReferenceId(
93 + 0,
94 + ),
95 + kind: Read,
96 + declaration: DeclarationId(
97 + 4,
98 + ),
99 + declaration (name): "bar",
100 + scope: ScopeId(
101 + 1,
102 + ),
103 + },
104 + Reference {
105 + id: ReferenceId(
106 + 1,
107 + ),
108 + kind: Write,
109 + declaration: DeclarationId(
110 + 4,
111 + ),
112 + declaration (name): "bar",
113 + scope: ScopeId(
114 + 1,
115 + ),
116 + },
117 + Reference {
118 + id: ReferenceId(
119 + 2,
120 + ),
121 + kind: Read,
122 + declaration: DeclarationId(
123 + 5,
124 + ),
125 + declaration (name): "baz",
126 + scope: ScopeId(
127 + 1,
128 + ),
129 + },
130 + Reference {
131 + id: ReferenceId(
132 + 3,
133 + ),
134 + kind: Write,
135 + declaration: DeclarationId(
136 + 5,
137 + ),
138 + declaration (name): "baz",
139 + scope: ScopeId(
140 + 1,
141 + ),
142 + },
143 + Reference {
144 + id: ReferenceId(
145 + 7,
146 + ),
147 + kind: Read,
148 + declaration: DeclarationId(
149 + 1,
150 + ),
151 + declaration (name): "props",
152 + scope: ScopeId(
153 + 1,
154 + ),
155 + },
156 + ],
157 children: [
158 Scope {
159 id: ScopeId(
@@ -68,27 +161,9 @@ Scope {
161 ),
162 kind: Function,
163 declarations: {
71 - "props": Declaration {
72 - id: DeclarationId(
73 - 1,
74 - ),
75 - kind: FunctionDeclaration,
76 - scope: ScopeId(
77 - 2,
78 - ),
79 - },
80 - "foo": Declaration {
81 - id: DeclarationId(
82 - 2,
83 - ),
84 - kind: FunctionDeclaration,
85 - scope: ScopeId(
86 - 2,
87 - ),
88 - },
164 "bar": Declaration {
165 id: DeclarationId(
91 - 4,
166 + 3,
167 ),
168 kind: Var,
169 scope: ScopeId(
@@ -99,11 +174,11 @@ Scope {
174 references: [
175 Reference {
176 id: ReferenceId(
102 - 3,
177 + 4,
178 ),
179 kind: Read,
180 declaration: DeclarationId(
106 - 4,
181 + 3,
182 ),
183 declaration (name): "bar",
184 scope: ScopeId(
@@ -112,11 +187,11 @@ Scope {
187 },
188 Reference {
189 id: ReferenceId(
115 - 4,
190 + 5,
191 ),
192 kind: Write,
193 declaration: DeclarationId(
119 - 4,
194 + 3,
195 ),
196 declaration (name): "bar",
197 scope: ScopeId(
@@ -125,7 +200,7 @@ Scope {
200 },
201 Reference {
202 id: ReferenceId(
128 - 5,
203 + 6,
204 ),
205 kind: Read,
206 declaration: DeclarationId(
@@ -136,107 +211,12 @@ Scope {
211 2,
212 ),
213 },
139 - Reference {
140 - id: ReferenceId(
141 - 6,
142 - ),
143 - kind: Read,
144 - declaration: DeclarationId(
145 - 5,
146 - ),
147 - declaration (name): "baz",
148 - scope: ScopeId(
149 - 2,
150 - ),
151 - },
152 - Reference {
153 - id: ReferenceId(
154 - 7,
155 - ),
156 - kind: Write,
157 - declaration: DeclarationId(
158 - 5,
159 - ),
160 - declaration (name): "baz",
161 - scope: ScopeId(
162 - 2,
163 - ),
164 - },
214 ],
215 children: [
216 Scope {
217 id: ScopeId(
218 3,
219 ),
171 - kind: Function,
172 - declarations: {
173 - "bar": Declaration {
174 - id: DeclarationId(
175 - 3,
176 - ),
177 - kind: Var,
178 - scope: ScopeId(
179 - 3,
180 - ),
181 - },
182 - },
183 - references: [
184 - Reference {
185 - id: ReferenceId(
186 - 0,
187 - ),
188 - kind: Read,
189 - declaration: DeclarationId(
190 - 3,
191 - ),
192 - declaration (name): "bar",
193 - scope: ScopeId(
194 - 3,
195 - ),
196 - },
197 - Reference {
198 - id: ReferenceId(
199 - 1,
200 - ),
201 - kind: Write,
202 - declaration: DeclarationId(
203 - 3,
204 - ),
205 - declaration (name): "bar",
206 - scope: ScopeId(
207 - 3,
208 - ),
209 - },
210 - Reference {
211 - id: ReferenceId(
212 - 2,
213 - ),
214 - kind: Read,
215 - declaration: DeclarationId(
216 - 1,
217 - ),
218 - declaration (name): "props",
219 - scope: ScopeId(
220 - 3,
221 - ),
222 - },
223 - ],
224 - children: [
225 - Scope {
226 - id: ScopeId(
227 - 4,
228 - ),
229 - kind: Block,
230 - declarations: {},
231 - references: [],
232 - children: [],
233 - },
234 - ],
235 - },
236 - Scope {
237 - id: ScopeId(
238 - 5,
239 - ),
220 kind: Block,
221 declarations: {},
222 references: [],
@@ -244,6 +224,15 @@ Scope {
224 },
225 ],
226 },
227 + Scope {
228 + id: ScopeId(
229 + 4,
230 + ),
231 + kind: Block,
232 + declarations: {},
233 + references: [],
234 + children: [],
235 + },
236 ],
237 },
238 ],