Skip to content

Commit

Permalink
fix(semantic/cfg): correct unreachability propagation in try-finally. (
Browse files Browse the repository at this point in the history
…#3667)

closes #3663


[oxlint-ecosystem-ci](https://github.com/rzvxa/oxlint-ecosystem-ci/actions/runs/9511509383/job/26217870705)

From this:

![Screenshot 2024-06-14
092916](https://github.com/oxc-project/oxc/assets/3788964/9e6a9a01-984a-4a19-8a73-abc3b71fb9c2)

To this:

![Screenshot 2024-06-14
092852](https://github.com/oxc-project/oxc/assets/3788964/46483bc2-a227-4416-b8da-07b11ab96990)

Since try-finally (without a catch) wouldn't join after finalization.
  • Loading branch information
rzvxa authored Jun 14, 2024
1 parent c7c22b7 commit e148a32
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 18 deletions.
31 changes: 16 additions & 15 deletions crates/oxc_linter/src/rules/eslint/no_unreachable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,25 +50,18 @@ impl Rule for NoUnreachable {
// In our first path we first check if each block is definitely unreachable, If it is then
// we set it as such, If we encounter an infinite loop we keep its end block since it can
// prevent other reachable blocks from ever getting executed.
let _: Control<()> = depth_first_search(graph, Some(root.cfg_id()), |event| match event {
DfsEvent::Finish(node, _) => {
let bb = cfg.basic_block(node);
let unreachable = if bb.unreachable {
true
} else {
graph
.edges_directed(node, Direction::Incoming)
.any(|edge| matches!(edge.weight(), EdgeType::Join))
};
let _: Control<()> = depth_first_search(graph, Some(root.cfg_id()), |event| {
if let DfsEvent::Finish(node, _) = event {
let unreachable = cfg.basic_block(node).unreachable;
unreachables[node.index()] = unreachable;

if let Some(it) = cfg.is_infinite_loop_start(node, nodes) {
infinite_loops.push(it);
if !unreachable {
if let Some(it) = cfg.is_infinite_loop_start(node, nodes) {
infinite_loops.push(it);
}
}

Control::Continue
}
_ => Control::Continue,
Control::Continue
});

// In the second path we go for each infinite loop end block and follow it marking all
Expand Down Expand Up @@ -255,6 +248,14 @@ fn test() {
b();
}
",
"
try {
a();
} finally {
b();
}
c();
",
];

let fail = vec![
Expand Down
6 changes: 5 additions & 1 deletion crates/oxc_semantic/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1248,7 +1248,11 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
self.cfg.add_edge(
finally_block_end_ix,
after_try_statement_block_ix,
EdgeType::Join,
if self.cfg.basic_block(after_try_block_graph_ix).unreachable {
EdgeType::Unreachable
} else {
EdgeType::Join
},
);
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_semantic/src/control_flow/builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ impl<'a> ControlFlowGraphBuilder<'a> {
pub fn add_edge(&mut self, a: BasicBlockId, b: BasicBlockId, weight: EdgeType) {
if matches!(weight, EdgeType::NewFunction) {
self.basic_block_mut(b).unreachable = false;
} else if self.basic_block(a).unreachable {
} else if matches!(weight, EdgeType::Unreachable) || self.basic_block(a).unreachable {
if self.graph.edges_directed(b, Direction::Incoming).count() == 0 {
self.basic_block_mut(b).unreachable = true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ digraph {
7 -> 8 [ label = "Unreachable" , style = "dotted" ]
9 -> 2 [ label = "Error(Implicit)" , style = "dotted" ]
3 -> 5 [ label = "Normal" ]
8 -> 9 [ label = "Join" , style = "dotted" ]
8 -> 9 [ label = "Unreachable" , style = "dotted" ]
10 -> 2 [ label = "Error(Implicit)" ]
9 -> 10 [ label = "Normal" , style = "dotted" ]
7 -> 10 [ label = "Jump" ]
Expand Down

0 comments on commit e148a32

Please sign in to comment.