From 6c7c824767e2e9c992da74e449da27889bbd4bb8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dorian=20P=C3=A9ron?= <peron@adacore.com>
Date: Fri, 3 May 2024 09:56:13 +0000
Subject: [PATCH] coverage: Make MCDC take in account last RHS of
 condition-coverage

Condition coverage extends branch coverage to treat the specific case
of last operands of boolean decisions not involved in control flow.
This is ultimately made for MCDC to be exhaustive on all boolean expressions.

This patch adds a call to `visit_branch_coverage_operation` to track the
top-level operand of the said decisions, and changes
`visit_coverage_standalone_condition` so MCDC branch registration is called
when enabled on these _last RHS_ cases.
---
 .../rustc_mir_build/src/build/coverageinfo.rs | 66 ++++++++++++-------
 .../rustc_mir_build/src/build/expr/into.rs    |  3 +
 2 files changed, 47 insertions(+), 22 deletions(-)

diff --git a/compiler/rustc_mir_build/src/build/coverageinfo.rs b/compiler/rustc_mir_build/src/build/coverageinfo.rs
index 855dcbbcb34..876faca5172 100644
--- a/compiler/rustc_mir_build/src/build/coverageinfo.rs
+++ b/compiler/rustc_mir_build/src/build/coverageinfo.rs
@@ -118,17 +118,35 @@ impl BranchInfoBuilder {
         }
     }
 
-    fn add_two_way_branch<'tcx>(
+    fn register_two_way_branch<'tcx>(
         &mut self,
+        tcx: TyCtxt<'tcx>,
         cfg: &mut CFG<'tcx>,
         source_info: SourceInfo,
         true_block: BasicBlock,
         false_block: BasicBlock,
     ) {
-        let true_marker = self.markers.inject_block_marker(cfg, source_info, true_block);
-        let false_marker = self.markers.inject_block_marker(cfg, source_info, false_block);
+        // Separate path for handling branches when MC/DC is enabled.
+        if let Some(mcdc_info) = self.mcdc_info.as_mut() {
+            let inject_block_marker =
+                |source_info, block| self.markers.inject_block_marker(cfg, source_info, block);
+            mcdc_info.visit_evaluated_condition(
+                tcx,
+                source_info,
+                true_block,
+                false_block,
+                inject_block_marker,
+            );
+        } else {
+            let true_marker = self.markers.inject_block_marker(cfg, source_info, true_block);
+            let false_marker = self.markers.inject_block_marker(cfg, source_info, false_block);
 
-        self.branch_spans.push(BranchSpan { span: source_info.span, true_marker, false_marker });
+            self.branch_spans.push(BranchSpan {
+                span: source_info.span,
+                true_marker,
+                false_marker,
+            });
+        }
     }
 
     pub(crate) fn into_done(self) -> Option<Box<mir::coverage::BranchInfo>> {
@@ -205,7 +223,14 @@ impl<'tcx> Builder<'_, 'tcx> {
             mir::TerminatorKind::if_(mir::Operand::Copy(place), true_block, false_block),
         );
 
-        branch_info.add_two_way_branch(&mut self.cfg, source_info, true_block, false_block);
+        // Separate path for handling branches when MC/DC is enabled.
+        branch_info.register_two_way_branch(
+            self.tcx,
+            &mut self.cfg,
+            source_info,
+            true_block,
+            false_block,
+        );
 
         let join_block = self.cfg.start_new_block();
         self.cfg.goto(true_block, source_info, join_block);
@@ -236,22 +261,13 @@ impl<'tcx> Builder<'_, 'tcx> {
 
         let source_info = SourceInfo { span: self.thir[expr_id].span, scope: self.source_scope };
 
-        // Separate path for handling branches when MC/DC is enabled.
-        if let Some(mcdc_info) = branch_info.mcdc_info.as_mut() {
-            let inject_block_marker = |source_info, block| {
-                branch_info.markers.inject_block_marker(&mut self.cfg, source_info, block)
-            };
-            mcdc_info.visit_evaluated_condition(
-                self.tcx,
-                source_info,
-                then_block,
-                else_block,
-                inject_block_marker,
-            );
-            return;
-        }
-
-        branch_info.add_two_way_branch(&mut self.cfg, source_info, then_block, else_block);
+        branch_info.register_two_way_branch(
+            self.tcx,
+            &mut self.cfg,
+            source_info,
+            then_block,
+            else_block,
+        );
     }
 
     /// If branch coverage is enabled, inject marker statements into `true_block`
@@ -270,6 +286,12 @@ impl<'tcx> Builder<'_, 'tcx> {
         // FIXME(#124144) This may need special handling when MC/DC is enabled.
 
         let source_info = SourceInfo { span: pattern.span, scope: self.source_scope };
-        branch_info.add_two_way_branch(&mut self.cfg, source_info, true_block, false_block);
+        branch_info.register_two_way_branch(
+            self.tcx,
+            &mut self.cfg,
+            source_info,
+            true_block,
+            false_block,
+        );
     }
 }
diff --git a/compiler/rustc_mir_build/src/build/expr/into.rs b/compiler/rustc_mir_build/src/build/expr/into.rs
index c08a3b6691a..6b4dd8b64a4 100644
--- a/compiler/rustc_mir_build/src/build/expr/into.rs
+++ b/compiler/rustc_mir_build/src/build/expr/into.rs
@@ -150,6 +150,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
             ExprKind::LogicalOp { op, lhs, rhs } => {
                 let condition_scope = this.local_scope();
                 let source_info = this.source_info(expr.span);
+
+                this.visit_coverage_branch_operation(op, expr.span);
+
                 // We first evaluate the left-hand side of the predicate ...
                 let (then_block, else_block) =
                     this.in_if_then_scope(condition_scope, expr.span, |this| {