From ec0110be0961846b1c69d7208f07c740564e0d8a Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 7 Dec 2023 14:04:10 +1100 Subject: [PATCH 1/3] coverage: Merge refined spans in a separate final pass This makes `push_refined_span` trivial, which will let us inline it and benefit from partial borrows of `refined_spans`. --- .../rustc_mir_transform/src/coverage/spans.rs | 29 ++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index 4db0a1db166..cce2910244b 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -89,10 +89,10 @@ pub(super) fn new( } } - pub fn merge_from(&mut self, mut other: CoverageSpan) { - debug_assert!(self.is_mergeable(&other)); + pub fn merge_from(&mut self, other: &Self) { + debug_assert!(self.is_mergeable(other)); self.span = self.span.to(other.span); - self.merged_spans.append(&mut other.merged_spans); + self.merged_spans.extend_from_slice(&other.merged_spans); } pub fn cutoff_statements_at(&mut self, cutoff_pos: BytePos) { @@ -267,7 +267,7 @@ fn to_refined_spans(mut self) -> Vec { if curr.is_mergeable(prev) { debug!(" same bcb (and neither is a closure), merge with prev={prev:?}"); let prev = self.take_prev(); - self.curr_mut().merge_from(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() { @@ -346,6 +346,17 @@ fn to_refined_spans(mut self) -> Vec { self.push_refined_span(prev); } + // Do one last merge pass, to simplify the output. + self.refined_spans.dedup_by(|b, a| { + if a.is_mergeable(b) { + debug!(?a, ?b, "merging list-adjacent refined spans"); + a.merge_from(b); + true + } else { + false + } + }); + // 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). @@ -354,15 +365,7 @@ fn to_refined_spans(mut self) -> Vec { } fn push_refined_span(&mut self, covspan: CoverageSpan) { - if let Some(last) = self.refined_spans.last_mut() - && last.is_mergeable(&covspan) - { - // Instead of pushing the new span, merge it with the last refined span. - debug!(?last, ?covspan, "merging new refined span with last refined span"); - last.merge_from(covspan); - } else { - self.refined_spans.push(covspan); - } + self.refined_spans.push(covspan); } /// If `curr` is part of a new macro expansion, carve out and push a separate From 9089d287801f93150845202066ac1a093f343e1a Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 7 Dec 2023 16:08:46 +1100 Subject: [PATCH 2/3] coverage: Inline `push_refined_span` --- .../rustc_mir_transform/src/coverage/spans.rs | 22 ++++++++----------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index cce2910244b..df26ff36998 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -275,7 +275,7 @@ fn to_refined_spans(mut self) -> Vec { " different bcbs and disjoint spans, so keep curr for next iter, and add prev={prev:?}", ); let prev = self.take_prev(); - self.push_refined_span(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 @@ -326,7 +326,7 @@ fn to_refined_spans(mut self) -> Vec { // It is never used as a field after this point. for dup in std::mem::take(&mut self.pending_dups) { debug!(" ...adding at least one pending dup={:?}", dup); - self.push_refined_span(dup); + self.refined_spans.push(dup); } // Async functions wrap a closure that implements the body to be executed. The enclosing @@ -343,7 +343,7 @@ fn to_refined_spans(mut self) -> Vec { }; if !body_ends_with_closure { - self.push_refined_span(prev); + self.refined_spans.push(prev); } // Do one last merge pass, to simplify the output. @@ -364,10 +364,6 @@ fn to_refined_spans(mut self) -> Vec { self.refined_spans } - fn push_refined_span(&mut self, covspan: CoverageSpan) { - self.refined_spans.push(covspan); - } - /// 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) { @@ -400,7 +396,7 @@ fn maybe_push_macro_name_span(&mut self) { " and curr starts a new macro expansion, so add a new span just for \ the macro `{visible_macro}!`, new span={macro_name_cov:?}", ); - self.push_refined_span(macro_name_cov); + self.refined_spans.push(macro_name_cov); } fn curr(&self) -> &CoverageSpan { @@ -462,7 +458,7 @@ fn maybe_flush_pending_dups(&mut self) { let mut pending_dups = std::mem::take(&mut self.pending_dups); for dup in pending_dups.drain(..) { debug!(" ...adding at least one pending={:?}", dup); - self.push_refined_span(dup); + self.refined_spans.push(dup); } // The list of dups is now empty, but we can recycle its capacity. assert!(pending_dups.is_empty() && self.pending_dups.is_empty()); @@ -528,10 +524,10 @@ fn carve_out_span_for_closure(&mut self) { 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.push_refined_span(dup); + self.refined_spans.push(dup); } } - self.push_refined_span(pre_closure); + self.refined_spans.push(pre_closure); } if has_post_closure_span { @@ -545,7 +541,7 @@ fn carve_out_span_for_closure(&mut self) { dup.span = dup.span.with_lo(right_cutoff); } let closure_covspan = self.take_curr(); // Prevent this curr from becoming prev. - self.push_refined_span(closure_covspan); // since self.prev() was already updated + self.refined_spans.push(closure_covspan); // since self.prev() was already updated } else { pending_dups.clear(); } @@ -648,7 +644,7 @@ fn cutoff_prev_at_overlapping_curr(&mut self) { } else { debug!(" ... adding modified prev={:?}", self.prev()); let prev = self.take_prev(); - self.push_refined_span(prev); + self.refined_spans.push(prev); } } else { // with `pending_dups`, `prev` cannot have any statements that don't overlap From 9a4321518c3ed461c9033bf02dcc1fa66da96ee3 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 7 Dec 2023 14:07:25 +1100 Subject: [PATCH 3/3] coverage: Simplify code that pushes to `refined_spans` --- .../rustc_mir_transform/src/coverage/spans.rs | 40 +++++++------------ 1 file changed, 14 insertions(+), 26 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index df26ff36998..c415a832994 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -322,9 +322,8 @@ fn to_refined_spans(mut self) -> Vec { let prev = self.take_prev(); debug!(" AT END, adding last prev={prev:?}"); - // Take `pending_dups` so that we can drain it while calling self methods. - // It is never used as a field after this point. - for dup in std::mem::take(&mut self.pending_dups) { + // Drain any remaining dups into the output. + for dup in self.pending_dups.drain(..) { debug!(" ...adding at least one pending dup={:?}", dup); self.refined_spans.push(dup); } @@ -453,19 +452,14 @@ fn maybe_flush_pending_dups(&mut self) { previous iteration, or prev started a new disjoint span" ); if last_dup.span.hi() <= self.curr().span.lo() { - // Temporarily steal `pending_dups` into a local, so that we can - // drain it while calling other self methods. - let mut pending_dups = std::mem::take(&mut self.pending_dups); - for dup in pending_dups.drain(..) { + for dup in self.pending_dups.drain(..) { debug!(" ...adding at least one pending={:?}", dup); self.refined_spans.push(dup); } - // The list of dups is now empty, but we can recycle its capacity. - assert!(pending_dups.is_empty() && self.pending_dups.is_empty()); - self.pending_dups = pending_dups; } else { self.pending_dups.clear(); } + assert!(self.pending_dups.is_empty()); } /// Advance `prev` to `curr` (if any), and `curr` to the next `CoverageSpan` in sorted order. @@ -512,21 +506,17 @@ fn carve_out_span_for_closure(&mut self) { let has_pre_closure_span = prev.span.lo() < right_cutoff; let has_post_closure_span = prev.span.hi() > right_cutoff; - // Temporarily steal `pending_dups` into a local, so that we can - // mutate and/or drain it while calling other self methods. - let mut pending_dups = std::mem::take(&mut self.pending_dups); - if has_pre_closure_span { let mut pre_closure = self.prev().clone(); pre_closure.span = pre_closure.span.with_hi(left_cutoff); debug!(" prev overlaps a closure. Adding span for pre_closure={:?}", pre_closure); - if !pending_dups.is_empty() { - 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); - } + + for mut dup in self.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.refined_spans.push(pre_closure); } @@ -536,19 +526,17 @@ fn carve_out_span_for_closure(&mut self) { // about how the `CoverageSpan`s are ordered.) self.prev_mut().span = self.prev().span.with_lo(right_cutoff); debug!(" Mutated prev.span to start after the closure. prev={:?}", self.prev()); - for dup in pending_dups.iter_mut() { + + for dup in &mut self.pending_dups { debug!(" ...and at least one overlapping dup={:?}", dup); dup.span = dup.span.with_lo(right_cutoff); } + let closure_covspan = self.take_curr(); // Prevent this curr from becoming prev. self.refined_spans.push(closure_covspan); // since self.prev() was already updated } else { - pending_dups.clear(); + self.pending_dups.clear(); } - - // Restore the modified post-closure spans, or the empty vector's capacity. - assert!(self.pending_dups.is_empty()); - self.pending_dups = pending_dups; } /// Called if `curr.span` equals `prev_original_span` (and potentially equal to all