Skip to content

Commit 011df58

Browse files
authored
Unrolled build for rust-lang#119208
Rollup merge of rust-lang#119208 - Zalathar:hoist, r=WaffleLapkin,Swatinem coverage: Hoist some complex code out of the main span refinement loop The span refinement loop in `spans.rs` takes the spans that have been extracted from MIR, and modifies them to produce more helpful output in coverage reports. It is also one of the most complicated pieces of code in the coverage instrumentor. It has an abundance of moving pieces that make it difficult to understand, and most attempts to modify it end up accidentally changing its behaviour in unacceptable ways. This PR nevertheless tries to make a dent in it by hoisting two pieces of special-case logic out of the main loop, and into separate preprocessing passes. Coverage tests show that the resulting mappings are *almost* identical, with all known differences being unimportant. This should hopefully unlock further simplifications to the refinement loop, since it now has fewer edge cases to worry about.
2 parents d62f05b + 514e026 commit 011df58

File tree

5 files changed

+196
-181
lines changed

5 files changed

+196
-181
lines changed

compiler/rustc_mir_transform/src/coverage/spans.rs

+11-129
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
1-
use std::cell::OnceCell;
2-
31
use rustc_data_structures::graph::WithNumNodes;
42
use rustc_index::IndexVec;
53
use rustc_middle::mir;
6-
use rustc_span::{BytePos, ExpnKind, MacroKind, Span, Symbol, DUMMY_SP};
4+
use rustc_span::{BytePos, Span, DUMMY_SP};
75

8-
use super::graph::{BasicCoverageBlock, CoverageGraph, START_BCB};
6+
use super::graph::{BasicCoverageBlock, CoverageGraph};
97
use crate::coverage::ExtractedHirInfo;
108

119
mod from_mir;
@@ -70,35 +68,17 @@ impl CoverageSpans {
7068
/// `dominates()` the `BasicBlock`s in this `CoverageSpan`.
7169
#[derive(Debug, Clone)]
7270
struct CoverageSpan {
73-
pub span: Span,
74-
pub expn_span: Span,
75-
pub current_macro_or_none: OnceCell<Option<Symbol>>,
76-
pub bcb: BasicCoverageBlock,
71+
span: Span,
72+
bcb: BasicCoverageBlock,
7773
/// List of all the original spans from MIR that have been merged into this
7874
/// span. Mainly used to precisely skip over gaps when truncating a span.
79-
pub merged_spans: Vec<Span>,
80-
pub is_closure: bool,
75+
merged_spans: Vec<Span>,
76+
is_closure: bool,
8177
}
8278

8379
impl CoverageSpan {
84-
pub fn for_fn_sig(fn_sig_span: Span) -> Self {
85-
Self::new(fn_sig_span, fn_sig_span, START_BCB, false)
86-
}
87-
88-
pub(super) fn new(
89-
span: Span,
90-
expn_span: Span,
91-
bcb: BasicCoverageBlock,
92-
is_closure: bool,
93-
) -> Self {
94-
Self {
95-
span,
96-
expn_span,
97-
current_macro_or_none: Default::default(),
98-
bcb,
99-
merged_spans: vec![span],
100-
is_closure,
101-
}
80+
fn new(span: Span, bcb: BasicCoverageBlock, is_closure: bool) -> Self {
81+
Self { span, bcb, merged_spans: vec![span], is_closure }
10282
}
10383

10484
pub fn merge_from(&mut self, other: &Self) {
@@ -123,37 +103,6 @@ impl CoverageSpan {
123103
pub fn is_in_same_bcb(&self, other: &Self) -> bool {
124104
self.bcb == other.bcb
125105
}
126-
127-
/// If the span is part of a macro, returns the macro name symbol.
128-
pub fn current_macro(&self) -> Option<Symbol> {
129-
self.current_macro_or_none
130-
.get_or_init(|| {
131-
if let ExpnKind::Macro(MacroKind::Bang, current_macro) =
132-
self.expn_span.ctxt().outer_expn_data().kind
133-
{
134-
return Some(current_macro);
135-
}
136-
None
137-
})
138-
.map(|symbol| symbol)
139-
}
140-
141-
/// If the span is part of a macro, and the macro is visible (expands directly to the given
142-
/// body_span), returns the macro name symbol.
143-
pub fn visible_macro(&self, body_span: Span) -> Option<Symbol> {
144-
let current_macro = self.current_macro()?;
145-
let parent_callsite = self.expn_span.parent_callsite()?;
146-
147-
// In addition to matching the context of the body span, the parent callsite
148-
// must also be the source callsite, i.e. the parent must have no parent.
149-
let is_visible_macro =
150-
parent_callsite.parent_callsite().is_none() && parent_callsite.eq_ctxt(body_span);
151-
is_visible_macro.then_some(current_macro)
152-
}
153-
154-
pub fn is_macro_expansion(&self) -> bool {
155-
self.current_macro().is_some()
156-
}
157106
}
158107

159108
/// Converts the initial set of `CoverageSpan`s (one per MIR `Statement` or `Terminator`) into a
@@ -164,10 +113,6 @@ impl CoverageSpan {
164113
/// execution
165114
/// * Carve out (leave uncovered) any span that will be counted by another MIR (notably, closures)
166115
struct CoverageSpansGenerator<'a> {
167-
/// A `Span` covering the function body of the MIR (typically from left curly brace to right
168-
/// curly brace).
169-
body_span: Span,
170-
171116
/// The BasicCoverageBlock Control Flow Graph (BCB CFG).
172117
basic_coverage_blocks: &'a CoverageGraph,
173118

@@ -244,7 +189,6 @@ impl<'a> CoverageSpansGenerator<'a> {
244189
);
245190

246191
let coverage_spans = Self {
247-
body_span: hir_info.body_span,
248192
basic_coverage_blocks,
249193
sorted_spans_iter: sorted_spans.into_iter(),
250194
some_curr: None,
@@ -266,7 +210,6 @@ impl<'a> CoverageSpansGenerator<'a> {
266210
// span-processing steps don't make sense yet.
267211
if self.some_prev.is_none() {
268212
debug!(" initial span");
269-
self.maybe_push_macro_name_span();
270213
continue;
271214
}
272215

@@ -278,15 +221,13 @@ impl<'a> CoverageSpansGenerator<'a> {
278221
debug!(" same bcb (and neither is a closure), merge with prev={prev:?}");
279222
let prev = self.take_prev();
280223
self.curr_mut().merge_from(&prev);
281-
self.maybe_push_macro_name_span();
282224
// Note that curr.span may now differ from curr_original_span
283225
} else if prev.span.hi() <= curr.span.lo() {
284226
debug!(
285227
" different bcbs and disjoint spans, so keep curr for next iter, and add prev={prev:?}",
286228
);
287229
let prev = self.take_prev();
288230
self.refined_spans.push(prev);
289-
self.maybe_push_macro_name_span();
290231
} else if prev.is_closure {
291232
// drop any equal or overlapping span (`curr`) and keep `prev` to test again in the
292233
// next iter
@@ -297,35 +238,11 @@ impl<'a> CoverageSpansGenerator<'a> {
297238
} else if curr.is_closure {
298239
self.carve_out_span_for_closure();
299240
} else if self.prev_original_span == curr.span {
300-
// Note that this compares the new (`curr`) span to `prev_original_span`.
301-
// In this branch, the actual span byte range of `prev_original_span` is not
302-
// important. What is important is knowing whether the new `curr` span was
303-
// **originally** the same as the original span of `prev()`. The original spans
304-
// reflect their original sort order, and for equal spans, conveys a partial
305-
// ordering based on CFG dominator priority.
306-
if prev.is_macro_expansion() && curr.is_macro_expansion() {
307-
// Macros that expand to include branching (such as
308-
// `assert_eq!()`, `assert_ne!()`, `info!()`, `debug!()`, or
309-
// `trace!()`) typically generate callee spans with identical
310-
// ranges (typically the full span of the macro) for all
311-
// `BasicBlocks`. This makes it impossible to distinguish
312-
// the condition (`if val1 != val2`) from the optional
313-
// branched statements (such as the call to `panic!()` on
314-
// assert failure). In this case it is better (or less
315-
// worse) to drop the optional branch bcbs and keep the
316-
// non-conditional statements, to count when reached.
317-
debug!(
318-
" curr and prev are part of a macro expansion, and curr has the same span \
319-
as prev, but is in a different bcb. Drop curr and keep prev for next iter. \
320-
prev={prev:?}",
321-
);
322-
self.take_curr(); // Discards curr.
323-
} else {
324-
self.update_pending_dups();
325-
}
241+
// `prev` and `curr` have the same span, or would have had the
242+
// same span before `prev` was modified by other spans.
243+
self.update_pending_dups();
326244
} else {
327245
self.cutoff_prev_at_overlapping_curr();
328-
self.maybe_push_macro_name_span();
329246
}
330247
}
331248

@@ -360,41 +277,6 @@ impl<'a> CoverageSpansGenerator<'a> {
360277
self.refined_spans
361278
}
362279

363-
/// If `curr` is part of a new macro expansion, carve out and push a separate
364-
/// span that ends just after the macro name and its subsequent `!`.
365-
fn maybe_push_macro_name_span(&mut self) {
366-
let curr = self.curr();
367-
368-
let Some(visible_macro) = curr.visible_macro(self.body_span) else { return };
369-
if let Some(prev) = &self.some_prev
370-
&& prev.expn_span.eq_ctxt(curr.expn_span)
371-
{
372-
return;
373-
}
374-
375-
// The split point is relative to `curr_original_span`,
376-
// because `curr.span` may have been merged with preceding spans.
377-
let split_point_after_macro_bang = self.curr_original_span.lo()
378-
+ BytePos(visible_macro.as_str().len() as u32)
379-
+ BytePos(1); // add 1 for the `!`
380-
debug_assert!(split_point_after_macro_bang <= curr.span.hi());
381-
if split_point_after_macro_bang > curr.span.hi() {
382-
// Something is wrong with the macro name span;
383-
// return now to avoid emitting malformed mappings (e.g. #117788).
384-
return;
385-
}
386-
387-
let mut macro_name_cov = curr.clone();
388-
macro_name_cov.span = macro_name_cov.span.with_hi(split_point_after_macro_bang);
389-
self.curr_mut().span = curr.span.with_lo(split_point_after_macro_bang);
390-
391-
debug!(
392-
" and curr starts a new macro expansion, so add a new span just for \
393-
the macro `{visible_macro}!`, new span={macro_name_cov:?}",
394-
);
395-
self.refined_spans.push(macro_name_cov);
396-
}
397-
398280
#[track_caller]
399281
fn curr(&self) -> &CoverageSpan {
400282
self.some_curr.as_ref().unwrap_or_else(|| bug!("some_curr is None (curr)"))

0 commit comments

Comments
 (0)