Skip to content

Commit

Permalink
fix(semantic/cfg): discrete finalization path after NewFunctions. (#…
Browse files Browse the repository at this point in the history
…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)
  • Loading branch information
rzvxa committed Jun 14, 2024
1 parent 67e0d30 commit abd6ac8
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 4 deletions.
18 changes: 18 additions & 0 deletions crates/oxc_linter/src/rules/eslint/no_this_before_super.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
4 changes: 4 additions & 0 deletions crates/oxc_semantic/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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 */
Expand Down Expand Up @@ -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();
Expand All @@ -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 {
Expand Down
18 changes: 14 additions & 4 deletions crates/oxc_semantic/src/control_flow/builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ pub struct ControlFlowGraphBuilder<'a> {
/// Contains the error unwinding path represented as a stack of `ErrorHarness`es
error_path: Vec<ErrorHarness>,
/// Stack of finalizers, the top most element is always the appropriate one for current node.
finalizers: Vec<BasicBlockId>,
finalizers: Vec<Option<BasicBlockId>>,
}

impl<'a> ControlFlowGraphBuilder<'a> {
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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."
);
}
Expand Down

0 comments on commit abd6ac8

Please sign in to comment.