Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

coverage: Hoist some complex code out of the main span refinement loop #119208

Merged
merged 6 commits into from
Jan 6, 2024
Merged

coverage: Hoist some complex code out of the main span refinement loop #119208

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
506b9f9
coverage: Avoid early returns from `mir_to_initial_sorted_coverage_sp…
Zalathar Dec 22, 2023
df0df52
coverage: Overhaul how "visible macros" are determined
Zalathar Dec 28, 2023
cd3a976
coverage: Hoist the removal of unwanted macro expansion spans
Zalathar Dec 21, 2023
d4d2f14
coverage: Hoist the splitting of visible macro invocations
Zalathar Dec 22, 2023
cd50843
coverage: Split out `SpanFromMir` from `CoverageSpan`
Zalathar Dec 22, 2023
514e026
coverage: Make the remaining fields of `CoverageSpan` non-public
Zalathar Dec 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
140 changes: 11 additions & 129 deletions compiler/rustc_mir_transform/src/coverage/spans.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
use std::cell::OnceCell;

use rustc_data_structures::graph::WithNumNodes;
use rustc_index::IndexVec;
use rustc_middle::mir;
use rustc_span::{BytePos, ExpnKind, MacroKind, Span, Symbol, DUMMY_SP};
use rustc_span::{BytePos, Span, DUMMY_SP};

use super::graph::{BasicCoverageBlock, CoverageGraph, START_BCB};
use super::graph::{BasicCoverageBlock, CoverageGraph};
use crate::coverage::ExtractedHirInfo;

mod from_mir;
Expand Down Expand Up @@ -70,35 +68,17 @@ impl CoverageSpans {
/// `dominates()` the `BasicBlock`s in this `CoverageSpan`.
#[derive(Debug, Clone)]
struct CoverageSpan {
pub span: Span,
pub expn_span: Span,
pub current_macro_or_none: OnceCell<Option<Symbol>>,
pub bcb: BasicCoverageBlock,
span: Span,
bcb: BasicCoverageBlock,
/// List of all the original spans from MIR that have been merged into this
/// span. Mainly used to precisely skip over gaps when truncating a span.
pub merged_spans: Vec<Span>,
pub is_closure: bool,
merged_spans: Vec<Span>,
is_closure: bool,
}

impl CoverageSpan {
pub fn for_fn_sig(fn_sig_span: Span) -> Self {
Self::new(fn_sig_span, fn_sig_span, START_BCB, false)
}

pub(super) fn new(
span: Span,
expn_span: Span,
bcb: BasicCoverageBlock,
is_closure: bool,
) -> Self {
Self {
span,
expn_span,
current_macro_or_none: Default::default(),
bcb,
merged_spans: vec![span],
is_closure,
}
fn new(span: Span, bcb: BasicCoverageBlock, is_closure: bool) -> Self {
Self { span, bcb, merged_spans: vec![span], is_closure }
}

pub fn merge_from(&mut self, other: &Self) {
Expand All @@ -123,37 +103,6 @@ impl CoverageSpan {
pub fn is_in_same_bcb(&self, other: &Self) -> bool {
self.bcb == other.bcb
}

/// If the span is part of a macro, returns the macro name symbol.
pub fn current_macro(&self) -> Option<Symbol> {
self.current_macro_or_none
.get_or_init(|| {
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
/// body_span), returns the macro name symbol.
pub fn visible_macro(&self, body_span: Span) -> Option<Symbol> {
let current_macro = self.current_macro()?;
let parent_callsite = self.expn_span.parent_callsite()?;

// In addition to matching the context of the body span, the parent callsite
// must also be the source callsite, i.e. the parent must have no parent.
let is_visible_macro =
parent_callsite.parent_callsite().is_none() && parent_callsite.eq_ctxt(body_span);
is_visible_macro.then_some(current_macro)
}

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
Expand All @@ -164,10 +113,6 @@ impl CoverageSpan {
/// execution
/// * Carve out (leave uncovered) any span that will be counted by another MIR (notably, closures)
struct CoverageSpansGenerator<'a> {
/// A `Span` covering the function body of the MIR (typically from left curly brace to right
/// curly brace).
body_span: Span,

/// The BasicCoverageBlock Control Flow Graph (BCB CFG).
basic_coverage_blocks: &'a CoverageGraph,

Expand Down Expand Up @@ -244,7 +189,6 @@ impl<'a> CoverageSpansGenerator<'a> {
);

let coverage_spans = Self {
body_span: hir_info.body_span,
basic_coverage_blocks,
sorted_spans_iter: sorted_spans.into_iter(),
some_curr: None,
Expand All @@ -266,7 +210,6 @@ impl<'a> CoverageSpansGenerator<'a> {
// span-processing steps don't make sense yet.
if self.some_prev.is_none() {
debug!(" initial span");
self.maybe_push_macro_name_span();
continue;
}

Expand All @@ -278,15 +221,13 @@ impl<'a> CoverageSpansGenerator<'a> {
debug!(" same bcb (and neither is a closure), merge with prev={prev:?}");
let prev = self.take_prev();
self.curr_mut().merge_from(&prev);
self.maybe_push_macro_name_span();
// Note that curr.span may now differ from curr_original_span
} else if prev.span.hi() <= curr.span.lo() {
debug!(
" different bcbs and disjoint spans, so keep curr for next iter, and add prev={prev:?}",
);
let prev = self.take_prev();
self.refined_spans.push(prev);
self.maybe_push_macro_name_span();
} else if prev.is_closure {
// drop any equal or overlapping span (`curr`) and keep `prev` to test again in the
// next iter
Expand All @@ -297,35 +238,11 @@ impl<'a> CoverageSpansGenerator<'a> {
} else if curr.is_closure {
self.carve_out_span_for_closure();
} else if self.prev_original_span == curr.span {
// Note that this compares the new (`curr`) span to `prev_original_span`.
// In this branch, the actual span byte range of `prev_original_span` is not
// important. What is important is knowing whether the new `curr` span was
// **originally** the same as the original span of `prev()`. The original spans
// reflect their original sort order, and for equal spans, conveys a partial
// ordering based on CFG dominator priority.
if prev.is_macro_expansion() && 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={prev:?}",
);
self.take_curr(); // Discards curr.
} else {
self.update_pending_dups();
}
// `prev` and `curr` have the same span, or would have had the
// same span before `prev` was modified by other spans.
self.update_pending_dups();
} else {
self.cutoff_prev_at_overlapping_curr();
self.maybe_push_macro_name_span();
}
}

Expand Down Expand Up @@ -360,41 +277,6 @@ impl<'a> CoverageSpansGenerator<'a> {
self.refined_spans
}

/// If `curr` is part of a new macro expansion, carve out and push a separate
/// span that ends just after the macro name and its subsequent `!`.
fn maybe_push_macro_name_span(&mut self) {
let curr = self.curr();

let Some(visible_macro) = curr.visible_macro(self.body_span) else { return };
if let Some(prev) = &self.some_prev
&& prev.expn_span.eq_ctxt(curr.expn_span)
{
return;
}

// The split point is relative to `curr_original_span`,
// because `curr.span` may have been merged with preceding spans.
let split_point_after_macro_bang = self.curr_original_span.lo()
+ BytePos(visible_macro.as_str().len() as u32)
+ BytePos(1); // add 1 for the `!`
debug_assert!(split_point_after_macro_bang <= curr.span.hi());
if split_point_after_macro_bang > curr.span.hi() {
// Something is wrong with the macro name span;
// return now to avoid emitting malformed mappings (e.g. #117788).
return;
}

let mut macro_name_cov = curr.clone();
macro_name_cov.span = macro_name_cov.span.with_hi(split_point_after_macro_bang);
self.curr_mut().span = curr.span.with_lo(split_point_after_macro_bang);

debug!(
" and curr starts a new macro expansion, so add a new span just for \
the macro `{visible_macro}!`, new span={macro_name_cov:?}",
);
self.refined_spans.push(macro_name_cov);
}

#[track_caller]
fn curr(&self) -> &CoverageSpan {
self.some_curr.as_ref().unwrap_or_else(|| bug!("some_curr is None (curr)"))
Expand Down
Loading