Drop branching blocks with same span as expanded macro

Fixes: #84561

compiler/rustc_mir/src/transform/coverage/spans.rs

@@ -11,7 +11,7 @@ use rustc_middle::mir::{
 use rustc_middle::ty::TyCtxt;
 
 use rustc_span::source_map::original_sp;
-use rustc_span::{BytePos, Span};
+use rustc_span::{BytePos, ExpnKind, MacroKind, Span};
 
 use std::cmp::Ordering;
 
@@ -67,19 +67,30 @@ impl CoverageStatement {
 #[derive(Debug, Clone)]
 pub(super) struct CoverageSpan {
     pub span: Span,
+    pub is_macro_expansion: bool,
     pub bcb: BasicCoverageBlock,
     pub coverage_statements: Vec<CoverageStatement>,
     pub is_closure: bool,
 }
 
 impl CoverageSpan {
     pub fn for_fn_sig(fn_sig_span: Span) -> Self {
-        Self { span: fn_sig_span, bcb: START_BCB, coverage_statements: vec![], is_closure: false }
+        // Whether the function signature is from a macro or not, it should not be treated like
+        // macro-expanded statements and terminators.
+        let is_macro_expansion = false;
+        Self {
+            span: fn_sig_span,
+            is_macro_expansion,
+            bcb: START_BCB,
+            coverage_statements: vec![],
+            is_closure: false,
+        }
     }
 
     pub fn for_statement(
         statement: &Statement<'tcx>,
         span: Span,
+        is_macro_expansion: bool,
         bcb: BasicCoverageBlock,
         bb: BasicBlock,
         stmt_index: usize,
@@ -94,15 +105,22 @@ impl CoverageSpan {
 
         Self {
             span,
+            is_macro_expansion,
             bcb,
             coverage_statements: vec![CoverageStatement::Statement(bb, span, stmt_index)],
             is_closure,
         }
     }
 
-    pub fn for_terminator(span: Span, bcb: BasicCoverageBlock, bb: BasicBlock) -> Self {
+    pub fn for_terminator(
+        span: Span,
+        is_macro_expansion: bool,
+        bcb: BasicCoverageBlock,
+        bb: BasicBlock,
+    ) -> Self {
         Self {
             span,
+            is_macro_expansion,
             bcb,
             coverage_statements: vec![CoverageStatement::Terminator(bb, span)],
             is_closure: false,
@@ -344,7 +362,27 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
             } else if self.prev_original_span == self.curr().span {
                 // Note that this compares the new span to `prev_original_span`, which may not
                 // be the full `prev.span` (if merged during the previous iteration).
-                self.hold_pending_dups_unless_dominated();
+                if self.prev().is_macro_expansion && self.curr().is_macro_expansion {
+                    // Macros that expand to include branching (such as
+                    // `assert_eq!()`, `assert_ne!()`, `info!()`, `debug!()`, or
+                    // `trace!()) typically generate callee spans with identical
+                    // ranges (typically the full span of the macro) for all
+                    // `BasicBlocks`. This makes it impossible to distinguish
+                    // the condition (`if val1 != val2`) from the optional
+                    // branched statements (such as the call to `panic!()` on
+                    // assert failure). In this case it is better (or less
+                    // worse) to drop the optional branch bcbs and keep the
+                    // non-conditional statements, to count when reached.
+                    debug!(
+                        "  curr and prev are part of a macro expansion, and curr has the same span \
+                        as prev, but is in a different bcb. Drop curr and keep prev for next iter. \
+                        prev={:?}",
+                        self.prev()
+                    );
+                    self.take_curr();
+                } else {
+                    self.hold_pending_dups_unless_dominated();
+                }
             } else {
                 self.cutoff_prev_at_overlapping_curr();
             }
@@ -401,14 +439,24 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
                     .iter()
                     .enumerate()
                     .filter_map(move |(index, statement)| {
-                        filtered_statement_span(statement, self.body_span).map(|span| {
-                            CoverageSpan::for_statement(statement, span, bcb, bb, index)
-                        })
+                        filtered_statement_span(statement, self.body_span).map(
+                            |(span, is_macro_expansion)| {
+                                CoverageSpan::for_statement(
+                                    statement,
+                                    span,
+                                    is_macro_expansion,
+                                    bcb,
+                                    bb,
+                                    index,
+                                )
+                            },
+                        )
                     })
-                    .chain(
-                        filtered_terminator_span(data.terminator(), self.body_span)
-                            .map(|span| CoverageSpan::for_terminator(span, bcb, bb)),
-                    )
+                    .chain(filtered_terminator_span(data.terminator(), self.body_span).map(
+                        |(span, is_macro_expansion)| {
+                            CoverageSpan::for_terminator(span, is_macro_expansion, bcb, bb)
+                        },
+                    ))
             })
             .collect()
     }
@@ -656,7 +704,7 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
 pub(super) fn filtered_statement_span(
     statement: &'a Statement<'tcx>,
     body_span: Span,
-) -> Option<Span> {
+) -> Option<(Span, bool)> {
     match statement.kind {
         // These statements have spans that are often outside the scope of the executed source code
         // for their parent `BasicBlock`.
@@ -701,7 +749,7 @@ pub(super) fn filtered_statement_span(
 pub(super) fn filtered_terminator_span(
     terminator: &'a Terminator<'tcx>,
     body_span: Span,
-) -> Option<Span> {
+) -> Option<(Span, bool)> {
     match terminator.kind {
         // These terminators have spans that don't positively contribute to computing a reasonable
         // span of actually executed source code. (For example, SwitchInt terminators extracted from
@@ -732,7 +780,13 @@ pub(super) fn filtered_terminator_span(
 }
 
 #[inline]
-fn function_source_span(span: Span, body_span: Span) -> Span {
+fn function_source_span(span: Span, body_span: Span) -> (Span, bool) {
+    let is_macro_expansion = span.ctxt() != body_span.ctxt()
+        && if let ExpnKind::Macro(MacroKind::Bang, _) = span.ctxt().outer_expn_data().kind {
+            true
+        } else {
+            false
+        };
     let span = original_sp(span, body_span).with_ctxt(body_span.ctxt());
-    if body_span.contains(span) { span } else { body_span }
+    (if body_span.contains(span) { span } else { body_span }, is_macro_expansion)
 }

compiler/rustc_mir/src/transform/coverage/tests.rs

@@ -1,6 +1,10 @@
 //! This crate hosts a selection of "unit tests" for components of the `InstrumentCoverage` MIR
 //! pass.
 //!
+//! ```shell
+//! ./x.py test --keep-stage 1 compiler/rustc_mir --test-args '--show-output coverage'
+//! ```
+//!
 //! The tests construct a few "mock" objects, as needed, to support the `InstrumentCoverage`
 //! functions and algorithms. Mocked objects include instances of `mir::Body`; including
 //! `Terminator`s of various `kind`s, and `Span` objects. Some functions used by or used on
@@ -679,10 +683,15 @@ fn test_make_bcb_counters() {
         let mut basic_coverage_blocks = graph::CoverageGraph::from_mir(&mir_body);
         let mut coverage_spans = Vec::new();
         for (bcb, data) in basic_coverage_blocks.iter_enumerated() {
-            if let Some(span) =
+            if let Some((span, is_macro_expansion)) =
                 spans::filtered_terminator_span(data.terminator(&mir_body), body_span)
             {
-                coverage_spans.push(spans::CoverageSpan::for_terminator(span, bcb, data.last_bb()));
+                coverage_spans.push(spans::CoverageSpan::for_terminator(
+                    span,
+                    is_macro_expansion,
+                    bcb,
+                    data.last_bb(),
+                ));
             }
         }
         let mut coverage_counters = counters::CoverageCounters::new(0);

src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-84561.txt

@@ -0,0 +1,102 @@
+    1|       |// FIXME(#84561): function-like macros produce unintuitive coverage results.
+    2|       |// This test demonstrates some of the problems.
+    3|       |
+    4|     18|#[derive(Debug, PartialEq, Eq)]
+                       ^5                ^0
+  ------------------
+  | <issue_84561::Foo as core::cmp::PartialEq>::eq:
+  |    4|     18|#[derive(Debug, PartialEq, Eq)]
+  ------------------
+  | Unexecuted instantiation: <issue_84561::Foo as core::cmp::PartialEq>::ne
+  ------------------
+    5|       |struct Foo(u32);
+    6|       |
+    7|      1|fn main() {
+    8|      1|    let bar = Foo(1);
+    9|      1|    assert_eq!(bar, Foo(1));
+   10|      1|    let baz = Foo(0);
+   11|      1|    assert_ne!(baz, Foo(1));
+   12|      1|    println!("{:?}", Foo(1));
+   13|      1|    println!("{:?}", bar);
+   14|      1|    println!("{:?}", baz);
+   15|      1|
+   16|      1|    assert_eq!(Foo(1), Foo(1));
+   17|      1|    assert_ne!(Foo(0), Foo(1));
+   18|      1|    assert_eq!(Foo(2), Foo(2));
+   19|      1|    let bar = Foo(1);
+   20|      1|    assert_ne!(Foo(0), Foo(3));
+   21|      1|    assert_ne!(Foo(0), Foo(4));
+   22|      1|    assert_eq!(Foo(3), Foo(3));
+   23|      1|    assert_ne!(Foo(0), Foo(5));
+   24|      1|    println!("{:?}", bar);
+   25|      1|    println!("{:?}", Foo(1));
+   26|      1|
+   27|      1|    let is_true = std::env::args().len() == 1;
+   28|      1|
+   29|      1|    assert_eq!(
+   30|      1|        Foo(1),
+   31|      1|        Foo(1)
+   32|      1|    );
+   33|      1|    assert_ne!(
+   34|      1|        Foo(0),
+   35|      1|        Foo(1)
+   36|      1|    );
+   37|      1|    assert_eq!(
+   38|      1|        Foo(2),
+   39|      1|        Foo(2)
+   40|      1|    );
+   41|      1|    let bar = Foo(1
+   42|      1|    );
+   43|      1|    assert_ne!(
+   44|      1|        Foo(0),
+   45|      1|        Foo(3)
+   46|      1|    );
+   47|      1|    if is_true {
+   48|      1|        assert_ne!(
+   49|      1|            Foo(0),
+   50|      1|            Foo(4)
+   51|      1|        );
+   52|       |    } else {
+   53|      0|        assert_eq!(
+   54|      0|            Foo(3),
+   55|      0|            Foo(3)
+   56|      0|        );
+   57|       |    }
+   58|       |    assert_ne!(
+   59|      1|        if is_true {
+   60|      1|            Foo(0)
+   61|       |        } else {
+   62|      0|            Foo(1)
+   63|       |        },
+   64|       |        Foo(5)
+   65|       |    );
+   66|      1|    assert_ne!(
+   67|      1|        Foo(5),
+   68|      1|        if is_true {
+   69|      1|            Foo(0)
+   70|       |        } else {
+   71|      0|            Foo(1)
+   72|       |        }
+   73|       |    );
+   74|       |    assert_ne!(
+   75|      1|        if is_true {
+   76|      1|            assert_eq!(
+   77|      1|                Foo(3),
+   78|      1|                Foo(3)
+   79|      1|            );
+   80|      1|            Foo(0)
+   81|       |        } else {
+   82|       |            assert_ne!(
+   83|      0|                if is_true {
+   84|      0|                    Foo(0)
+   85|       |                } else {
+   86|      0|                    Foo(1)
+   87|       |                },
+   88|       |                Foo(5)
+   89|       |            );
+   90|      0|            Foo(1)
+   91|       |        },
+   92|       |        Foo(5)
+   93|       |    );
+   94|      1|}
+

src/test/run-make-fulldeps/coverage/issue-84561.rs

@@ -0,0 +1,94 @@
+// FIXME(#84561): function-like macros produce unintuitive coverage results.
+// This test demonstrates some of the problems.
+
+#[derive(Debug, PartialEq, Eq)]
+struct Foo(u32);
+
+fn main() {
+    let bar = Foo(1);
+    assert_eq!(bar, Foo(1));
+    let baz = Foo(0);
+    assert_ne!(baz, Foo(1));
+    println!("{:?}", Foo(1));
+    println!("{:?}", bar);
+    println!("{:?}", baz);
+
+    assert_eq!(Foo(1), Foo(1));
+    assert_ne!(Foo(0), Foo(1));
+    assert_eq!(Foo(2), Foo(2));
+    let bar = Foo(1);
+    assert_ne!(Foo(0), Foo(3));
+    assert_ne!(Foo(0), Foo(4));
+    assert_eq!(Foo(3), Foo(3));
+    assert_ne!(Foo(0), Foo(5));
+    println!("{:?}", bar);
+    println!("{:?}", Foo(1));
+
+    let is_true = std::env::args().len() == 1;
+
+    assert_eq!(
+        Foo(1),
+        Foo(1)
+    );
+    assert_ne!(
+        Foo(0),
+        Foo(1)
+    );
+    assert_eq!(
+        Foo(2),
+        Foo(2)
+    );
+    let bar = Foo(1
+    );
+    assert_ne!(
+        Foo(0),
+        Foo(3)
+    );
+    if is_true {
+        assert_ne!(
+            Foo(0),
+            Foo(4)
+        );
+    } else {
+        assert_eq!(
+            Foo(3),
+            Foo(3)
+        );
+    }
+    assert_ne!(
+        if is_true {
+            Foo(0)
+        } else {
+            Foo(1)
+        },
+        Foo(5)
+    );
+    assert_ne!(
+        Foo(5),
+        if is_true {
+            Foo(0)
+        } else {
+            Foo(1)
+        }
+    );
+    assert_ne!(
+        if is_true {
+            assert_eq!(
+                Foo(3),
+                Foo(3)
+            );
+            Foo(0)
+        } else {
+            assert_ne!(
+                if is_true {
+                    Foo(0)
+                } else {
+                    Foo(1)
+                },
+                Foo(5)
+            );
+            Foo(1)
+        },
+        Foo(5)
+    );
+}