From 53019093bbeed1e93bd0286bd4db60ef5f22f1f3 Mon Sep 17 00:00:00 2001 From: rzvxa <3788964+rzvxa@users.noreply.github.com> Date: Thu, 13 Jun 2024 07:28:30 +0000 Subject: [PATCH] feat(semantic/cfg): add `Condition` instruction. (#3567) --- .../src/rules/eslint/getter_return.rs | 1 + .../src/rules/react/require_render_return.rs | 1 + crates/oxc_semantic/src/builder.rs | 61 ++++++++++++++----- .../src/control_flow/builder/mod.rs | 24 +++++++- crates/oxc_semantic/src/control_flow/dot.rs | 22 ++++++- crates/oxc_semantic/src/control_flow/mod.rs | 1 + ..._cfg_files@cond_expr_in_arrow_fn.js-2.snap | 11 ++-- ...g__cfg_files@cond_expr_in_arrow_fn.js.snap | 6 +- ...cfg_files@conditional_expression.js-2.snap | 13 ++-- ...__cfg_files@conditional_expression.js.snap | 6 +- ...n__cfg__cfg_files@do_while_break.js-2.snap | 2 +- ...ion__cfg__cfg_files@do_while_break.js.snap | 2 +- ...tegration__cfg__cfg_files@for_in.js-2.snap | 2 +- ...integration__cfg__cfg_files@for_in.js.snap | 2 +- ...egration__cfg__cfg_files@if_else.js-2.snap | 43 +++++++------ ...ntegration__cfg__cfg_files@if_else.js.snap | 18 +++--- ...cfg__cfg_files@if_stmt_in_for_in.js-2.snap | 54 ++++++++-------- ...__cfg__cfg_files@if_stmt_in_for_in.js.snap | 24 +++++--- ...g__cfg_files@labeled_block_break.js-2.snap | 23 ++++--- ...cfg__cfg_files@labeled_block_break.js.snap | 12 ++-- ...ogical_expressions_short_circuit.js-2.snap | 55 ++++++++++------- ...@logical_expressions_short_circuit.js.snap | 32 +++++++--- ...__cfg__cfg_files@switch_in_while.js-2.snap | 10 +-- ...on__cfg__cfg_files@switch_in_while.js.snap | 6 +- ..._cfg__cfg_files@switch_statement.js-2.snap | 22 +++---- ...n__cfg__cfg_files@switch_statement.js.snap | 10 +-- ...ntegration__cfg__cfg_files@while.js-2.snap | 2 +- .../integration__cfg__cfg_files@while.js.snap | 2 +- 28 files changed, 299 insertions(+), 168 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/getter_return.rs b/crates/oxc_linter/src/rules/eslint/getter_return.rs index 10582862c66683..037bddf2006d2f 100644 --- a/crates/oxc_linter/src/rules/eslint/getter_return.rs +++ b/crates/oxc_linter/src/rules/eslint/getter_return.rs @@ -276,6 +276,7 @@ impl GetterReturn { | InstructionKind::Break(_) | InstructionKind::Continue(_) | InstructionKind::Iteration(_) + | InstructionKind::Condition | InstructionKind::Statement => {} } } diff --git a/crates/oxc_linter/src/rules/react/require_render_return.rs b/crates/oxc_linter/src/rules/react/require_render_return.rs index 49d67a1b17b330..fc75760e2bd56b 100644 --- a/crates/oxc_linter/src/rules/react/require_render_return.rs +++ b/crates/oxc_linter/src/rules/react/require_render_return.rs @@ -127,6 +127,7 @@ fn contains_return_statement<'a>(node: &AstNode<'a>, ctx: &LintContext<'a>) -> b | InstructionKind::Break(_) | InstructionKind::Continue(_) | InstructionKind::Iteration(_) + | InstructionKind::Condition | InstructionKind::Statement => {} } } diff --git a/crates/oxc_semantic/src/builder.rs b/crates/oxc_semantic/src/builder.rs index c56a760aec7721..d039aeff31a67f 100644 --- a/crates/oxc_semantic/src/builder.rs +++ b/crates/oxc_semantic/src/builder.rs @@ -569,7 +569,10 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> { let start_of_condition_graph_ix = self.cfg.new_basic_block_normal(); /* cfg */ + self.record_ast_nodes(); self.visit_expression(&stmt.test); + let test_node = self.retrieve_recorded_ast_nodes().into_iter().next(); + self.cfg.append_condition_to(start_of_condition_graph_ix, test_node); /* cfg */ let end_of_condition_graph_ix = self.cfg.current_node_ix; @@ -675,10 +678,18 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> { let kind = AstKind::ConditionalExpression(self.alloc(expr)); self.enter_node(kind); + /* cfg - condition basic block */ + let before_conditional_graph_ix = self.cfg.current_node_ix; + let start_of_condition_graph_ix = self.cfg.new_basic_block_normal(); + /* cfg */ + + self.record_ast_nodes(); self.visit_expression(&expr.test); + let test_node = self.retrieve_recorded_ast_nodes().into_iter().next(); + self.cfg.append_condition_to(start_of_condition_graph_ix, test_node); /* cfg */ - let before_conditional_expr_graph_ix = self.cfg.current_node_ix; + let after_condition_graph_ix = self.cfg.current_node_ix; // conditional expression basic block let before_consequent_expr_graph_ix = self.cfg.new_basic_block_normal(); /* cfg */ @@ -696,7 +707,12 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> { let after_alternate_graph_ix = self.cfg.current_node_ix; /* bb after conditional expression joins consequent and alternate */ let after_conditional_graph_ix = self.cfg.new_basic_block_normal(); - /* cfg */ + + self.cfg.add_edge( + before_conditional_graph_ix, + start_of_condition_graph_ix, + EdgeType::Normal, + ); self.cfg.add_edge( after_consequent_expr_graph_ix, @@ -704,17 +720,15 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> { EdgeType::Normal, ); self.cfg.add_edge( - before_conditional_expr_graph_ix, + after_condition_graph_ix, before_consequent_expr_graph_ix, - EdgeType::Normal, + EdgeType::Jump, ); - self.cfg.add_edge( - before_conditional_expr_graph_ix, - start_alternate_graph_ix, - EdgeType::Normal, - ); + self.cfg.add_edge(after_condition_graph_ix, start_alternate_graph_ix, EdgeType::Normal); self.cfg.add_edge(after_alternate_graph_ix, after_conditional_graph_ix, EdgeType::Normal); + /* cfg */ + self.leave_node(kind); } @@ -735,7 +749,10 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> { let test_graph_ix = self.cfg.new_basic_block_normal(); /* cfg */ if let Some(test) = &stmt.test { + self.record_ast_nodes(); self.visit_expression(test); + let test_node = self.retrieve_recorded_ast_nodes().into_iter().next(); + self.cfg.append_condition_to(test_graph_ix, test_node); } /* cfg */ @@ -916,10 +933,18 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> { let kind = AstKind::IfStatement(self.alloc(stmt)); self.enter_node(kind); + /* cfg - condition basic block */ + let before_if_stmt_graph_ix = self.cfg.current_node_ix; + let start_of_condition_graph_ix = self.cfg.new_basic_block_normal(); + /* cfg */ + + self.record_ast_nodes(); self.visit_expression(&stmt.test); + let test_node = self.retrieve_recorded_ast_nodes().into_iter().next(); + self.cfg.append_condition_to(start_of_condition_graph_ix, test_node); /* cfg */ - let before_if_stmt_graph_ix = self.cfg.current_node_ix; + let after_test_graph_ix = self.cfg.current_node_ix; let before_consequent_stmt_graph_ix = self.cfg.new_basic_block_normal(); /* cfg */ @@ -944,13 +969,11 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> { /* cfg - bb after if statement joins consequent and alternate */ let after_if_graph_ix = self.cfg.new_basic_block_normal(); + self.cfg.add_edge(before_if_stmt_graph_ix, start_of_condition_graph_ix, EdgeType::Normal); + self.cfg.add_edge(after_consequent_stmt_graph_ix, after_if_graph_ix, EdgeType::Normal); - self.cfg.add_edge( - before_if_stmt_graph_ix, - before_consequent_stmt_graph_ix, - EdgeType::Normal, - ); + self.cfg.add_edge(after_test_graph_ix, before_consequent_stmt_graph_ix, EdgeType::Jump); if let Some((start_of_alternate_stmt_graph_ix, after_alternate_stmt_graph_ix)) = else_graph_ix @@ -1105,13 +1128,16 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> { self.enter_node(kind); if let Some(expr) = &case.test { + self.record_ast_nodes(); self.visit_expression(expr); + let test_node = self.retrieve_recorded_ast_nodes().into_iter().next(); + self.cfg.append_condition_to(self.cfg.current_node_ix, test_node); } /* 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(after_test_graph_ix, statements_in_switch_graph_ix, EdgeType::Normal); + self.cfg.add_edge(after_test_graph_ix, statements_in_switch_graph_ix, EdgeType::Jump); /* cfg */ self.visit_statements(&case.consequent); @@ -1274,7 +1300,10 @@ impl<'a> Visit<'a> for SemanticBuilder<'a> { let condition_graph_ix = self.cfg.new_basic_block_normal(); /* cfg */ + self.record_ast_nodes(); self.visit_expression(&stmt.test); + let test_node = self.retrieve_recorded_ast_nodes().into_iter().next(); + self.cfg.append_condition_to(condition_graph_ix, test_node); /* cfg - body basic block */ let body_graph_ix = self.cfg.new_basic_block_normal(); diff --git a/crates/oxc_semantic/src/control_flow/builder/mod.rs b/crates/oxc_semantic/src/control_flow/builder/mod.rs index 35404c17521e23..6573649df7ba27 100644 --- a/crates/oxc_semantic/src/control_flow/builder/mod.rs +++ b/crates/oxc_semantic/src/control_flow/builder/mod.rs @@ -52,11 +52,15 @@ impl<'a> ControlFlowGraphBuilder<'a> { ControlFlowGraph { graph: self.graph, basic_blocks: self.basic_blocks } } - /// # Panics pub fn current_basic_block(&mut self) -> &mut BasicBlock { + self.basic_block_mut(self.current_node_ix) + } + + /// # Panics + pub fn basic_block_mut(&mut self, basic_block: BasicBlockId) -> &mut BasicBlock { let idx = *self .graph - .node_weight(self.current_node_ix) + .node_weight(basic_block) .expect("expected `self.current_node_ix` to be a valid node index in self.graph"); self.basic_blocks .get_mut(idx) @@ -145,6 +149,10 @@ impl<'a> ControlFlowGraphBuilder<'a> { ); } + pub fn append_condition_to(&mut self, block: BasicBlockId, node: Option) { + self.push_instruction_to(block, InstructionKind::Condition, node); + } + pub fn append_iteration(&mut self, node: Option, kind: IterationInstructionKind) { self.push_instruction(InstructionKind::Iteration(kind), node); } @@ -195,7 +203,17 @@ impl<'a> ControlFlowGraphBuilder<'a> { #[inline] pub(self) fn push_instruction(&mut self, kind: InstructionKind, node_id: Option) { - self.current_basic_block().instructions.push(Instruction { kind, node_id }); + self.push_instruction_to(self.current_node_ix, kind, node_id); + } + + #[inline] + pub(self) fn push_instruction_to( + &mut self, + block: BasicBlockId, + kind: InstructionKind, + node_id: Option, + ) { + self.basic_block_mut(block).instructions.push(Instruction { kind, node_id }); } #[must_use] diff --git a/crates/oxc_semantic/src/control_flow/dot.rs b/crates/oxc_semantic/src/control_flow/dot.rs index f3497380e0f0f0..a7d7176fb95340 100644 --- a/crates/oxc_semantic/src/control_flow/dot.rs +++ b/crates/oxc_semantic/src/control_flow/dot.rs @@ -27,6 +27,17 @@ impl<'a, 'b> DebugDotContext<'a, 'b> { fn debug_ast_kind(self, id: AstNodeId) -> String { self.0.kind(id).debug_name().into_owned() } + + fn try_eval_literal(self, id: AstNodeId) -> Option { + match self.0.kind(id) { + AstKind::NumericLiteral(lit) => Some(lit.value.to_string()), + AstKind::BooleanLiteral(lit) => Some(lit.value.to_string()), + AstKind::StringLiteral(lit) => Some(lit.value.to_string()), + AstKind::BigintLiteral(lit) => Some(lit.raw.to_string()), + AstKind::NullLiteral(_) => Some("null".to_string()), + _ => None, + } + } } impl<'a, 'b> From<&'b AstNodes<'a>> for DebugDotContext<'a, 'b> { @@ -76,12 +87,13 @@ impl DisplayDot for Instruction { InstructionKind::Statement => "statement", InstructionKind::Unreachable => "unreachable", InstructionKind::Throw => "throw", + InstructionKind::Condition => "condition", + InstructionKind::Iteration(IterationInstructionKind::Of) => "iteration ", + InstructionKind::Iteration(IterationInstructionKind::In) => "iteration ", InstructionKind::Break(LabeledInstruction::Labeled) => "break