mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Drop branching blocks with same span as expanded macro

Browse Source 
Fixes: #84561
This commit is contained in:
Rich Kadel 2021-04-26 02:45:46 -07:00
parent 50ca3ac24f
commit c26afb765c
4 changed files with 276 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

84
compiler/rustc_mir/src/transform/coverage/spans.rs
View File

@ -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<CoverageStatement>,
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<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
@ -742,7 +790,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)
}

13
compiler/rustc_mir/src/transform/coverage/tests.rs
View File

@ -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);

102
src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-84561.txt Normal file
View File

@ -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|}

94
src/test/run-make-fulldeps/coverage/issue-84561.rs Normal file
View File

@ -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)
);
}