From 617de8cfb59252a501fc7e0ab7c2510dd8a8a7f4 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 30 Jun 2024 17:36:16 +1000 Subject: [PATCH 1/2] coverage: Move span unexpansion into its own submodule --- .../src/coverage/mappings.rs | 5 +- .../rustc_mir_transform/src/coverage/mod.rs | 1 + .../rustc_mir_transform/src/coverage/spans.rs | 5 -- .../src/coverage/spans/from_mir.rs | 56 +------------------ .../src/coverage/unexpand.rs | 54 ++++++++++++++++++ 5 files changed, 59 insertions(+), 62 deletions(-) create mode 100644 compiler/rustc_mir_transform/src/coverage/unexpand.rs diff --git a/compiler/rustc_mir_transform/src/coverage/mappings.rs b/compiler/rustc_mir_transform/src/coverage/mappings.rs index 759bb7c1f9d96..e003de4e1fd71 100644 --- a/compiler/rustc_mir_transform/src/coverage/mappings.rs +++ b/compiler/rustc_mir_transform/src/coverage/mappings.rs @@ -9,9 +9,8 @@ use rustc_middle::ty::TyCtxt; use rustc_span::Span; use crate::coverage::graph::{BasicCoverageBlock, CoverageGraph, START_BCB}; -use crate::coverage::spans::{ - extract_refined_covspans, unexpand_into_body_span_with_visible_macro, -}; +use crate::coverage::spans::extract_refined_covspans; +use crate::coverage::unexpand::unexpand_into_body_span_with_visible_macro; use crate::coverage::ExtractedHirInfo; /// Associates an ordinary executable code span with its corresponding BCB. diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index 4a64d21f3d17b..d55bde311c17e 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -6,6 +6,7 @@ mod mappings; mod spans; #[cfg(test)] mod tests; +mod unexpand; use rustc_middle::mir::coverage::{ CodeRegion, CoverageKind, DecisionInfo, FunctionCoverageInfo, Mapping, MappingKind, diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index 84a70d1f02d75..7612c01c52ec4 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -14,11 +14,6 @@ use crate::coverage::ExtractedHirInfo; mod from_mir; -// FIXME(#124545) It's awkward that we have to re-export this, because it's an -// internal detail of `from_mir` that is also needed when handling branch and -// MC/DC spans. Ideally we would find a more natural home for it. -pub(super) use from_mir::unexpand_into_body_span_with_visible_macro; - pub(super) fn extract_refined_covspans( mir_body: &mir::Body<'_>, hir_info: &ExtractedHirInfo, diff --git a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs index 09deb7534bfde..2ca166929eec8 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs @@ -4,12 +4,13 @@ use rustc_middle::mir::{ self, AggregateKind, FakeReadCause, Rvalue, Statement, StatementKind, Terminator, TerminatorKind, }; -use rustc_span::{ExpnKind, MacroKind, Span, Symbol}; +use rustc_span::{Span, Symbol}; use crate::coverage::graph::{ BasicCoverageBlock, BasicCoverageBlockData, CoverageGraph, START_BCB, }; use crate::coverage::spans::Covspan; +use crate::coverage::unexpand::unexpand_into_body_span_with_visible_macro; use crate::coverage::ExtractedHirInfo; pub(crate) struct ExtractedCovspans { @@ -215,59 +216,6 @@ fn filtered_terminator_span(terminator: &Terminator<'_>) -> Option { } } -/// Returns 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`, `None` is returned. -/// -/// [^1]Expansions result from Rust syntax including macros, syntactic sugar, -/// etc.). -pub(crate) fn unexpand_into_body_span_with_visible_macro( - original_span: Span, - body_span: Span, -) -> Option<(Span, Option)> { - let (span, prev) = unexpand_into_body_span_with_prev(original_span, body_span)?; - - let visible_macro = prev - .map(|prev| match prev.ctxt().outer_expn_data().kind { - ExpnKind::Macro(MacroKind::Bang, name) => Some(name), - _ => None, - }) - .flatten(); - - Some((span, visible_macro)) -} - -/// Walks through the expansion ancestors of `original_span` to find a span that -/// is contained in `body_span` and has the same [`SyntaxContext`] as `body_span`. -/// The ancestor that was traversed just before the matching span (if any) is -/// also returned. -/// -/// For example, a return value of `Some((ancestor, Some(prev))` means that: -/// - `ancestor == original_span.find_ancestor_inside_same_ctxt(body_span)` -/// - `ancestor == prev.parent_callsite()` -/// -/// [`SyntaxContext`]: rustc_span::SyntaxContext -fn unexpand_into_body_span_with_prev( - original_span: Span, - body_span: Span, -) -> Option<(Span, Option)> { - let mut prev = None; - let mut curr = original_span; - - while !body_span.contains(curr) || !curr.eq_ctxt(body_span) { - prev = Some(curr); - curr = curr.parent_callsite()?; - } - - debug_assert_eq!(Some(curr), original_span.find_ancestor_in_same_ctxt(body_span)); - if let Some(prev) = prev { - debug_assert_eq!(Some(curr), prev.parent_callsite()); - } - - Some((curr, prev)) -} - #[derive(Debug)] pub(crate) struct Hole { pub(crate) span: Span, diff --git a/compiler/rustc_mir_transform/src/coverage/unexpand.rs b/compiler/rustc_mir_transform/src/coverage/unexpand.rs new file mode 100644 index 0000000000000..18532b8ee4581 --- /dev/null +++ b/compiler/rustc_mir_transform/src/coverage/unexpand.rs @@ -0,0 +1,54 @@ +use rustc_span::{ExpnKind, MacroKind, Span, Symbol}; + +/// Returns 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`, `None` is returned. +/// +/// [^1]Expansions result from Rust syntax including macros, syntactic sugar, +/// etc.). +pub(crate) fn unexpand_into_body_span_with_visible_macro( + original_span: Span, + body_span: Span, +) -> Option<(Span, Option)> { + let (span, prev) = unexpand_into_body_span_with_prev(original_span, body_span)?; + + let visible_macro = prev + .map(|prev| match prev.ctxt().outer_expn_data().kind { + ExpnKind::Macro(MacroKind::Bang, name) => Some(name), + _ => None, + }) + .flatten(); + + Some((span, visible_macro)) +} + +/// Walks through the expansion ancestors of `original_span` to find a span that +/// is contained in `body_span` and has the same [`SyntaxContext`] as `body_span`. +/// The ancestor that was traversed just before the matching span (if any) is +/// also returned. +/// +/// For example, a return value of `Some((ancestor, Some(prev))` means that: +/// - `ancestor == original_span.find_ancestor_inside_same_ctxt(body_span)` +/// - `ancestor == prev.parent_callsite()` +/// +/// [`SyntaxContext`]: rustc_span::SyntaxContext +fn unexpand_into_body_span_with_prev( + original_span: Span, + body_span: Span, +) -> Option<(Span, Option)> { + let mut prev = None; + let mut curr = original_span; + + while !body_span.contains(curr) || !curr.eq_ctxt(body_span) { + prev = Some(curr); + curr = curr.parent_callsite()?; + } + + debug_assert_eq!(Some(curr), original_span.find_ancestor_in_same_ctxt(body_span)); + if let Some(prev) = prev { + debug_assert_eq!(Some(curr), prev.parent_callsite()); + } + + Some((curr, prev)) +} From 6c3314905574651fa2e004173187bd5d202f0df1 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 30 Jun 2024 18:20:45 +1000 Subject: [PATCH 2/2] coverage: Avoid getting extra unexpansion info when we don't need it These particular callers don't actually use the returned macro information, so they can use a simpler span-unexpansion function that doesn't return it. --- .../src/coverage/mappings.rs | 9 +++--- .../src/coverage/unexpand.rs | 28 +++++++++++-------- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/mappings.rs b/compiler/rustc_mir_transform/src/coverage/mappings.rs index e003de4e1fd71..235992ac5470d 100644 --- a/compiler/rustc_mir_transform/src/coverage/mappings.rs +++ b/compiler/rustc_mir_transform/src/coverage/mappings.rs @@ -10,7 +10,7 @@ use rustc_span::Span; use crate::coverage::graph::{BasicCoverageBlock, CoverageGraph, START_BCB}; use crate::coverage::spans::extract_refined_covspans; -use crate::coverage::unexpand::unexpand_into_body_span_with_visible_macro; +use crate::coverage::unexpand::unexpand_into_body_span; use crate::coverage::ExtractedHirInfo; /// Associates an ordinary executable code span with its corresponding BCB. @@ -201,8 +201,7 @@ pub(super) fn extract_branch_pairs( if !raw_span.ctxt().outer_expn_data().is_root() { return None; } - let (span, _) = - unexpand_into_body_span_with_visible_macro(raw_span, hir_info.body_span)?; + let span = unexpand_into_body_span(raw_span, hir_info.body_span)?; let bcb_from_marker = |marker: BlockMarkerId| basic_coverage_blocks.bcb_from_bb(block_markers[marker]?); @@ -237,7 +236,7 @@ pub(super) fn extract_mcdc_mappings( if !raw_span.ctxt().outer_expn_data().is_root() { return None; } - let (span, _) = unexpand_into_body_span_with_visible_macro(raw_span, body_span)?; + let span = unexpand_into_body_span(raw_span, body_span)?; let true_bcb = bcb_from_marker(true_marker)?; let false_bcb = bcb_from_marker(false_marker)?; @@ -260,7 +259,7 @@ pub(super) fn extract_mcdc_mappings( mcdc_decisions.extend(branch_info.mcdc_decision_spans.iter().filter_map( |decision: &mir::coverage::MCDCDecisionSpan| { - let (span, _) = unexpand_into_body_span_with_visible_macro(decision.span, body_span)?; + let span = unexpand_into_body_span(decision.span, body_span)?; let end_bcbs = decision .end_markers diff --git a/compiler/rustc_mir_transform/src/coverage/unexpand.rs b/compiler/rustc_mir_transform/src/coverage/unexpand.rs index 18532b8ee4581..8cde291b9073e 100644 --- a/compiler/rustc_mir_transform/src/coverage/unexpand.rs +++ b/compiler/rustc_mir_transform/src/coverage/unexpand.rs @@ -1,12 +1,18 @@ use rustc_span::{ExpnKind, MacroKind, Span, Symbol}; -/// Returns 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`, `None` is returned. +/// Walks through the expansion ancestors of `original_span` to find a span that +/// is contained in `body_span` and has the same [syntax context] as `body_span`. +pub(crate) fn unexpand_into_body_span(original_span: Span, body_span: Span) -> Option { + // Because we don't need to return any extra ancestor information, + // we can just delegate directly to `find_ancestor_inside_same_ctxt`. + original_span.find_ancestor_inside_same_ctxt(body_span) +} + +/// Walks through the expansion ancestors of `original_span` to find a span that +/// is contained in `body_span` and has the same [syntax context] as `body_span`. /// -/// [^1]Expansions result from Rust syntax including macros, syntactic sugar, -/// etc.). +/// If the returned span represents a bang-macro invocation (e.g. `foo!(..)`), +/// the returned symbol will be the name of that macro (e.g. `foo`). pub(crate) fn unexpand_into_body_span_with_visible_macro( original_span: Span, body_span: Span, @@ -24,15 +30,15 @@ pub(crate) fn unexpand_into_body_span_with_visible_macro( } /// Walks through the expansion ancestors of `original_span` to find a span that -/// is contained in `body_span` and has the same [`SyntaxContext`] as `body_span`. +/// is contained in `body_span` and has the same [syntax context] as `body_span`. /// The ancestor that was traversed just before the matching span (if any) is /// also returned. /// -/// For example, a return value of `Some((ancestor, Some(prev))` means that: +/// For example, a return value of `Some((ancestor, Some(prev)))` means that: /// - `ancestor == original_span.find_ancestor_inside_same_ctxt(body_span)` -/// - `ancestor == prev.parent_callsite()` +/// - `prev.parent_callsite() == ancestor` /// -/// [`SyntaxContext`]: rustc_span::SyntaxContext +/// [syntax context]: rustc_span::SyntaxContext fn unexpand_into_body_span_with_prev( original_span: Span, body_span: Span, @@ -45,7 +51,7 @@ fn unexpand_into_body_span_with_prev( curr = curr.parent_callsite()?; } - debug_assert_eq!(Some(curr), original_span.find_ancestor_in_same_ctxt(body_span)); + debug_assert_eq!(Some(curr), original_span.find_ancestor_inside_same_ctxt(body_span)); if let Some(prev) = prev { debug_assert_eq!(Some(curr), prev.parent_callsite()); }