From aeb67ad870af2abde47b7be81651aa5a3fec6faa Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Mon, 26 Apr 2021 02:45:46 -0700 Subject: [PATCH 1/4] Drop branching blocks with same span as expanded macro Fixes: #84561 --- .../rustc_mir/src/transform/coverage/spans.rs | 84 ++++++++++++--- .../rustc_mir/src/transform/coverage/tests.rs | 13 ++- .../expected_show_coverage.issue-84561.txt | 102 ++++++++++++++++++ .../run-make-fulldeps/coverage/issue-84561.rs | 94 ++++++++++++++++ 4 files changed, 276 insertions(+), 17 deletions(-) create mode 100644 src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-84561.txt create mode 100644 src/test/run-make-fulldeps/coverage/issue-84561.rs diff --git a/compiler/rustc_mir/src/transform/coverage/spans.rs b/compiler/rustc_mir/src/transform/coverage/spans.rs index 249f5e835cd78..8dd6f7402fee7 100644 --- a/compiler/rustc_mir/src/transform/coverage/spans.rs +++ b/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,6 +67,7 @@ impl CoverageStatement { #[derive(Debug, Clone)] pub(super) struct CoverageSpan { pub span: Span, + pub is_macro_expansion: bool, pub bcb: BasicCoverageBlock, pub coverage_statements: Vec, pub is_closure: bool, @@ -74,12 +75,22 @@ pub(super) struct CoverageSpan { 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 { +) -> 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 { +) -> 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) } diff --git a/compiler/rustc_mir/src/transform/coverage/tests.rs b/compiler/rustc_mir/src/transform/coverage/tests.rs index dee112443d337..dbbd677fd638d 100644 --- a/compiler/rustc_mir/src/transform/coverage/tests.rs +++ b/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); diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-84561.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-84561.txt new file mode 100644 index 0000000000000..34d584f9eae66 --- /dev/null +++ b/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 + ------------------ + | ::eq: + | 4| 18|#[derive(Debug, PartialEq, Eq)] + ------------------ + | Unexecuted instantiation: ::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|} + diff --git a/src/test/run-make-fulldeps/coverage/issue-84561.rs b/src/test/run-make-fulldeps/coverage/issue-84561.rs new file mode 100644 index 0000000000000..a5a0e1dc7581d --- /dev/null +++ b/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) + ); +} From 2c4fc3e8f4e3886d1c3b9cb437e428cc4e3d2113 Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Mon, 26 Apr 2021 16:27:54 -0700 Subject: [PATCH 2/4] More improvements to macro coverage --- .../rustc_mir/src/transform/coverage/spans.rs | 159 ++++++++---- .../rustc_mir/src/transform/coverage/tests.rs | 4 +- .../expected_show_coverage.inner_items.txt | 6 +- .../expected_show_coverage.issue-84561.txt | 236 ++++++++++++------ .../expected_show_coverage.uses_crate.txt | 6 +- ...pected_show_coverage.uses_inline_crate.txt | 12 +- .../run-make-fulldeps/coverage/issue-84561.rs | 98 +++++++- 7 files changed, 366 insertions(+), 155 deletions(-) diff --git a/compiler/rustc_mir/src/transform/coverage/spans.rs b/compiler/rustc_mir/src/transform/coverage/spans.rs index 8dd6f7402fee7..97eda1e37e1fa 100644 --- a/compiler/rustc_mir/src/transform/coverage/spans.rs +++ b/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, ExpnKind, MacroKind, Span}; +use rustc_span::{BytePos, ExpnKind, MacroKind, Span, Symbol}; use std::cmp::Ordering; @@ -67,7 +67,7 @@ impl CoverageStatement { #[derive(Debug, Clone)] pub(super) struct CoverageSpan { pub span: Span, - pub is_macro_expansion: bool, + pub expn_span: Span, pub bcb: BasicCoverageBlock, pub coverage_statements: Vec, pub is_closure: bool, @@ -75,12 +75,9 @@ pub(super) struct CoverageSpan { impl CoverageSpan { pub fn for_fn_sig(fn_sig_span: Span) -> Self { - // 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, + expn_span: fn_sig_span, bcb: START_BCB, coverage_statements: vec![], is_closure: false, @@ -90,7 +87,7 @@ impl CoverageSpan { pub fn for_statement( statement: &Statement<'tcx>, span: Span, - is_macro_expansion: bool, + expn_span: Span, bcb: BasicCoverageBlock, bb: BasicBlock, stmt_index: usize, @@ -105,7 +102,7 @@ impl CoverageSpan { Self { span, - is_macro_expansion, + expn_span, bcb, coverage_statements: vec![CoverageStatement::Statement(bb, span, stmt_index)], is_closure, @@ -114,13 +111,13 @@ impl CoverageSpan { pub fn for_terminator( span: Span, - is_macro_expansion: bool, + expn_span: Span, bcb: BasicCoverageBlock, bb: BasicBlock, ) -> Self { Self { span, - is_macro_expansion, + expn_span, bcb, coverage_statements: vec![CoverageStatement::Terminator(bb, span)], is_closure: false, @@ -176,6 +173,34 @@ impl CoverageSpan { .collect::>() .join("\n") } + + /// If the span is part of a macro, and the macro is visible (expands directly to the given + /// body_span), returns the macro name symbol. + pub fn current_macro(&self) -> Option { + if let ExpnKind::Macro(MacroKind::Bang, current_macro) = + self.expn_span.ctxt().outer_expn_data().kind + { + return Some(current_macro); + } + None + } + + /// If the span is part of a macro, and the macro is visible (expands directly to the given + /// body_span), returns the macro name symbol. + pub fn visible_macro(&self, body_span: Span) -> Option { + if let Some(current_macro) = self.current_macro() { + if self.expn_span.parent().unwrap_or_else(|| bug!("macro must have a parent")).ctxt() + == body_span.ctxt() + { + return Some(current_macro); + } + } + None + } + + pub fn is_macro_expansion(&self) -> bool { + self.current_macro().is_some() + } } /// Converts the initial set of `CoverageSpan`s (one per MIR `Statement` or `Terminator`) into a @@ -219,6 +244,9 @@ pub struct CoverageSpans<'a, 'tcx> { /// Assigned from `curr_original_span` from the previous iteration. prev_original_span: Span, + /// A copy of the expn_span from the prior iteration. + prev_expn_span: Option, + /// One or more `CoverageSpan`s with the same `Span` but different `BasicCoverageBlock`s, and /// no `BasicCoverageBlock` in this list dominates another `BasicCoverageBlock` in the list. /// If a new `curr` span also fits this criteria (compared to an existing list of @@ -273,15 +301,13 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { curr_original_span: Span::with_root_ctxt(BytePos(0), BytePos(0)), some_prev: None, prev_original_span: Span::with_root_ctxt(BytePos(0), BytePos(0)), + prev_expn_span: None, pending_dups: Vec::new(), }; let sorted_spans = coverage_spans.mir_to_initial_sorted_coverage_spans(); coverage_spans.sorted_spans_iter = Some(sorted_spans.into_iter()); - coverage_spans.some_prev = coverage_spans.sorted_spans_iter.as_mut().unwrap().next(); - coverage_spans.prev_original_span = - coverage_spans.some_prev.as_ref().expect("at least one span").span; coverage_spans.to_refined_spans() } @@ -335,10 +361,14 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { /// de-duplicated `CoverageSpan`s. fn to_refined_spans(mut self) -> Vec { while self.next_coverage_span() { - if self.curr().is_mergeable(self.prev()) { + if self.some_prev.is_none() { + debug!(" initial span"); + self.check_invoked_macro_name_span(); + } else if self.curr().is_mergeable(self.prev()) { debug!(" same bcb (and neither is a closure), merge with prev={:?}", self.prev()); let prev = self.take_prev(); self.curr_mut().merge_from(prev); + self.check_invoked_macro_name_span(); // Note that curr.span may now differ from curr_original_span } else if self.prev_ends_before_curr() { debug!( @@ -347,7 +377,8 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { self.prev() ); let prev = self.take_prev(); - self.refined_spans.push(prev); + self.push_refined_span(prev); + self.check_invoked_macro_name_span(); } else if self.prev().is_closure { // drop any equal or overlapping span (`curr`) and keep `prev` to test again in the // next iter @@ -362,7 +393,7 @@ 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). - if self.prev().is_macro_expansion && self.curr().is_macro_expansion { + 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 @@ -385,15 +416,16 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { } } else { self.cutoff_prev_at_overlapping_curr(); + self.check_invoked_macro_name_span(); } } debug!(" AT END, adding last prev={:?}", self.prev()); let prev = self.take_prev(); - let CoverageSpans { pending_dups, mut refined_spans, .. } = self; + let pending_dups = self.pending_dups.split_off(0); for dup in pending_dups { debug!(" ...adding at least one pending dup={:?}", dup); - refined_spans.push(dup); + self.push_refined_span(dup); } // Async functions wrap a closure that implements the body to be executed. The enclosing @@ -403,21 +435,60 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { // excluded. The closure's `Return` is the only one that will be counted. This provides // adequate coverage, and more intuitive counts. (Avoids double-counting the closing brace // of the function body.) - let body_ends_with_closure = if let Some(last_covspan) = refined_spans.last() { + let body_ends_with_closure = if let Some(last_covspan) = self.refined_spans.last() { last_covspan.is_closure && last_covspan.span.hi() == self.body_span.hi() } else { false }; if !body_ends_with_closure { - refined_spans.push(prev); + self.push_refined_span(prev); } // Remove `CoverageSpan`s derived from closures, originally added to ensure the coverage // regions for the current function leave room for the closure's own coverage regions // (injected separately, from the closure's own MIR). - refined_spans.retain(|covspan| !covspan.is_closure); - refined_spans + self.refined_spans.retain(|covspan| !covspan.is_closure); + self.refined_spans + } + + fn push_refined_span(&mut self, covspan: CoverageSpan) { + let len = self.refined_spans.len(); + if len > 0 { + let last = &mut self.refined_spans[len - 1]; + if last.is_mergeable(&covspan) { + debug!( + "merging new refined span with last refined span, last={:?}, covspan={:?}", + last, covspan + ); + last.merge_from(covspan); + return; + } + } + self.refined_spans.push(covspan) + } + + fn check_invoked_macro_name_span(&mut self) { + if let Some(visible_macro) = self.curr().visible_macro(self.body_span) { + if self.prev_expn_span.map_or(true, |prev_expn_span| { + self.curr().expn_span.ctxt() != prev_expn_span.ctxt() + }) { + let merged_prefix_len = self.curr_original_span.lo() - self.curr().span.lo(); + let after_macro_bang = merged_prefix_len + + BytePos(visible_macro.to_string().bytes().count() as u32 + 1); + let mut macro_name_cov = self.curr().clone(); + self.curr_mut().span = + self.curr().span.with_lo(self.curr().span.lo() + after_macro_bang); + macro_name_cov.span = + macro_name_cov.span.with_hi(macro_name_cov.span.lo() + after_macro_bang); + debug!( + " and curr starts a new macro expansion, so add a new span just for \ + the macro `{}!`, new span={:?}", + visible_macro, macro_name_cov + ); + self.push_refined_span(macro_name_cov); + } + } } // Generate a set of `CoverageSpan`s from the filtered set of `Statement`s and `Terminator`s of @@ -440,22 +511,15 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { .enumerate() .filter_map(move |(index, statement)| { filtered_statement_span(statement, self.body_span).map( - |(span, is_macro_expansion)| { + |(span, expn_span)| { CoverageSpan::for_statement( - statement, - span, - is_macro_expansion, - bcb, - bb, - index, + statement, span, expn_span, bcb, bb, index, ) }, ) }) .chain(filtered_terminator_span(data.terminator(), self.body_span).map( - |(span, is_macro_expansion)| { - CoverageSpan::for_terminator(span, is_macro_expansion, bcb, bb) - }, + |(span, expn_span)| CoverageSpan::for_terminator(span, expn_span, bcb, bb), )) }) .collect() @@ -509,7 +573,7 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { let pending_dups = self.pending_dups.split_off(0); for dup in pending_dups.into_iter() { debug!(" ...adding at least one pending={:?}", dup); - self.refined_spans.push(dup); + self.push_refined_span(dup); } } else { self.pending_dups.clear(); @@ -521,12 +585,13 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { /// Advance `prev` to `curr` (if any), and `curr` to the next `CoverageSpan` in sorted order. fn next_coverage_span(&mut self) -> bool { if let Some(curr) = self.some_curr.take() { + self.prev_expn_span = Some(curr.expn_span); self.some_prev = Some(curr); self.prev_original_span = self.curr_original_span; } while let Some(curr) = self.sorted_spans_iter.as_mut().unwrap().next() { debug!("FOR curr={:?}", curr); - if self.prev_starts_after_next(&curr) { + if self.some_prev.is_some() && self.prev_starts_after_next(&curr) { debug!( " prev.span starts after curr.span, so curr will be dropped (skipping past \ closure?); prev={:?}", @@ -583,10 +648,10 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { for mut dup in pending_dups.iter().cloned() { dup.span = dup.span.with_hi(left_cutoff); debug!(" ...and at least one pre_closure dup={:?}", dup); - self.refined_spans.push(dup); + self.push_refined_span(dup); } } - self.refined_spans.push(pre_closure); + self.push_refined_span(pre_closure); } if has_post_closure_span { // Update prev.span to start after the closure (and discard curr) @@ -597,7 +662,7 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { } self.pending_dups.append(&mut pending_dups); let closure_covspan = self.take_curr(); - self.refined_spans.push(closure_covspan); // since self.prev() was already updated + self.push_refined_span(closure_covspan); // since self.prev() was already updated } else { pending_dups.clear(); } @@ -688,7 +753,7 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { } else { debug!(" ... adding modified prev={:?}", self.prev()); let prev = self.take_prev(); - self.refined_spans.push(prev); + self.push_refined_span(prev); } } else { // with `pending_dups`, `prev` cannot have any statements that don't overlap @@ -704,7 +769,7 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { pub(super) fn filtered_statement_span( statement: &'a Statement<'tcx>, body_span: Span, -) -> Option<(Span, bool)> { +) -> Option<(Span, Span)> { match statement.kind { // These statements have spans that are often outside the scope of the executed source code // for their parent `BasicBlock`. @@ -749,7 +814,7 @@ pub(super) fn filtered_statement_span( pub(super) fn filtered_terminator_span( terminator: &'a Terminator<'tcx>, body_span: Span, -) -> Option<(Span, bool)> { +) -> Option<(Span, Span)> { 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 @@ -779,14 +844,10 @@ pub(super) fn filtered_terminator_span( } } +/// Returns the span within the function source body, and the given span, which will be different +/// if the given span is an expansion (macro, syntactic sugar, etc.). #[inline] -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 }, is_macro_expansion) +fn function_source_span(span: Span, body_span: Span) -> (Span, Span) { + let original_span = original_sp(span, body_span).with_ctxt(body_span.ctxt()); + (if body_span.contains(original_span) { original_span } else { body_span }, span) } diff --git a/compiler/rustc_mir/src/transform/coverage/tests.rs b/compiler/rustc_mir/src/transform/coverage/tests.rs index dbbd677fd638d..9b84173c8a293 100644 --- a/compiler/rustc_mir/src/transform/coverage/tests.rs +++ b/compiler/rustc_mir/src/transform/coverage/tests.rs @@ -683,12 +683,12 @@ 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, is_macro_expansion)) = + if let Some((span, expn_span)) = spans::filtered_terminator_span(data.terminator(&mir_body), body_span) { coverage_spans.push(spans::CoverageSpan::for_terminator( span, - is_macro_expansion, + expn_span, bcb, data.last_bb(), )); diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.inner_items.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.inner_items.txt index f5b5184044f65..883254a09ba7d 100644 --- a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.inner_items.txt +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.inner_items.txt @@ -1,9 +1,9 @@ 1| |#![allow(unused_assignments, unused_variables, dead_code)] 2| | 3| 1|fn main() { - 4| | // Initialize test constants in a way that cannot be determined at compile time, to ensure - 5| | // rustc and LLVM cannot optimize out statements (or coverage counters) downstream from - 6| | // dependent conditions. + 4| 1| // Initialize test constants in a way that cannot be determined at compile time, to ensure + 5| 1| // rustc and LLVM cannot optimize out statements (or coverage counters) downstream from + 6| 1| // dependent conditions. 7| 1| let is_true = std::env::args().len() == 1; 8| 1| 9| 1| let mut countdown = 0; diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-84561.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-84561.txt index 34d584f9eae66..5d266b5db15f2 100644 --- a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-84561.txt +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-84561.txt @@ -1,17 +1,17 @@ - 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 + 1| |// This demonstrated Issue #84561: function-like macros produce unintuitive coverage results. + 2| | + 3| |// expect-exit-status-101 + 4| 21|#[derive(PartialEq, Eq)] + ^0 ------------------ | ::eq: - | 4| 18|#[derive(Debug, PartialEq, Eq)] + | 4| 21|#[derive(PartialEq, Eq)] ------------------ | Unexecuted instantiation: ::ne ------------------ 5| |struct Foo(u32); - 6| | - 7| 1|fn main() { + 6| 1|fn test2() { + 7| 1| let is_true = std::env::args().len() == 1; 8| 1| let bar = Foo(1); 9| 1| assert_eq!(bar, Foo(1)); 10| 1| let baz = Foo(0); @@ -23,80 +23,158 @@ 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)); + 19| 1| let bar = Foo(0); + 20| 1| assert_ne!(bar, 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| | ); + 22| 1| assert_eq!(Foo(3), Foo(3), "with a message"); + ^0 + 23| 1| println!("{:?}", bar); + 24| 1| println!("{:?}", Foo(1)); + 25| 1| + 26| 1| assert_ne!(Foo(0), Foo(5), "{}", if is_true { "true message" } else { "false message" }); + ^0 ^0 ^0 + 27| 1| assert_ne!( + 28| | Foo(0) + 29| | , + 30| | Foo(5) + 31| | , + 32| 0| "{}" + 33| 0| , + 34| 0| if + 35| 0| is_true + 36| | { + 37| 0| "true message" + 38| | } else { + 39| 0| "false message" + 40| | } + 41| | ); + 42| | + 43| 1| let is_true = std::env::args().len() == 1; + 44| 1| + 45| 1| assert_eq!( + 46| 1| Foo(1), + 47| 1| Foo(1) + 48| 1| ); + 49| 1| assert_ne!( + 50| 1| Foo(0), + 51| 1| Foo(1) + 52| 1| ); + 53| 1| assert_eq!( + 54| 1| Foo(2), + 55| 1| Foo(2) + 56| 1| ); + 57| 1| let bar = Foo(1); + 58| 1| assert_ne!( + 59| 1| bar, + 60| 1| Foo(3) + 61| 1| ); + 62| 1| if is_true { + 63| 1| assert_ne!( + 64| 1| Foo(0), + 65| 1| Foo(4) + 66| 1| ); + 67| | } else { + 68| 0| assert_eq!( + 69| 0| Foo(3), + 70| 0| Foo(3) + 71| 0| ); + 72| | } + 73| 1| if is_true { + 74| 1| assert_ne!( + 75| | Foo(0), + 76| | Foo(4), + 77| 0| "with a message" + 78| | ); + 79| | } else { + 80| 0| assert_eq!( + 81| | Foo(3), + 82| | Foo(3), + 83| 0| "with a message" + 84| | ); + 85| | } + 86| 1| assert_ne!( + 87| 1| if is_true { + 88| 1| Foo(0) + 89| | } else { 90| 0| Foo(1) 91| | }, 92| | Foo(5) 93| | ); - 94| 1|} + 94| 1| assert_ne!( + 95| 1| Foo(5), + 96| 1| if is_true { + 97| 1| Foo(0) + 98| | } else { + 99| 0| Foo(1) + 100| | } + 101| | ); + 102| 1| assert_ne!( + 103| 1| if is_true { + 104| 1| assert_eq!( + 105| 1| Foo(3), + 106| 1| Foo(3) + 107| 1| ); + 108| 1| Foo(0) + 109| | } else { + 110| 0| assert_ne!( + 111| 0| if is_true { + 112| 0| Foo(0) + 113| | } else { + 114| 0| Foo(1) + 115| | }, + 116| | Foo(5) + 117| | ); + 118| 0| Foo(1) + 119| | }, + 120| | Foo(5), + 121| 0| "with a message" + 122| | ); + 123| 1| assert_eq!( + 124| | Foo(1), + 125| | Foo(3), + 126| 1| "this assert should fail" + 127| | ); + 128| 0| assert_eq!( + 129| | Foo(3), + 130| | Foo(3), + 131| 0| "this assert should not be reached" + 132| | ); + 133| 0|} + 134| | + 135| |impl std::fmt::Debug for Foo { + 136| | fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + 137| 7| write!(f, "try and succeed")?; + ^0 + 138| 7| Ok(()) + 139| 7| } + 140| |} + 141| | + 142| |static mut DEBUG_LEVEL_ENABLED: bool = false; + 143| | + 144| |macro_rules! debug { + 145| | ($($arg:tt)+) => ( + 146| | if unsafe { DEBUG_LEVEL_ENABLED } { + 147| | println!($($arg)+); + 148| | } + 149| | ); + 150| |} + 151| | + 152| 1|fn test1() { + 153| 1| debug!("debug is enabled"); + ^0 + 154| 1| debug!("debug is enabled"); + ^0 + 155| 1| let _ = 0; + 156| 1| debug!("debug is enabled"); + ^0 + 157| 1| unsafe { + 158| 1| DEBUG_LEVEL_ENABLED = true; + 159| 1| } + 160| 1| debug!("debug is enabled"); + 161| 1|} + 162| | + 163| 1|fn main() { + 164| 1| test1(); + 165| 1| test2(); + 166| 1|} diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_crate.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_crate.txt index f5beb9ef24a0e..c2d5143a61816 100644 --- a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_crate.txt +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_crate.txt @@ -3,9 +3,9 @@ 3| |use std::fmt::Debug; 4| | 5| 1|pub fn used_function() { - 6| | // Initialize test constants in a way that cannot be determined at compile time, to ensure - 7| | // rustc and LLVM cannot optimize out statements (or coverage counters) downstream from - 8| | // dependent conditions. + 6| 1| // Initialize test constants in a way that cannot be determined at compile time, to ensure + 7| 1| // rustc and LLVM cannot optimize out statements (or coverage counters) downstream from + 8| 1| // dependent conditions. 9| 1| let is_true = std::env::args().len() == 1; 10| 1| let mut countdown = 0; 11| 1| if is_true { diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_inline_crate.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_inline_crate.txt index cc98956e3073a..dab31cbf4ac9e 100644 --- a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_inline_crate.txt +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_inline_crate.txt @@ -5,9 +5,9 @@ 5| |use std::fmt::Debug; 6| | 7| 1|pub fn used_function() { - 8| | // Initialize test constants in a way that cannot be determined at compile time, to ensure - 9| | // rustc and LLVM cannot optimize out statements (or coverage counters) downstream from - 10| | // dependent conditions. + 8| 1| // Initialize test constants in a way that cannot be determined at compile time, to ensure + 9| 1| // rustc and LLVM cannot optimize out statements (or coverage counters) downstream from + 10| 1| // dependent conditions. 11| 1| let is_true = std::env::args().len() == 1; 12| 1| let mut countdown = 0; 13| 1| if is_true { @@ -19,9 +19,9 @@ 18| | 19| |#[inline(always)] 20| 1|pub fn used_inline_function() { - 21| | // Initialize test constants in a way that cannot be determined at compile time, to ensure - 22| | // rustc and LLVM cannot optimize out statements (or coverage counters) downstream from - 23| | // dependent conditions. + 21| 1| // Initialize test constants in a way that cannot be determined at compile time, to ensure + 22| 1| // rustc and LLVM cannot optimize out statements (or coverage counters) downstream from + 23| 1| // dependent conditions. 24| 1| let is_true = std::env::args().len() == 1; 25| 1| let mut countdown = 0; 26| 1| if is_true { diff --git a/src/test/run-make-fulldeps/coverage/issue-84561.rs b/src/test/run-make-fulldeps/coverage/issue-84561.rs index a5a0e1dc7581d..5c8fd0b7caeaa 100644 --- a/src/test/run-make-fulldeps/coverage/issue-84561.rs +++ b/src/test/run-make-fulldeps/coverage/issue-84561.rs @@ -1,10 +1,10 @@ -// FIXME(#84561): function-like macros produce unintuitive coverage results. -// This test demonstrates some of the problems. +// This demonstrated Issue #84561: function-like macros produce unintuitive coverage results. -#[derive(Debug, PartialEq, Eq)] +// expect-exit-status-101 +#[derive(PartialEq, Eq)] struct Foo(u32); - -fn main() { +fn test2() { + let is_true = std::env::args().len() == 1; let bar = Foo(1); assert_eq!(bar, Foo(1)); let baz = Foo(0); @@ -16,14 +16,30 @@ fn main() { 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)); + let bar = Foo(0); + assert_ne!(bar, Foo(3)); assert_ne!(Foo(0), Foo(4)); - assert_eq!(Foo(3), Foo(3)); - assert_ne!(Foo(0), Foo(5)); + assert_eq!(Foo(3), Foo(3), "with a message"); println!("{:?}", bar); println!("{:?}", Foo(1)); + assert_ne!(Foo(0), Foo(5), "{}", if is_true { "true message" } else { "false message" }); + assert_ne!( + Foo(0) + , + Foo(5) + , + "{}" + , + if + is_true + { + "true message" + } else { + "false message" + } + ); + let is_true = std::env::args().len() == 1; assert_eq!( @@ -38,10 +54,9 @@ fn main() { Foo(2), Foo(2) ); - let bar = Foo(1 - ); + let bar = Foo(1); assert_ne!( - Foo(0), + bar, Foo(3) ); if is_true { @@ -55,6 +70,19 @@ fn main() { Foo(3) ); } + if is_true { + assert_ne!( + Foo(0), + Foo(4), + "with a message" + ); + } else { + assert_eq!( + Foo(3), + Foo(3), + "with a message" + ); + } assert_ne!( if is_true { Foo(0) @@ -89,6 +117,50 @@ fn main() { ); Foo(1) }, - Foo(5) + Foo(5), + "with a message" + ); + assert_eq!( + Foo(1), + Foo(3), + "this assert should fail" + ); + assert_eq!( + Foo(3), + Foo(3), + "this assert should not be reached" + ); +} + +impl std::fmt::Debug for Foo { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "try and succeed")?; + Ok(()) + } +} + +static mut DEBUG_LEVEL_ENABLED: bool = false; + +macro_rules! debug { + ($($arg:tt)+) => ( + if unsafe { DEBUG_LEVEL_ENABLED } { + println!($($arg)+); + } ); } + +fn test1() { + debug!("debug is enabled"); + debug!("debug is enabled"); + let _ = 0; + debug!("debug is enabled"); + unsafe { + DEBUG_LEVEL_ENABLED = true; + } + debug!("debug is enabled"); +} + +fn main() { + test1(); + test2(); +} From bbf6bcee80dcfb566b4d975ff6c3476c0864352e Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Sat, 24 Apr 2021 17:22:29 -0700 Subject: [PATCH 3/4] spanview debug output caused ICE when a function had no body --- compiler/rustc_mir/src/util/spanview.rs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_mir/src/util/spanview.rs b/compiler/rustc_mir/src/util/spanview.rs index a9a30e407b4b0..9abfa4a8dc68b 100644 --- a/compiler/rustc_mir/src/util/spanview.rs +++ b/compiler/rustc_mir/src/util/spanview.rs @@ -99,7 +99,11 @@ where W: Write, { let def_id = body.source.def_id(); - let body_span = hir_body(tcx, def_id).value.span; + let hir_body = hir_body(tcx, def_id); + if hir_body.is_none() { + return Ok(()); + } + let body_span = hir_body.unwrap().value.span; let mut span_viewables = Vec::new(); for (bb, data) in body.basic_blocks().iter_enumerated() { match spanview { @@ -664,19 +668,16 @@ fn fn_span<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> Span { let hir_id = tcx.hir().local_def_id_to_hir_id(def_id.as_local().expect("expected DefId is local")); let fn_decl_span = tcx.hir().span(hir_id); - let body_span = hir_body(tcx, def_id).value.span; - if fn_decl_span.ctxt() == body_span.ctxt() { - fn_decl_span.to(body_span) + if let Some(body_span) = hir_body(tcx, def_id).map(|hir_body| hir_body.value.span) { + if fn_decl_span.ctxt() == body_span.ctxt() { fn_decl_span.to(body_span) } else { body_span } } else { - // This probably occurs for functions defined via macros - body_span + fn_decl_span } } -fn hir_body<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> &'tcx rustc_hir::Body<'tcx> { +fn hir_body<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> Option<&'tcx rustc_hir::Body<'tcx>> { let hir_node = tcx.hir().get_if_local(def_id).expect("expected DefId is local"); - let fn_body_id = hir::map::associated_body(hir_node).expect("HIR node is a function with body"); - tcx.hir().body(fn_body_id) + hir::map::associated_body(hir_node).map(|fn_body_id| tcx.hir().body(fn_body_id)) } fn escape_html(s: &str) -> String { From fd85fd308b536bd42073f58636358ca147822947 Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Tue, 27 Apr 2021 21:45:30 -0700 Subject: [PATCH 4/4] addressed review feedback --- .../rustc_mir/src/transform/coverage/spans.rs | 52 ++++++++++++++----- .../expected_show_coverage.issue-84561.txt | 26 ++++++++-- .../run-make-fulldeps/coverage/issue-84561.rs | 18 ++++++- 3 files changed, 78 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_mir/src/transform/coverage/spans.rs b/compiler/rustc_mir/src/transform/coverage/spans.rs index 97eda1e37e1fa..ed373f03b5a55 100644 --- a/compiler/rustc_mir/src/transform/coverage/spans.rs +++ b/compiler/rustc_mir/src/transform/coverage/spans.rs @@ -13,6 +13,7 @@ use rustc_middle::ty::TyCtxt; use rustc_span::source_map::original_sp; use rustc_span::{BytePos, ExpnKind, MacroKind, Span, Symbol}; +use std::cell::RefCell; use std::cmp::Ordering; #[derive(Debug, Copy, Clone)] @@ -68,6 +69,7 @@ impl CoverageStatement { pub(super) struct CoverageSpan { pub span: Span, pub expn_span: Span, + pub current_macro_or_none: RefCell>>, pub bcb: BasicCoverageBlock, pub coverage_statements: Vec, pub is_closure: bool, @@ -78,6 +80,7 @@ impl CoverageSpan { Self { span: fn_sig_span, expn_span: fn_sig_span, + current_macro_or_none: Default::default(), bcb: START_BCB, coverage_statements: vec![], is_closure: false, @@ -103,6 +106,7 @@ impl CoverageSpan { Self { span, expn_span, + current_macro_or_none: Default::default(), bcb, coverage_statements: vec![CoverageStatement::Statement(bb, span, stmt_index)], is_closure, @@ -118,6 +122,7 @@ impl CoverageSpan { Self { span, expn_span, + current_macro_or_none: Default::default(), bcb, coverage_statements: vec![CoverageStatement::Terminator(bb, span)], is_closure: false, @@ -174,15 +179,19 @@ impl CoverageSpan { .join("\n") } - /// If the span is part of a macro, and the macro is visible (expands directly to the given - /// body_span), returns the macro name symbol. + /// If the span is part of a macro, returns the macro name symbol. pub fn current_macro(&self) -> Option { - if let ExpnKind::Macro(MacroKind::Bang, current_macro) = - self.expn_span.ctxt().outer_expn_data().kind - { - return Some(current_macro); - } - None + self.current_macro_or_none + .borrow_mut() + .get_or_insert_with(|| { + if let ExpnKind::Macro(MacroKind::Bang, current_macro) = + self.expn_span.ctxt().outer_expn_data().kind + { + return Some(current_macro); + } + None + }) + .map(|symbol| symbol) } /// If the span is part of a macro, and the macro is visible (expands directly to the given @@ -474,8 +483,8 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { self.curr().expn_span.ctxt() != prev_expn_span.ctxt() }) { let merged_prefix_len = self.curr_original_span.lo() - self.curr().span.lo(); - let after_macro_bang = merged_prefix_len - + BytePos(visible_macro.to_string().bytes().count() as u32 + 1); + let after_macro_bang = + merged_prefix_len + BytePos(visible_macro.as_str().bytes().count() as u32 + 1); let mut macro_name_cov = self.curr().clone(); self.curr_mut().span = self.curr().span.with_lo(self.curr().span.lo() + after_macro_bang); @@ -766,6 +775,9 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { } } +/// See `function_source_span()` for a description of the two returned spans. +/// If the MIR `Statement` is not contributive to computing coverage spans, +/// returns `None`. pub(super) fn filtered_statement_span( statement: &'a Statement<'tcx>, body_span: Span, @@ -811,6 +823,9 @@ pub(super) fn filtered_statement_span( } } +/// See `function_source_span()` for a description of the two returned spans. +/// If the MIR `Terminator` is not contributive to computing coverage spans, +/// returns `None`. pub(super) fn filtered_terminator_span( terminator: &'a Terminator<'tcx>, body_span: Span, @@ -844,8 +859,21 @@ pub(super) fn filtered_terminator_span( } } -/// Returns the span within the function source body, and the given span, which will be different -/// if the given span is an expansion (macro, syntactic sugar, etc.). +/// Returns two spans from the given span (the span associated with a +/// `Statement` or `Terminator`): +/// +/// 1. An extrapolated span (pre-expansion[^1]) corresponding to a range within +/// the function's body source. This span is guaranteed to be contained +/// within, or equal to, the `body_span`. If the extrapolated span is not +/// contained within the `body_span`, the `body_span` is returned. +/// 2. The actual `span` value from the `Statement`, before expansion. +/// +/// Only the first span is used when computing coverage code regions. The second +/// span is useful if additional expansion data is needed (such as to look up +/// the macro name for a composed span within that macro).) +/// +/// [^1]Expansions result from Rust syntax including macros, syntactic +/// sugar, etc.). #[inline] fn function_source_span(span: Span, body_span: Span) -> (Span, Span) { let original_span = original_sp(span, body_span).with_ctxt(body_span.ctxt()); diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-84561.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-84561.txt index 5d266b5db15f2..8256daa1419b3 100644 --- a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-84561.txt +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-84561.txt @@ -10,7 +10,7 @@ | Unexecuted instantiation: ::ne ------------------ 5| |struct Foo(u32); - 6| 1|fn test2() { + 6| 1|fn test3() { 7| 1| let is_true = std::env::args().len() == 1; 8| 1| let bar = Foo(1); 9| 1| assert_eq!(bar, Foo(1)); @@ -173,8 +173,24 @@ 160| 1| debug!("debug is enabled"); 161| 1|} 162| | - 163| 1|fn main() { - 164| 1| test1(); - 165| 1| test2(); - 166| 1|} + 163| |macro_rules! call_debug { + 164| | ($($arg:tt)+) => ( + 165| 1| fn call_print(s: &str) { + 166| 1| print!("{}", s); + 167| 1| } + 168| | + 169| | call_print("called from call_debug: "); + 170| | debug!($($arg)+); + 171| | ); + 172| |} + 173| | + 174| 1|fn test2() { + 175| 1| call_debug!("debug is enabled"); + 176| 1|} + 177| | + 178| 1|fn main() { + 179| 1| test1(); + 180| 1| test2(); + 181| 1| test3(); + 182| 1|} diff --git a/src/test/run-make-fulldeps/coverage/issue-84561.rs b/src/test/run-make-fulldeps/coverage/issue-84561.rs index 5c8fd0b7caeaa..b39a289c45e20 100644 --- a/src/test/run-make-fulldeps/coverage/issue-84561.rs +++ b/src/test/run-make-fulldeps/coverage/issue-84561.rs @@ -3,7 +3,7 @@ // expect-exit-status-101 #[derive(PartialEq, Eq)] struct Foo(u32); -fn test2() { +fn test3() { let is_true = std::env::args().len() == 1; let bar = Foo(1); assert_eq!(bar, Foo(1)); @@ -160,7 +160,23 @@ fn test1() { debug!("debug is enabled"); } +macro_rules! call_debug { + ($($arg:tt)+) => ( + fn call_print(s: &str) { + print!("{}", s); + } + + call_print("called from call_debug: "); + debug!($($arg)+); + ); +} + +fn test2() { + call_debug!("debug is enabled"); +} + fn main() { test1(); test2(); + test3(); }