From c3ad8a86b180772dfee9cf2f2b32b0a6848893b4 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Thu, 10 Aug 2023 10:59:51 -0400 Subject: [PATCH] [rust] Semantic analysis resolves break/continue --- .../forget_semantic_analysis/src/analyzer.rs | 178 +++++++++++------- .../src/scope_manager.rs | 43 ++--- .../src/scope_view.rs | 15 -- ...test__fixtures@globals-and-imports.js.snap | 5 - .../analysis_test__fixtures@labels.js.snap | 26 --- ...sis_test__fixtures@simple-function.js.snap | 5 - 6 files changed, 127 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 872cd72a3c..e51515b648 100644 --- a/compiler/forget/crates/forget_semantic_analysis/src/analyzer.rs +++ b/compiler/forget/crates/forget_semantic_analysis/src/analyzer.rs @@ -5,7 +5,9 @@ use forget_estree::{ Pattern, Program, SourceRange, SourceType, Statement, VariableDeclarationKind, Visitor, }; -use crate::{AstNode, DeclarationKind, LabelKind, ReferenceKind, ScopeId, ScopeKind, ScopeManager}; +use crate::{ + AstNode, DeclarationKind, LabelId, LabelKind, ReferenceKind, ScopeId, ScopeKind, ScopeManager, +}; pub fn analyze(ast: &Program) -> ScopeManager { let mut analyzer = Analyzer::new(); @@ -15,6 +17,7 @@ pub fn analyze(ast: &Program) -> ScopeManager { struct Analyzer { manager: ScopeManager, + labels: Vec, current: ScopeId, } @@ -22,7 +25,64 @@ impl Analyzer { fn new() -> Self { let manager = ScopeManager::new(); let current = manager.root_id(); - Self { manager, current } + let labels = Default::default(); + Self { + manager, + labels, + current, + } + } + + fn enter_label(&mut self, id: LabelId, mut f: F) + where + F: FnMut(&mut Self) -> (), + { + self.labels.push(id); + f(self); + let last = self.labels.pop().unwrap(); + assert_eq!(last, id); + } + + fn lookup_break(&self, name: Option<&str>) -> Option { + for id in self.labels.iter().rev() { + let label = self.manager.label(*id); + match (name, &label.name) { + // If this is a labeled break, only return if an exact match + // is in scope + (Some(name), Some(label_name)) if name == label_name => { + return Some(label.id); + } + // If this is an unlabeld break, return the innermost label id + (None, _) => { + return Some(label.id); + } + _ => { /* no-op */ } + } + } + None + } + + fn lookup_continue(&self, name: Option<&str>) -> Option { + for id in self.labels.iter().rev() { + let label = self.manager.label(*id); + // Skip labels that are not for loops, can only continue to a loop + if label.kind != LabelKind::Loop { + continue; + } + match (name, &label.name) { + // If this is a labeled break, only return if an exact match + // is in scope + (Some(name), Some(label_name)) if &name == label_name => { + return Some(label.id); + } + // If this is an unlabeld break, return the innermost label id + (None, _) => { + return Some(label.id); + } + _ => { /* no-op */ } + } + } + None } fn enter(&mut self, kind: ScopeKind, mut f: F) -> ScopeId @@ -196,12 +256,6 @@ impl Analyzer { body: &Statement, _range: Option, ) { - // Record an anonymous label for the statement to resolve unlabeled break/continue - let label = self - .manager - .add_anonymous_label(self.current, LabelKind::Loop); - self.manager.node_labels.insert(ast, label); - let mut for_scope: Option = None; match left { ForInInit::VariableDeclaration(left) => { @@ -215,7 +269,13 @@ impl Analyzer { } } self.visit_expression(right); - self.visit_statement(body); + let id = self + .manager + .add_anonymous_label(self.current, LabelKind::Loop); + self.manager.node_labels.insert(ast, id); + self.enter_label(id, |visitor| { + visitor.visit_statement(body); + }); if let Some(for_scope) = for_scope { self.close_scope(for_scope); } @@ -365,34 +425,22 @@ impl Visitor for Analyzer { } fn visit_break_statement(&mut self, ast: &forget_estree::BreakStatement) { - if let Some(label_node) = &ast.label { - if let Some(label) = self - .manager - .lookup_label(self.current, &label_node.name) - .cloned() - { + if let Some(label_id) = + self.lookup_break(ast.label.as_ref().map(|ident| ident.name.as_str())) + { + self.manager + .node_labels + .insert(AstNode::from(ast), label_id); + if let Some(label_node) = &ast.label { self.manager .node_labels - .insert(AstNode::from(ast), label.id); - self.manager - .node_labels - .insert(AstNode::from(label_node), label.id); - } else { - self.manager.diagnostics.push(Diagnostic::invalid_syntax( - "Unknown break label", - label_node.range, - )); + .insert(AstNode::from(label_node), label_id); } } else { - if let Some(label) = self.manager.lookup_break(self.current).cloned() { - self.manager - .node_labels - .insert(AstNode::from(ast), label.id); - } else { - self.manager - .diagnostics - .push(Diagnostic::invalid_syntax("Invalid break", ast.range)); - } + self.manager.diagnostics.push(Diagnostic::invalid_syntax( + "Non-syntactic break, could not resolve break target", + ast.range, + )); } } @@ -414,34 +462,22 @@ impl Visitor for Analyzer { } fn visit_continue_statement(&mut self, ast: &forget_estree::ContinueStatement) { - if let Some(label_node) = &ast.label { - if let Some(label) = self - .manager - .lookup_label(self.current, &label_node.name) - .cloned() - { + if let Some(label_id) = + self.lookup_continue(ast.label.as_ref().map(|ident| ident.name.as_str())) + { + self.manager + .node_labels + .insert(AstNode::from(ast), label_id); + if let Some(label_node) = &ast.label { self.manager .node_labels - .insert(AstNode::from(ast), label.id); - self.manager - .node_labels - .insert(AstNode::from(label_node), label.id); - } else { - self.manager.diagnostics.push(Diagnostic::invalid_syntax( - "Unknown continue label", - label_node.range, - )); + .insert(AstNode::from(label_node), label_id); } } else { - if let Some(label) = self.manager.lookup_continue(self.current).cloned() { - self.manager - .node_labels - .insert(AstNode::from(ast), label.id); - } else { - self.manager - .diagnostics - .push(Diagnostic::invalid_syntax("Invalid continue", ast.range)); - } + self.manager.diagnostics.push(Diagnostic::invalid_syntax( + "Non-syntactic continue, could not resolve continue target", + ast.range, + )); } } @@ -485,7 +521,13 @@ impl Visitor for Analyzer { if let Some(update) = &ast.update { self.visit_expression(update); } - self.visit_statement(&ast.body); + let id = self + .manager + .add_anonymous_label(self.current, LabelKind::Loop); + self.manager.node_labels.insert(AstNode::from(ast), id); + self.enter_label(id, |visitor| { + visitor.visit_statement(&ast.body); + }); if let Some(for_scope) = for_scope { self.close_scope(for_scope); } @@ -522,7 +564,9 @@ impl Visitor for Analyzer { .manager .add_label(self.current, kind, ast.label.name.clone()); self.manager.node_labels.insert(AstNode::from(ast), id); - self.visit_statement(body); + self.enter_label(id, |visitor| { + visitor.visit_statement(body); + }) } fn visit_member_expression(&mut self, ast: &forget_estree::MemberExpression) { @@ -578,10 +622,16 @@ impl Visitor for Analyzer { fn visit_switch_statement(&mut self, ast: &forget_estree::SwitchStatement) { self.visit_expression(&ast.discriminant); - self.enter(ScopeKind::Switch, |visitor| { - for case_ in &ast.cases { - visitor.visit_switch_case(case_); - } + let id = self + .manager + .add_anonymous_label(self.current, LabelKind::Other); + self.manager.node_labels.insert(AstNode::from(ast), id); + self.enter_label(id, |visitor| { + visitor.enter(ScopeKind::Switch, |visitor| { + for case_ in &ast.cases { + visitor.visit_switch_case(case_); + } + }); }); } @@ -651,7 +701,7 @@ impl Visitor for Analyzer { // should never result in an empty JSXIdentifier node. but just in // case we report this rather than silently fail self.manager.diagnostics.push(Diagnostic::invalid_syntax( - "Expected JSXOpenintElement.name to be non-empty", + "Expected JSXOpeningElement.name to be non-empty", name.range, )); } 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 412e973d93..d52ce3d850 100644 --- a/compiler/forget/crates/forget_semantic_analysis/src/scope_manager.rs +++ b/compiler/forget/crates/forget_semantic_analysis/src/scope_manager.rs @@ -40,7 +40,6 @@ impl ScopeManager { id: root_id, kind: ScopeKind::Global, parent: None, - labels: Default::default(), declarations: Default::default(), references: Default::default(), children: Default::default(), @@ -166,28 +165,6 @@ impl ScopeManager { }) } - pub fn lookup_label(&self, scope: ScopeId, name: &str) -> Option<&Label> { - let mut current = &self.scopes[scope.0]; - loop { - if let Some(id) = current.labels.get(name) { - return Some(&self.labels[id.0]); - } - if let Some(parent) = current.parent { - current = &self.scopes[parent.0]; - } else { - return None; - } - } - } - - pub fn lookup_break(&self, _scope: ScopeId) -> Option<&Label> { - todo!() - } - - pub fn lookup_continue(&self, _scope: ScopeId) -> Option<&Label> { - todo!() - } - pub fn lookup_declaration(&self, scope: ScopeId, name: &str) -> Option<&Declaration> { let mut current = &self.scopes[scope.0]; loop { @@ -212,7 +189,6 @@ impl ScopeManager { id, kind, parent: Some(parent), - labels: Default::default(), declarations: Default::default(), references: Default::default(), children: Default::default(), @@ -223,16 +199,23 @@ impl ScopeManager { pub(crate) fn add_label(&mut self, scope: ScopeId, kind: LabelKind, name: String) -> LabelId { let id = LabelId(self.labels.len()); - self.labels.push(Label { id, kind, scope }); - self.scopes[scope.0].labels.insert(name, id); + self.labels.push(Label { + id, + kind, + scope, + name: Some(name), + }); id } pub(crate) fn add_anonymous_label(&mut self, scope: ScopeId, kind: LabelKind) -> LabelId { let id = LabelId(self.labels.len()); - let name = format!("#{}", id.0); - self.labels.push(Label { id, kind, scope }); - self.scopes[scope.0].labels.insert(name, id); + self.labels.push(Label { + id, + kind, + scope, + name: None, + }); id } @@ -300,7 +283,6 @@ pub struct Scope { pub id: ScopeId, pub kind: ScopeKind, pub parent: Option, - pub labels: IndexMap, pub declarations: IndexMap, pub references: Vec, pub children: Vec, @@ -317,6 +299,7 @@ pub struct Label { pub id: LabelId, pub kind: LabelKind, pub scope: ScopeId, + pub name: Option, } #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Copy, Clone)] diff --git a/compiler/forget/crates/forget_semantic_analysis/src/scope_view.rs b/compiler/forget/crates/forget_semantic_analysis/src/scope_view.rs index de4270b1da..af369c5a91 100644 --- a/compiler/forget/crates/forget_semantic_analysis/src/scope_view.rs +++ b/compiler/forget/crates/forget_semantic_analysis/src/scope_view.rs @@ -83,20 +83,6 @@ impl<'m> ScopeView<'m> { impl<'m> std::fmt::Debug for ScopeView<'m> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let labels: IndexMap<_, _> = self - .scope - .labels - .iter() - .map(|(name, label)| { - ( - name.clone(), - LabelView { - manager: &self.manager, - label: self.manager.label(*label), - }, - ) - }) - .collect(); let declarations: IndexMap<_, _> = self .scope .declarations @@ -132,7 +118,6 @@ impl<'m> std::fmt::Debug for ScopeView<'m> { f.debug_struct("Scope") .field("id", &self.scope.id) .field("kind", &self.scope.kind) - .field("labels", &labels) .field("declarations", &declarations) .field("references", &references) .field("children", &children) diff --git a/compiler/forget/crates/forget_semantic_analysis/tests/snapshots/analysis_test__fixtures@globals-and-imports.js.snap b/compiler/forget/crates/forget_semantic_analysis/tests/snapshots/analysis_test__fixtures@globals-and-imports.js.snap index 10e8be739d..5dc4aa200a 100644 --- a/compiler/forget/crates/forget_semantic_analysis/tests/snapshots/analysis_test__fixtures@globals-and-imports.js.snap +++ b/compiler/forget/crates/forget_semantic_analysis/tests/snapshots/analysis_test__fixtures@globals-and-imports.js.snap @@ -29,7 +29,6 @@ Scope { 0, ), kind: Global, - labels: {}, declarations: {}, references: [], children: [ @@ -38,7 +37,6 @@ Scope { 1, ), kind: Module, - labels: {}, declarations: { "Component": Declaration { id: DeclarationId( @@ -57,7 +55,6 @@ Scope { 2, ), kind: Function, - labels: {}, declarations: { "props": Declaration { id: DeclarationId( @@ -170,7 +167,6 @@ Scope { 3, ), kind: Function, - labels: {}, declarations: {}, references: [], children: [], @@ -180,7 +176,6 @@ Scope { 4, ), kind: Function, - labels: {}, declarations: {}, references: [], children: [], diff --git a/compiler/forget/crates/forget_semantic_analysis/tests/snapshots/analysis_test__fixtures@labels.js.snap b/compiler/forget/crates/forget_semantic_analysis/tests/snapshots/analysis_test__fixtures@labels.js.snap index 3d90fd2649..2fe1e40bc6 100644 --- a/compiler/forget/crates/forget_semantic_analysis/tests/snapshots/analysis_test__fixtures@labels.js.snap +++ b/compiler/forget/crates/forget_semantic_analysis/tests/snapshots/analysis_test__fixtures@labels.js.snap @@ -25,7 +25,6 @@ Scope { 0, ), kind: Global, - labels: {}, declarations: {}, references: [], children: [ @@ -34,7 +33,6 @@ Scope { 1, ), kind: Module, - labels: {}, declarations: { "Component": Declaration { id: DeclarationId( @@ -53,26 +51,6 @@ Scope { 2, ), kind: Function, - labels: { - "foo": Label { - id: LabelId( - 0, - ), - kind: Loop, - scope: ScopeId( - 2, - ), - }, - "bar": Label { - id: LabelId( - 1, - ), - kind: Other, - scope: ScopeId( - 2, - ), - }, - }, declarations: { "props": Declaration { id: DeclarationId( @@ -114,7 +92,6 @@ Scope { 3, ), kind: For, - labels: {}, declarations: { "x": Declaration { id: DeclarationId( @@ -160,7 +137,6 @@ Scope { 4, ), kind: Block, - labels: {}, declarations: {}, references: [ Reference { @@ -222,7 +198,6 @@ Scope { 5, ), kind: Block, - labels: {}, declarations: {}, references: [], children: [], @@ -236,7 +211,6 @@ Scope { 6, ), kind: Block, - labels: {}, declarations: {}, references: [], 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 f03f5c92e6..a743c0c3d4 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 @@ -21,7 +21,6 @@ Scope { 0, ), kind: Global, - labels: {}, declarations: {}, references: [], children: [ @@ -30,7 +29,6 @@ Scope { 1, ), kind: Module, - labels: {}, declarations: { "Component": Declaration { id: DeclarationId( @@ -49,7 +47,6 @@ Scope { 2, ), kind: Function, - labels: {}, declarations: { "a": Declaration { id: DeclarationId( @@ -100,7 +97,6 @@ Scope { 3, ), kind: Function, - labels: {}, declarations: { "foo_": Declaration { id: DeclarationId( @@ -119,7 +115,6 @@ Scope { 4, ), kind: Function, - labels: {}, declarations: { "c": Declaration { id: DeclarationId(