Skip to content

Commit

Permalink
improvement(semantic/cfg): better control flow for switch statements. (
Browse files Browse the repository at this point in the history
…#3547)

There was an issue similar to the one in loops see #3451 to #3453
  • Loading branch information
rzvxa committed Jun 13, 2024
1 parent 9b30971 commit defef05
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 47 deletions.
63 changes: 30 additions & 33 deletions crates/oxc_semantic/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1027,37 +1027,37 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {

/* cfg */
let discriminant_graph_ix = self.cfg.current_node_ix;
self.cfg.switch_case_conditions.push(vec![]);

self.cfg.ctx(None).default().allow_break();
let mut ends_of_switch_cases = vec![];
let mut switch_case_graph_spans = vec![];
let mut have_default_case = false;
/* cfg */

for case in &stmt.cases {
let before_case_graph_ix = self.cfg.new_basic_block_normal();
self.visit_switch_case(case);
ends_of_switch_cases.push(self.cfg.current_node_ix);
if case.is_default_case() {
have_default_case = true;
}
switch_case_graph_spans.push((before_case_graph_ix, self.cfg.current_node_ix));
}

/* cfg */
let switch_case_conditions = self.cfg.switch_case_conditions.pop().expect(
"there must be a corresponding previous_switch_case_last_block in a switch statement",
);

// for each switch case
for i in 0..switch_case_conditions.len() {
let switch_case_condition_graph_ix = switch_case_conditions[i];
for i in 0..switch_case_graph_spans.len() {
let case_graph_span = switch_case_graph_spans[i];

// every switch case condition can be skipped,
// so there's a possible jump from it to the next switch case condition
for y in switch_case_conditions.iter().skip(i + 1) {
self.cfg.add_edge(switch_case_condition_graph_ix, *y, EdgeType::Normal);
for y in switch_case_graph_spans.iter().skip(i + 1) {
self.cfg.add_edge(case_graph_span.0, y.0, EdgeType::Normal);
}

// connect the end of each switch statement to
// the condition of the next switch statement
if switch_case_conditions.len() > i + 1 {
let end_of_switch_case = ends_of_switch_cases[i];
let next_switch_statement_condition = switch_case_conditions[i + 1];
if switch_case_graph_spans.len() > i + 1 {
let (_, end_of_switch_case) = switch_case_graph_spans[i];
let (next_switch_statement_condition, _) = switch_case_graph_spans[i + 1];

self.cfg.add_edge(
end_of_switch_case,
Expand All @@ -1066,19 +1066,26 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
);
}

self.cfg.add_edge(discriminant_graph_ix, case_graph_span.0, EdgeType::Normal);
}

let end_of_switch_case_statement = self.cfg.new_basic_block_normal();

if let Some(last) = switch_case_graph_spans.last() {
self.cfg.add_edge(last.1, end_of_switch_case_statement, EdgeType::Normal);
}

// if we don't have a default case there should be an edge from discriminant to the end of
// the statement.
if !have_default_case {
self.cfg.add_edge(
discriminant_graph_ix,
switch_case_condition_graph_ix,
end_of_switch_case_statement,
EdgeType::Normal,
);
}

if let Some(last) = switch_case_conditions.last() {
self.cfg.add_edge(*last, self.cfg.current_node_ix, EdgeType::Normal);
}

let current_node_ix = self.cfg.current_node_ix;
self.cfg.ctx(None).mark_break(current_node_ix).resolve();
self.cfg.ctx(None).mark_break(end_of_switch_case_statement).resolve();
/* cfg */

self.leave_scope();
Expand All @@ -1089,24 +1096,14 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> {
let kind = AstKind::SwitchCase(self.alloc(case));
self.enter_node(kind);

/* cfg */
// make a new basic block so that we can jump to it later from the switch
// discriminant and the switch cases above it (if they don't test ss true)
let switch_cond_graph_ix = self.cfg.new_basic_block_normal();
self.cfg
.switch_case_conditions
.last_mut()
.expect("there must be a switch_case_conditions while in a switch case")
.push(switch_cond_graph_ix);
/* cfg */

if let Some(expr) = &case.test {
self.visit_expression(expr);
}

/* cfg */
let after_test_graph_ix = self.cfg.current_node_ix;
let statements_in_switch_graph_ix = self.cfg.new_basic_block_normal();
self.cfg.add_edge(switch_cond_graph_ix, statements_in_switch_graph_ix, EdgeType::Normal);
self.cfg.add_edge(after_test_graph_ix, statements_in_switch_graph_ix, EdgeType::Normal);
/* cfg */

self.visit_statements(&case.consequent);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ digraph {
7 [ label = "" ]
8 [ label = "ExpressionStatement" ]
9 [ label = "" ]
10 [ label = "" ]
1 -> 0 [ label = "Error(Implicit)" ]
2 -> 0 [ label = "Error(Implicit)" ]
3 -> 0 [ label = "Error(Implicit)" ]
Expand All @@ -29,11 +30,13 @@ digraph {
6 -> 7 [ label = "Normal" ]
3 -> 4 [ label = "Normal" ]
3 -> 7 [ label = "Normal" ]
7 -> 8 [ label = "Normal" ]
9 -> 0 [ label = "Error(Implicit)" ]
8 -> 9 [ label = "Normal" ]
3 -> 9 [ label = "Normal" ]
10 -> 0 [ label = "Error(Implicit)" ]
1 -> 2 [ label = "Normal" ]
2 -> 3 [ label = "Normal" ]
8 -> 2 [ label = "Backedge" ]
2 -> 9 [ label = "Normal" ]
9 -> 2 [ label = "Backedge" ]
2 -> 10 [ label = "Normal" ]
5 -> 2 [ label = "Jump" ]
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,7 @@ bb8: {
bb9: {

}

bb10: {

}
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,10 @@ digraph {
16 [ label = "ExpressionStatement\nExpressionStatement\nExpressionStatement" ]
17 [ label = "" ]
18 [ label = "ExpressionStatement\nreturn <value>" ]
19 [ label = "unreachable\nExpressionStatement\nreturn <value>" ]
20 [ label = "unreachable" ]
21 [ label = "" ]
19 [ label = "unreachable" ]
20 [ label = "ExpressionStatement\nreturn <value>" ]
21 [ label = "unreachable" ]
22 [ label = "" ]
1 -> 0 [ label = "Error(Implicit)" ]
3 -> 2 [ label = "Error(Implicit)" ]
1 -> 3 [ label = "NewFunction" ]
Expand Down Expand Up @@ -81,11 +82,12 @@ digraph {
16 -> 17 [ label = "Normal" ]
3 -> 15 [ label = "Normal" ]
3 -> 17 [ label = "Normal" ]
17 -> 19 [ label = "Normal" ]
5 -> 19 [ label = "Jump" ]
10 -> 19 [ label = "Jump" ]
20 -> 2 [ label = "Error(Implicit)" ]
19 -> 20 [ label = "Unreachable" , style = "dotted" ]
21 -> 0 [ label = "Error(Implicit)" ]
1 -> 21 [ label = "Normal" ]
19 -> 20 [ label = "Normal" ]
5 -> 20 [ label = "Jump" ]
10 -> 20 [ label = "Jump" ]
21 -> 2 [ label = "Error(Implicit)" ]
20 -> 21 [ label = "Unreachable" , style = "dotted" ]
22 -> 0 [ label = "Error(Implicit)" ]
1 -> 22 [ label = "Normal" ]
}
Original file line number Diff line number Diff line change
Expand Up @@ -90,14 +90,17 @@ bb18: {

bb19: {
unreachable
}

bb20: {
statement
return <value>
}

bb20: {
bb21: {
unreachable
}

bb21: {
bb22: {

}

0 comments on commit defef05

Please sign in to comment.