From abd6ac8811dba9f18b6e4a51e32fff7a0aabec18 Mon Sep 17 00:00:00 2001 From: rzvxa <3788964+rzvxa@users.noreply.github.com> Date: Fri, 14 Jun 2024 08:32:02 +0000 Subject: [PATCH] fix(semantic/cfg): discrete finalization path after `NewFunction`s. (#3671) closes #3668 [oxlint-ecosystem-ci](https://github.com/rzvxa/oxlint-ecosystem-ci/actions/runs/9512489987/job/26220576138) For this code: ```js function f() { try { return a(); } catch (err) { throw new class CustomError extends Error { constructor() { super(err); } }; } finally { this.b(); } } ``` We went from this: ![image](https://github.com/oxc-project/oxc/assets/3788964/bcb751aa-50cf-4c0a-8975-e01697ff78b2) To this: ![Screenshot 2024-06-14 110805](https://github.com/oxc-project/oxc/assets/3788964/03a03525-5326-47b1-8d6c-69720f7f3149) --- .../src/rules/eslint/no_this_before_super.rs | 18 ++++++++++++++++++ crates/oxc_semantic/src/builder.rs | 4 ++++ .../src/control_flow/builder/mod.rs | 18 ++++++++++++++---- 3 files changed, 36 insertions(+), 4 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_this_before_super.rs b/crates/oxc_linter/src/rules/eslint/no_this_before_super.rs index ab3e0a319342a..69d31058a1a05 100644 --- a/crates/oxc_linter/src/rules/eslint/no_this_before_super.rs +++ b/crates/oxc_linter/src/rules/eslint/no_this_before_super.rs @@ -273,6 +273,24 @@ fn test() { // allows `this`/`super` after `super()`. ("class A extends B { }", None), ("class A extends B { constructor() { super(); } }", None), + ( + " + function f() { + try { + return a(); + } + catch (err) { + throw new class CustomError extends Error { + constructor() { + super(err); + } + }; + } + finally { + this.b(); + } + } + ", None), ("class A extends B { constructor() { super(); this.c = this.d; } }", None), ("class A extends B { constructor() { super(); this.c(); } }", None), ("class A extends B { constructor() { super(); super.c(); } }", None), diff --git a/crates/oxc_semantic/src/builder.rs b/crates/oxc_semantic/src/builder.rs index 354b570f122c7..bb6cf3d0ba833 100644 --- a/crates/oxc_semantic/src/builder.rs +++ b/crates/oxc_semantic/src/builder.rs @@ -1367,6 +1367,7 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> { /* cfg */ let before_function_graph_ix = self.cfg.current_node_ix; + self.cfg.push_finalization_stack(); let error_harness = self.cfg.attach_error_harness(ErrorEdgeKind::Implicit); let function_graph_ix = self.cfg.new_basic_block_function(); self.cfg.ctx(None).new_function(); @@ -1391,6 +1392,7 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> { /* cfg */ self.cfg.ctx(None).resolve_expect(CtxFlags::FUNCTION); self.cfg.release_error_harness(error_harness); + self.cfg.pop_finalization_stack(); let after_function_graph_ix = self.cfg.new_basic_block_normal(); self.cfg.add_edge(before_function_graph_ix, after_function_graph_ix, EdgeType::Normal); /* cfg */ @@ -1463,6 +1465,7 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> { /* cfg */ let current_node_ix = self.cfg.current_node_ix; + self.cfg.push_finalization_stack(); let error_harness = self.cfg.attach_error_harness(ErrorEdgeKind::Implicit); let function_graph_ix = self.cfg.new_basic_block_function(); self.cfg.ctx(None).new_function(); @@ -1483,6 +1486,7 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> { /* cfg */ self.cfg.ctx(None).resolve_expect(CtxFlags::FUNCTION); self.cfg.release_error_harness(error_harness); + self.cfg.pop_finalization_stack(); self.cfg.current_node_ix = current_node_ix; /* cfg */ if let Some(parameters) = &expr.type_parameters { diff --git a/crates/oxc_semantic/src/control_flow/builder/mod.rs b/crates/oxc_semantic/src/control_flow/builder/mod.rs index dc8f7c112d012..d72e9d730b755 100644 --- a/crates/oxc_semantic/src/control_flow/builder/mod.rs +++ b/crates/oxc_semantic/src/control_flow/builder/mod.rs @@ -23,7 +23,7 @@ pub struct ControlFlowGraphBuilder<'a> { /// Contains the error unwinding path represented as a stack of `ErrorHarness`es error_path: Vec, /// Stack of finalizers, the top most element is always the appropriate one for current node. - finalizers: Vec, + finalizers: Vec>, } impl<'a> ControlFlowGraphBuilder<'a> { @@ -81,7 +81,7 @@ impl<'a> ControlFlowGraphBuilder<'a> { self.error_path.last().expect("normal basic blocks need an error harness to attach to"); self.add_edge(graph_ix, *error_graph_ix, EdgeType::Error(*error_edge_kind)); - if let Some(finalizer) = self.finalizers.last() { + if let Some(Some(finalizer)) = self.finalizers.last() { self.add_edge(graph_ix, *finalizer, EdgeType::Finalize); } @@ -139,16 +139,26 @@ impl<'a> ControlFlowGraphBuilder<'a> { /// Returns the `BasicBlockId` of the created finalizer block. pub fn attach_finalizer(&mut self) -> BasicBlockId { let graph_ix = self.new_basic_block(); - self.finalizers.push(graph_ix); + self.finalizers.push(Some(graph_ix)); graph_ix } + pub fn push_finalization_stack(&mut self) { + self.finalizers.push(None); + } + + pub fn pop_finalization_stack(&mut self) { + let result = self.finalizers.pop(); + debug_assert!(result.as_ref().is_some_and(Option::is_none)); + } + /// # Panics if last finalizer doesn't match the expected `BasicBlockId`. pub fn release_finalizer(&mut self, expect: BasicBlockId) { // return early if there is no finalizer. let Some(finalizer) = self.finalizers.pop() else { return }; assert_eq!( - finalizer, expect, + finalizer, + Some(expect), "expected finalizer doesn't match the last finalizer pushed onto the stack." ); }