From 4e922eb3888270507f2c42c8e421795e86735b03 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 21 Feb 2024 22:57:35 +1100 Subject: [PATCH 1/4] coverage: Remove some lingering references to `pending_dups` --- compiler/rustc_mir_transform/src/coverage/spans.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index 98fb1d8e1c940..cdc8a950640a9 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -182,10 +182,9 @@ struct SpansRefiner { /// dominance between the `BasicCoverageBlock`s of equal `Span`s. sorted_spans_iter: std::vec::IntoIter, - /// The current coverage span to compare to its `prev`, to possibly merge, discard, force the - /// discard of the `prev` (and or `pending_dups`), or keep both (with `prev` moved to - /// `pending_dups`). If `curr` is not discarded or merged, it becomes `prev` for the next - /// iteration. + /// The current coverage span to compare to its `prev`, to possibly merge, discard, + /// or cause `prev` to be modified or discarded. + /// If `curr` is not discarded or merged, it becomes `prev` for the next iteration. some_curr: Option, /// The coverage span from a prior iteration; typically assigned from that iteration's `curr`. @@ -332,8 +331,7 @@ impl SpansRefiner { /// If `prev`s span extends left of the closure (`curr`), carve out the closure's span from /// `prev`'s span. (The closure's coverage counters will be injected when processing the /// closure's own MIR.) Add the portion of the span to the left of the closure; and if the span - /// extends to the right of the closure, update `prev` to that portion of the span. For any - /// `pending_dups`, repeat the same process. + /// extends to the right of the closure, update `prev` to that portion of the span. fn carve_out_span_for_closure(&mut self) { let prev = self.prev(); let curr = self.curr(); From 14c22e393899ffcac8a884e379a09d4781351632 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 21 Feb 2024 22:20:29 +1100 Subject: [PATCH 2/4] coverage: Remove the final merge pass after span refinement This step used to be essential, but after other recent changes to span refinement it doesn't appear to be particularly necessary. Removing the final merge step will make it easier to simplify or replace the span refinement code. --- .../rustc_mir_transform/src/coverage/spans.rs | 22 ---- tests/coverage/fn_sig_into_try.cov-map | 27 +++-- tests/coverage/issue-84561.cov-map | 9 +- tests/coverage/try_error_result.cov-map | 106 +++++++++--------- 4 files changed, 75 insertions(+), 89 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index cdc8a950640a9..32cb9cd94d42e 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -159,17 +159,6 @@ struct RefinedCovspan { is_closure: bool, } -impl RefinedCovspan { - fn is_mergeable(&self, other: &Self) -> bool { - self.bcb == other.bcb && !self.is_closure && !other.is_closure - } - - fn merge_from(&mut self, other: &Self) { - debug_assert!(self.is_mergeable(other)); - self.span = self.span.to(other.span); - } -} - /// Converts the initial set of coverage spans (one per MIR `Statement` or `Terminator`) into a /// minimal set of coverage spans, using the BCB CFG to determine where it is safe and useful to: /// @@ -258,17 +247,6 @@ impl SpansRefiner { self.refined_spans.push(prev.into_refined()); } - // 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 spans 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). diff --git a/tests/coverage/fn_sig_into_try.cov-map b/tests/coverage/fn_sig_into_try.cov-map index c3969f8ce9990..177d2229e042f 100644 --- a/tests/coverage/fn_sig_into_try.cov-map +++ b/tests/coverage/fn_sig_into_try.cov-map @@ -7,45 +7,48 @@ Number of file 0 mappings: 1 - Code(Counter(0)) at (prev + 10, 1) to (start + 5, 2) Function name: fn_sig_into_try::b -Raw bytes (28): 0x[01, 01, 02, 01, 00, 00, 02, 04, 01, 11, 01, 03, 0f, 00, 03, 0f, 00, 10, 02, 01, 05, 00, 0c, 07, 01, 01, 00, 02] +Raw bytes (33): 0x[01, 01, 02, 01, 00, 00, 02, 05, 01, 11, 01, 02, 01, 01, 03, 05, 00, 0f, 00, 00, 0f, 00, 10, 02, 01, 05, 00, 0c, 07, 01, 01, 00, 02] Number of files: 1 - file 0 => global file 1 Number of expressions: 2 - expression 0 operands: lhs = Counter(0), rhs = Zero - expression 1 operands: lhs = Zero, rhs = Expression(0, Sub) -Number of file 0 mappings: 4 -- Code(Counter(0)) at (prev + 17, 1) to (start + 3, 15) -- Code(Zero) at (prev + 3, 15) to (start + 0, 16) +Number of file 0 mappings: 5 +- Code(Counter(0)) at (prev + 17, 1) to (start + 2, 1) +- Code(Counter(0)) at (prev + 3, 5) to (start + 0, 15) +- Code(Zero) at (prev + 0, 15) to (start + 0, 16) - Code(Expression(0, Sub)) at (prev + 1, 5) to (start + 0, 12) = (c0 - Zero) - Code(Expression(1, Add)) at (prev + 1, 1) to (start + 0, 2) = (Zero + (c0 - Zero)) Function name: fn_sig_into_try::c -Raw bytes (28): 0x[01, 01, 02, 01, 00, 00, 02, 04, 01, 18, 01, 03, 17, 00, 03, 17, 00, 18, 02, 01, 05, 00, 0c, 07, 01, 01, 00, 02] +Raw bytes (33): 0x[01, 01, 02, 01, 00, 00, 02, 05, 01, 18, 01, 02, 01, 01, 03, 0d, 00, 17, 00, 00, 17, 00, 18, 02, 01, 05, 00, 0c, 07, 01, 01, 00, 02] Number of files: 1 - file 0 => global file 1 Number of expressions: 2 - expression 0 operands: lhs = Counter(0), rhs = Zero - expression 1 operands: lhs = Zero, rhs = Expression(0, Sub) -Number of file 0 mappings: 4 -- Code(Counter(0)) at (prev + 24, 1) to (start + 3, 23) -- Code(Zero) at (prev + 3, 23) to (start + 0, 24) +Number of file 0 mappings: 5 +- Code(Counter(0)) at (prev + 24, 1) to (start + 2, 1) +- Code(Counter(0)) at (prev + 3, 13) to (start + 0, 23) +- Code(Zero) at (prev + 0, 23) to (start + 0, 24) - Code(Expression(0, Sub)) at (prev + 1, 5) to (start + 0, 12) = (c0 - Zero) - Code(Expression(1, Add)) at (prev + 1, 1) to (start + 0, 2) = (Zero + (c0 - Zero)) Function name: fn_sig_into_try::d -Raw bytes (28): 0x[01, 01, 02, 01, 00, 00, 02, 04, 01, 1f, 01, 04, 0f, 00, 04, 0f, 00, 10, 02, 01, 05, 00, 0c, 07, 01, 01, 00, 02] +Raw bytes (33): 0x[01, 01, 02, 01, 00, 00, 02, 05, 01, 1f, 01, 03, 13, 01, 04, 05, 00, 0f, 00, 00, 0f, 00, 10, 02, 01, 05, 00, 0c, 07, 01, 01, 00, 02] Number of files: 1 - file 0 => global file 1 Number of expressions: 2 - expression 0 operands: lhs = Counter(0), rhs = Zero - expression 1 operands: lhs = Zero, rhs = Expression(0, Sub) -Number of file 0 mappings: 4 -- Code(Counter(0)) at (prev + 31, 1) to (start + 4, 15) -- Code(Zero) at (prev + 4, 15) to (start + 0, 16) +Number of file 0 mappings: 5 +- Code(Counter(0)) at (prev + 31, 1) to (start + 3, 19) +- Code(Counter(0)) at (prev + 4, 5) to (start + 0, 15) +- Code(Zero) at (prev + 0, 15) to (start + 0, 16) - Code(Expression(0, Sub)) at (prev + 1, 5) to (start + 0, 12) = (c0 - Zero) - Code(Expression(1, Add)) at (prev + 1, 1) to (start + 0, 2) diff --git a/tests/coverage/issue-84561.cov-map b/tests/coverage/issue-84561.cov-map index a81884ea942d8..f7eb9b560a4ff 100644 --- a/tests/coverage/issue-84561.cov-map +++ b/tests/coverage/issue-84561.cov-map @@ -1,13 +1,14 @@ Function name: ::fmt -Raw bytes (29): 0x[01, 01, 02, 01, 05, 05, 02, 04, 01, 8a, 01, 05, 01, 25, 05, 01, 25, 00, 26, 02, 01, 09, 00, 0f, 07, 01, 05, 00, 06] +Raw bytes (34): 0x[01, 01, 02, 01, 05, 05, 02, 05, 01, 8a, 01, 05, 00, 44, 01, 01, 09, 00, 25, 05, 00, 25, 00, 26, 02, 01, 09, 00, 0f, 07, 01, 05, 00, 06] Number of files: 1 - file 0 => global file 1 Number of expressions: 2 - expression 0 operands: lhs = Counter(0), rhs = Counter(1) - expression 1 operands: lhs = Counter(1), rhs = Expression(0, Sub) -Number of file 0 mappings: 4 -- Code(Counter(0)) at (prev + 138, 5) to (start + 1, 37) -- Code(Counter(1)) at (prev + 1, 37) to (start + 0, 38) +Number of file 0 mappings: 5 +- Code(Counter(0)) at (prev + 138, 5) to (start + 0, 68) +- Code(Counter(0)) at (prev + 1, 9) to (start + 0, 37) +- Code(Counter(1)) at (prev + 0, 37) to (start + 0, 38) - Code(Expression(0, Sub)) at (prev + 1, 9) to (start + 0, 15) = (c0 - c1) - Code(Expression(1, Add)) at (prev + 1, 5) to (start + 0, 6) diff --git a/tests/coverage/try_error_result.cov-map b/tests/coverage/try_error_result.cov-map index 83f1869a31e6c..f64e07524809a 100644 --- a/tests/coverage/try_error_result.cov-map +++ b/tests/coverage/try_error_result.cov-map @@ -44,15 +44,16 @@ Number of file 0 mappings: 4 = (c1 + (c0 - c1)) Function name: try_error_result::main -Raw bytes (28): 0x[01, 01, 02, 01, 05, 05, 02, 04, 01, 73, 01, 02, 0c, 05, 03, 05, 00, 06, 02, 02, 05, 00, 0b, 07, 01, 01, 00, 02] +Raw bytes (33): 0x[01, 01, 02, 01, 05, 05, 02, 05, 01, 73, 01, 01, 2c, 01, 02, 05, 00, 0c, 05, 01, 05, 00, 06, 02, 02, 05, 00, 0b, 07, 01, 01, 00, 02] Number of files: 1 - file 0 => global file 1 Number of expressions: 2 - expression 0 operands: lhs = Counter(0), rhs = Counter(1) - expression 1 operands: lhs = Counter(1), rhs = Expression(0, Sub) -Number of file 0 mappings: 4 -- Code(Counter(0)) at (prev + 115, 1) to (start + 2, 12) -- Code(Counter(1)) at (prev + 3, 5) to (start + 0, 6) +Number of file 0 mappings: 5 +- Code(Counter(0)) at (prev + 115, 1) to (start + 1, 44) +- Code(Counter(0)) at (prev + 2, 5) to (start + 0, 12) +- Code(Counter(1)) at (prev + 1, 5) to (start + 0, 6) - Code(Expression(0, Sub)) at (prev + 2, 5) to (start + 0, 11) = (c0 - c1) - Code(Expression(1, Add)) at (prev + 1, 1) to (start + 0, 2) @@ -89,61 +90,64 @@ Number of file 0 mappings: 11 = (((c4 + Zero) + Zero) + c3) Function name: try_error_result::test2 -Raw bytes (280): 0x[01, 01, 24, 01, 07, 00, 09, 03, 0d, 41, 00, 1e, 00, 41, 00, 1e, 00, 41, 00, 4a, 00, 4e, 00, 52, 41, 03, 0d, 52, 41, 03, 0d, 4e, 00, 52, 41, 03, 0d, 4a, 00, 4e, 00, 52, 41, 03, 0d, 66, 00, 45, 00, 45, 00, 66, 00, 45, 00, 7a, 00, 4d, 00, 4d, 00, 7a, 00, 4d, 00, 83, 01, 0d, 87, 01, 00, 00, 8b, 01, 8f, 01, 00, 19, 00, 28, 01, 3e, 01, 03, 17, 03, 08, 09, 00, 0e, 52, 02, 09, 04, 1a, 41, 06, 0d, 00, 2f, 00, 00, 2f, 00, 30, 1e, 00, 31, 03, 35, 00, 04, 11, 00, 12, 1a, 02, 11, 04, 12, 00, 05, 11, 00, 14, 1a, 00, 17, 00, 41, 19, 00, 41, 00, 42, 00, 00, 43, 00, 5f, 00, 00, 5f, 00, 60, 00, 01, 0d, 00, 20, 00, 01, 11, 00, 14, 00, 00, 17, 00, 41, 00, 00, 41, 00, 42, 00, 00, 43, 00, 60, 00, 00, 60, 00, 61, 00, 01, 0d, 00, 20, 46, 04, 11, 00, 14, 4e, 00, 17, 00, 42, 00, 00, 42, 00, 43, 4a, 00, 44, 00, 61, 00, 00, 61, 00, 62, 46, 01, 0d, 00, 20, 62, 01, 11, 00, 14, 45, 00, 17, 01, 36, 00, 01, 36, 00, 37, 66, 01, 12, 00, 2f, 00, 00, 2f, 00, 30, 62, 01, 0d, 00, 20, 76, 01, 11, 00, 14, 4d, 00, 17, 01, 36, 00, 02, 11, 00, 12, 7a, 01, 12, 00, 2f, 00, 01, 11, 00, 12, 76, 02, 0d, 00, 20, 0d, 03, 05, 00, 0b, 7f, 01, 01, 00, 02] +Raw bytes (288): 0x[01, 01, 25, 01, 07, 00, 09, 03, 0d, 41, 00, 41, 00, 22, 00, 41, 00, 22, 00, 41, 00, 4e, 00, 52, 00, 56, 41, 03, 0d, 56, 41, 03, 0d, 52, 00, 56, 41, 03, 0d, 4e, 00, 52, 00, 56, 41, 03, 0d, 6a, 00, 45, 00, 45, 00, 6a, 00, 45, 00, 7e, 00, 4d, 00, 4d, 00, 7e, 00, 4d, 00, 87, 01, 0d, 8b, 01, 00, 00, 8f, 01, 93, 01, 00, 19, 00, 29, 01, 3e, 01, 03, 17, 03, 08, 09, 00, 0e, 56, 02, 09, 04, 1a, 41, 06, 0d, 00, 2f, 00, 00, 2f, 00, 30, 22, 00, 31, 00, 63, 22, 01, 0d, 02, 35, 00, 03, 11, 00, 12, 1e, 02, 11, 04, 12, 00, 05, 11, 00, 14, 1e, 00, 17, 00, 41, 19, 00, 41, 00, 42, 00, 00, 43, 00, 5f, 00, 00, 5f, 00, 60, 00, 01, 0d, 00, 20, 00, 01, 11, 00, 14, 00, 00, 17, 00, 41, 00, 00, 41, 00, 42, 00, 00, 43, 00, 60, 00, 00, 60, 00, 61, 00, 01, 0d, 00, 20, 4a, 04, 11, 00, 14, 52, 00, 17, 00, 42, 00, 00, 42, 00, 43, 4e, 00, 44, 00, 61, 00, 00, 61, 00, 62, 4a, 01, 0d, 00, 20, 66, 01, 11, 00, 14, 45, 00, 17, 01, 36, 00, 01, 36, 00, 37, 6a, 01, 12, 00, 2f, 00, 00, 2f, 00, 30, 66, 01, 0d, 00, 20, 7a, 01, 11, 00, 14, 4d, 00, 17, 01, 36, 00, 02, 11, 00, 12, 7e, 01, 12, 00, 2f, 00, 01, 11, 00, 12, 7a, 02, 0d, 00, 20, 0d, 03, 05, 00, 0b, 83, 01, 01, 01, 00, 02] Number of files: 1 - file 0 => global file 1 -Number of expressions: 36 +Number of expressions: 37 - expression 0 operands: lhs = Counter(0), rhs = Expression(1, Add) - expression 1 operands: lhs = Zero, rhs = Counter(2) - expression 2 operands: lhs = Expression(0, Add), rhs = Counter(3) - expression 3 operands: lhs = Counter(16), rhs = Zero -- expression 4 operands: lhs = Expression(7, Sub), rhs = Zero -- expression 5 operands: lhs = Counter(16), rhs = Zero -- expression 6 operands: lhs = Expression(7, Sub), rhs = Zero -- expression 7 operands: lhs = Counter(16), rhs = Zero -- expression 8 operands: lhs = Expression(18, Sub), rhs = Zero +- expression 4 operands: lhs = Counter(16), rhs = Zero +- expression 5 operands: lhs = Expression(8, Sub), rhs = Zero +- expression 6 operands: lhs = Counter(16), rhs = Zero +- expression 7 operands: lhs = Expression(8, Sub), rhs = Zero +- expression 8 operands: lhs = Counter(16), rhs = Zero - expression 9 operands: lhs = Expression(19, Sub), rhs = Zero -- expression 10 operands: lhs = Expression(20, Sub), rhs = Counter(16) -- expression 11 operands: lhs = Expression(0, Add), rhs = Counter(3) -- expression 12 operands: lhs = Expression(20, Sub), rhs = Counter(16) -- expression 13 operands: lhs = Expression(0, Add), rhs = Counter(3) -- expression 14 operands: lhs = Expression(19, Sub), rhs = Zero -- expression 15 operands: lhs = Expression(20, Sub), rhs = Counter(16) -- expression 16 operands: lhs = Expression(0, Add), rhs = Counter(3) -- expression 17 operands: lhs = Expression(18, Sub), rhs = Zero +- expression 10 operands: lhs = Expression(20, Sub), rhs = Zero +- expression 11 operands: lhs = Expression(21, Sub), rhs = Counter(16) +- expression 12 operands: lhs = Expression(0, Add), rhs = Counter(3) +- expression 13 operands: lhs = Expression(21, Sub), rhs = Counter(16) +- expression 14 operands: lhs = Expression(0, Add), rhs = Counter(3) +- expression 15 operands: lhs = Expression(20, Sub), rhs = Zero +- expression 16 operands: lhs = Expression(21, Sub), rhs = Counter(16) +- expression 17 operands: lhs = Expression(0, Add), rhs = Counter(3) - expression 18 operands: lhs = Expression(19, Sub), rhs = Zero -- expression 19 operands: lhs = Expression(20, Sub), rhs = Counter(16) -- expression 20 operands: lhs = Expression(0, Add), rhs = Counter(3) -- expression 21 operands: lhs = Expression(25, Sub), rhs = Zero -- expression 22 operands: lhs = Counter(17), rhs = Zero +- expression 19 operands: lhs = Expression(20, Sub), rhs = Zero +- expression 20 operands: lhs = Expression(21, Sub), rhs = Counter(16) +- expression 21 operands: lhs = Expression(0, Add), rhs = Counter(3) +- expression 22 operands: lhs = Expression(26, Sub), rhs = Zero - expression 23 operands: lhs = Counter(17), rhs = Zero -- expression 24 operands: lhs = Expression(25, Sub), rhs = Zero -- expression 25 operands: lhs = Counter(17), rhs = Zero -- expression 26 operands: lhs = Expression(30, Sub), rhs = Zero -- expression 27 operands: lhs = Counter(19), rhs = Zero +- expression 24 operands: lhs = Counter(17), rhs = Zero +- expression 25 operands: lhs = Expression(26, Sub), rhs = Zero +- expression 26 operands: lhs = Counter(17), rhs = Zero +- expression 27 operands: lhs = Expression(31, Sub), rhs = Zero - expression 28 operands: lhs = Counter(19), rhs = Zero -- expression 29 operands: lhs = Expression(30, Sub), rhs = Zero -- expression 30 operands: lhs = Counter(19), rhs = Zero -- expression 31 operands: lhs = Expression(32, Add), rhs = Counter(3) -- expression 32 operands: lhs = Expression(33, Add), rhs = Zero -- expression 33 operands: lhs = Zero, rhs = Expression(34, Add) -- expression 34 operands: lhs = Expression(35, Add), rhs = Zero -- expression 35 operands: lhs = Counter(6), rhs = Zero -Number of file 0 mappings: 40 +- expression 29 operands: lhs = Counter(19), rhs = Zero +- expression 30 operands: lhs = Expression(31, Sub), rhs = Zero +- expression 31 operands: lhs = Counter(19), rhs = Zero +- expression 32 operands: lhs = Expression(33, Add), rhs = Counter(3) +- expression 33 operands: lhs = Expression(34, Add), rhs = Zero +- expression 34 operands: lhs = Zero, rhs = Expression(35, Add) +- expression 35 operands: lhs = Expression(36, Add), rhs = Zero +- expression 36 operands: lhs = Counter(6), rhs = Zero +Number of file 0 mappings: 41 - Code(Counter(0)) at (prev + 62, 1) to (start + 3, 23) - Code(Expression(0, Add)) at (prev + 8, 9) to (start + 0, 14) = (c0 + (Zero + c2)) -- Code(Expression(20, Sub)) at (prev + 2, 9) to (start + 4, 26) +- Code(Expression(21, Sub)) at (prev + 2, 9) to (start + 4, 26) = ((c0 + (Zero + c2)) - c3) - Code(Counter(16)) at (prev + 6, 13) to (start + 0, 47) - Code(Zero) at (prev + 0, 47) to (start + 0, 48) -- Code(Expression(7, Sub)) at (prev + 0, 49) to (start + 3, 53) +- Code(Expression(8, Sub)) at (prev + 0, 49) to (start + 0, 99) + = (c16 - Zero) +- Code(Expression(8, Sub)) at (prev + 1, 13) to (start + 2, 53) = (c16 - Zero) -- Code(Zero) at (prev + 4, 17) to (start + 0, 18) -- Code(Expression(6, Sub)) at (prev + 2, 17) to (start + 4, 18) +- Code(Zero) at (prev + 3, 17) to (start + 0, 18) +- Code(Expression(7, Sub)) at (prev + 2, 17) to (start + 4, 18) = ((c16 - Zero) - Zero) - Code(Zero) at (prev + 5, 17) to (start + 0, 20) -- Code(Expression(6, Sub)) at (prev + 0, 23) to (start + 0, 65) +- Code(Expression(7, Sub)) at (prev + 0, 23) to (start + 0, 65) = ((c16 - Zero) - Zero) - Code(Counter(6)) at (prev + 0, 65) to (start + 0, 66) - Code(Zero) at (prev + 0, 67) to (start + 0, 95) @@ -155,35 +159,35 @@ Number of file 0 mappings: 40 - Code(Zero) at (prev + 0, 67) to (start + 0, 96) - Code(Zero) at (prev + 0, 96) to (start + 0, 97) - Code(Zero) at (prev + 1, 13) to (start + 0, 32) -- Code(Expression(17, Sub)) at (prev + 4, 17) to (start + 0, 20) +- Code(Expression(18, Sub)) at (prev + 4, 17) to (start + 0, 20) = (((((c0 + (Zero + c2)) - c3) - c16) - Zero) - Zero) -- Code(Expression(19, Sub)) at (prev + 0, 23) to (start + 0, 66) +- Code(Expression(20, Sub)) at (prev + 0, 23) to (start + 0, 66) = (((c0 + (Zero + c2)) - c3) - c16) - Code(Zero) at (prev + 0, 66) to (start + 0, 67) -- Code(Expression(18, Sub)) at (prev + 0, 68) to (start + 0, 97) +- Code(Expression(19, Sub)) at (prev + 0, 68) to (start + 0, 97) = ((((c0 + (Zero + c2)) - c3) - c16) - Zero) - Code(Zero) at (prev + 0, 97) to (start + 0, 98) -- Code(Expression(17, Sub)) at (prev + 1, 13) to (start + 0, 32) +- Code(Expression(18, Sub)) at (prev + 1, 13) to (start + 0, 32) = (((((c0 + (Zero + c2)) - c3) - c16) - Zero) - Zero) -- Code(Expression(24, Sub)) at (prev + 1, 17) to (start + 0, 20) +- Code(Expression(25, Sub)) at (prev + 1, 17) to (start + 0, 20) = ((c17 - Zero) - Zero) - Code(Counter(17)) at (prev + 0, 23) to (start + 1, 54) - Code(Zero) at (prev + 1, 54) to (start + 0, 55) -- Code(Expression(25, Sub)) at (prev + 1, 18) to (start + 0, 47) +- Code(Expression(26, Sub)) at (prev + 1, 18) to (start + 0, 47) = (c17 - Zero) - Code(Zero) at (prev + 0, 47) to (start + 0, 48) -- Code(Expression(24, Sub)) at (prev + 1, 13) to (start + 0, 32) +- Code(Expression(25, Sub)) at (prev + 1, 13) to (start + 0, 32) = ((c17 - Zero) - Zero) -- Code(Expression(29, Sub)) at (prev + 1, 17) to (start + 0, 20) +- Code(Expression(30, Sub)) at (prev + 1, 17) to (start + 0, 20) = ((c19 - Zero) - Zero) - Code(Counter(19)) at (prev + 0, 23) to (start + 1, 54) - Code(Zero) at (prev + 2, 17) to (start + 0, 18) -- Code(Expression(30, Sub)) at (prev + 1, 18) to (start + 0, 47) +- Code(Expression(31, Sub)) at (prev + 1, 18) to (start + 0, 47) = (c19 - Zero) - Code(Zero) at (prev + 1, 17) to (start + 0, 18) -- Code(Expression(29, Sub)) at (prev + 2, 13) to (start + 0, 32) +- Code(Expression(30, Sub)) at (prev + 2, 13) to (start + 0, 32) = ((c19 - Zero) - Zero) - Code(Counter(3)) at (prev + 3, 5) to (start + 0, 11) -- Code(Expression(31, Add)) at (prev + 1, 1) to (start + 0, 2) +- Code(Expression(32, Add)) at (prev + 1, 1) to (start + 0, 2) = (((Zero + ((c6 + Zero) + Zero)) + Zero) + c3) From f380bd77a90336f36edab26a4d0d96298ab9edf4 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 21 Feb 2024 22:38:28 +1100 Subject: [PATCH 3/4] coverage: Closure spans don't need to become refined spans Now that we don't perform a final merge pass on refined spans, there's no need to create refined spans for closure/hole spans, so we can just discard them immediately. --- .../rustc_mir_transform/src/coverage/spans.rs | 33 +++++++------------ 1 file changed, 11 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index 32cb9cd94d42e..51efdbbfcf2b9 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -102,13 +102,6 @@ impl CurrCovspan { let Self { span, bcb, is_closure } = self; PrevCovspan { span, bcb, merged_spans: vec![span], is_closure } } - - fn into_refined(self) -> RefinedCovspan { - // This is only called in cases where `curr` is a closure span that has - // been carved out of `prev`. - debug_assert!(self.is_closure); - self.into_prev().into_refined() - } } #[derive(Debug)] @@ -138,15 +131,15 @@ impl PrevCovspan { self.span = self.span.with_hi(max_hi); } - if self.merged_spans.is_empty() { None } else { Some(self.into_refined()) } + if self.merged_spans.is_empty() { None } else { self.into_refined() } } - fn refined_copy(&self) -> RefinedCovspan { + fn refined_copy(&self) -> Option { let &Self { span, bcb, merged_spans: _, is_closure } = self; - RefinedCovspan { span, bcb, is_closure } + (!is_closure).then_some(RefinedCovspan { span, bcb }) } - fn into_refined(self) -> RefinedCovspan { + fn into_refined(self) -> Option { // Even though we consume self, we can just reuse the copying impl. self.refined_copy() } @@ -156,7 +149,6 @@ impl PrevCovspan { struct RefinedCovspan { span: Span, bcb: BasicCoverageBlock, - is_closure: bool, } /// Converts the initial set of coverage spans (one per MIR `Statement` or `Terminator`) into a @@ -225,7 +217,7 @@ impl SpansRefiner { " different bcbs and disjoint spans, so keep curr for next iter, and add prev={prev:?}", ); let prev = self.take_prev().into_refined(); - self.refined_spans.push(prev); + self.refined_spans.extend(prev); } else if prev.is_closure { // drop any equal or overlapping span (`curr`) and keep `prev` to test again in the // next iter @@ -244,13 +236,9 @@ impl SpansRefiner { // so add it to the output as well. if let Some(prev) = self.some_prev.take() { debug!(" AT END, adding last prev={prev:?}"); - self.refined_spans.push(prev.into_refined()); + self.refined_spans.extend(prev.into_refined()); } - // Remove spans 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). - self.refined_spans.retain(|covspan| !covspan.is_closure); self.refined_spans } @@ -313,6 +301,7 @@ impl SpansRefiner { fn carve_out_span_for_closure(&mut self) { let prev = self.prev(); let curr = self.curr(); + assert!(!prev.is_closure && curr.is_closure); let left_cutoff = curr.span.lo(); let right_cutoff = curr.span.hi(); @@ -320,7 +309,7 @@ impl SpansRefiner { let has_post_closure_span = prev.span.hi() > right_cutoff; if has_pre_closure_span { - let mut pre_closure = self.prev().refined_copy(); + let mut pre_closure = prev.refined_copy().expect("prev is not a closure span"); pre_closure.span = pre_closure.span.with_hi(left_cutoff); debug!(" prev overlaps a closure. Adding span for pre_closure={:?}", pre_closure); self.refined_spans.push(pre_closure); @@ -331,9 +320,9 @@ impl SpansRefiner { self.prev_mut().span = self.prev().span.with_lo(right_cutoff); debug!(" Mutated prev.span to start after the closure. prev={:?}", self.prev()); - // Prevent this curr from becoming prev. - let closure_covspan = self.take_curr().into_refined(); - self.refined_spans.push(closure_covspan); // since self.prev() was already updated + // Discard this curr, since it's a closure span. + let curr = self.take_curr(); + assert!(curr.is_closure); } } From 303c3330c717036cb5f81b8cea02da50a84b3cb5 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 21 Feb 2024 22:53:37 +1100 Subject: [PATCH 4/4] coverage: Rename `is_closure` to `is_hole` When refining covspans, we don't specifically care which ones represent closures; we just want to know which ones represent "holes" that should be carved out of other spans and then discarded. (Closures are currently the only source of hole spans, but in the future we might want to also create hole spans for nested items and inactive `#[cfg(..)]` regions.) --- .../rustc_mir_transform/src/coverage/spans.rs | 76 +++++++++---------- .../src/coverage/spans/from_mir.rs | 25 +++--- 2 files changed, 49 insertions(+), 52 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index 51efdbbfcf2b9..1d82fcf7ac479 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -90,17 +90,17 @@ pub(super) fn generate_coverage_spans( struct CurrCovspan { span: Span, bcb: BasicCoverageBlock, - is_closure: bool, + is_hole: bool, } impl CurrCovspan { - fn new(span: Span, bcb: BasicCoverageBlock, is_closure: bool) -> Self { - Self { span, bcb, is_closure } + fn new(span: Span, bcb: BasicCoverageBlock, is_hole: bool) -> Self { + Self { span, bcb, is_hole } } fn into_prev(self) -> PrevCovspan { - let Self { span, bcb, is_closure } = self; - PrevCovspan { span, bcb, merged_spans: vec![span], is_closure } + let Self { span, bcb, is_hole } = self; + PrevCovspan { span, bcb, merged_spans: vec![span], is_hole } } } @@ -111,12 +111,12 @@ struct PrevCovspan { /// 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. merged_spans: Vec, - is_closure: bool, + is_hole: bool, } impl PrevCovspan { fn is_mergeable(&self, other: &CurrCovspan) -> bool { - self.bcb == other.bcb && !self.is_closure && !other.is_closure + self.bcb == other.bcb && !self.is_hole && !other.is_hole } fn merge_from(&mut self, other: &CurrCovspan) { @@ -135,8 +135,8 @@ impl PrevCovspan { } fn refined_copy(&self) -> Option { - let &Self { span, bcb, merged_spans: _, is_closure } = self; - (!is_closure).then_some(RefinedCovspan { span, bcb }) + let &Self { span, bcb, merged_spans: _, is_hole } = self; + (!is_hole).then_some(RefinedCovspan { span, bcb }) } fn into_refined(self) -> Option { @@ -209,7 +209,7 @@ impl SpansRefiner { let curr = self.curr(); if prev.is_mergeable(curr) { - debug!(" same bcb (and neither is a closure), merge with prev={prev:?}"); + debug!(?prev, "curr will be merged into prev"); let curr = self.take_curr(); self.prev_mut().merge_from(&curr); } else if prev.span.hi() <= curr.span.lo() { @@ -218,15 +218,13 @@ impl SpansRefiner { ); let prev = self.take_prev().into_refined(); self.refined_spans.extend(prev); - } else if prev.is_closure { + } else if prev.is_hole { // drop any equal or overlapping span (`curr`) and keep `prev` to test again in the // next iter - debug!( - " curr overlaps a closure (prev). Drop curr and keep prev for next iter. prev={prev:?}", - ); + debug!(?prev, "prev (a hole) overlaps curr, so discarding curr"); self.take_curr(); // Discards curr. - } else if curr.is_closure { - self.carve_out_span_for_closure(); + } else if curr.is_hole { + self.carve_out_span_for_hole(); } else { self.cutoff_prev_at_overlapping_curr(); } @@ -281,48 +279,44 @@ impl SpansRefiner { { // Skip curr because prev has already advanced beyond the end of curr. // This can only happen if a prior iteration updated `prev` to skip past - // a region of code, such as skipping past a closure. - debug!( - " prev.span starts after curr.span, so curr will be dropped (skipping past \ - closure?); prev={prev:?}", - ); + // a region of code, such as skipping past a hole. + debug!(?prev, "prev.span starts after curr.span, so curr will be dropped"); } else { - self.some_curr = Some(CurrCovspan::new(curr.span, curr.bcb, curr.is_closure)); + self.some_curr = Some(CurrCovspan::new(curr.span, curr.bcb, curr.is_hole)); return true; } } false } - /// If `prev`s span extends left of the closure (`curr`), carve out the closure's span from - /// `prev`'s span. (The closure's coverage counters will be injected when processing the - /// closure's own MIR.) Add the portion of the span to the left of the closure; and if the span - /// extends to the right of the closure, update `prev` to that portion of the span. - fn carve_out_span_for_closure(&mut self) { + /// If `prev`s span extends left of the hole (`curr`), carve out the hole's span from + /// `prev`'s span. Add the portion of the span to the left of the hole; and if the span + /// extends to the right of the hole, update `prev` to that portion of the span. + fn carve_out_span_for_hole(&mut self) { let prev = self.prev(); let curr = self.curr(); - assert!(!prev.is_closure && curr.is_closure); + assert!(!prev.is_hole && curr.is_hole); let left_cutoff = curr.span.lo(); let right_cutoff = curr.span.hi(); - let has_pre_closure_span = prev.span.lo() < right_cutoff; - let has_post_closure_span = prev.span.hi() > right_cutoff; - - if has_pre_closure_span { - let mut pre_closure = prev.refined_copy().expect("prev is not a closure span"); - pre_closure.span = pre_closure.span.with_hi(left_cutoff); - debug!(" prev overlaps a closure. Adding span for pre_closure={:?}", pre_closure); - self.refined_spans.push(pre_closure); + let has_pre_hole_span = prev.span.lo() < right_cutoff; + let has_post_hole_span = prev.span.hi() > right_cutoff; + + if has_pre_hole_span { + let mut pre_hole = prev.refined_copy().expect("prev is not a hole span"); + pre_hole.span = pre_hole.span.with_hi(left_cutoff); + debug!(?pre_hole, "prev overlaps a hole; adding pre-hole span"); + self.refined_spans.push(pre_hole); } - if has_post_closure_span { - // Mutate `prev.span` to start after the closure (and discard curr). + if has_post_hole_span { + // Mutate `prev.span` to start after the hole (and discard curr). self.prev_mut().span = self.prev().span.with_lo(right_cutoff); - debug!(" Mutated prev.span to start after the closure. prev={:?}", self.prev()); + debug!(prev=?self.prev(), "mutated prev to start after the hole"); - // Discard this curr, since it's a closure span. + // Discard this curr, since it's a hole span. let curr = self.take_curr(); - assert!(curr.is_closure); + assert!(curr.is_hole); } } 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 b91ab811918a7..3afad72cfa4c6 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs @@ -52,14 +52,14 @@ pub(super) fn mir_to_initial_sorted_coverage_spans( // - Span A extends further left, or // - Both have the same start and span A extends further right .then_with(|| Ord::cmp(&a.span.hi(), &b.span.hi()).reverse()) - // If two spans have the same lo & hi, put closure spans first, - // as they take precedence over non-closure spans. - .then_with(|| Ord::cmp(&a.is_closure, &b.is_closure).reverse()) + // If two spans have the same lo & hi, put hole spans first, + // as they take precedence over non-hole spans. + .then_with(|| Ord::cmp(&a.is_hole, &b.is_hole).reverse()) // After deduplication, we want to keep only the most-dominated BCB. .then_with(|| basic_coverage_blocks.cmp_in_dominator_order(a.bcb, b.bcb).reverse()) }); - // Among covspans with the same span, keep only one. Closure spans take + // Among covspans with the same span, keep only one. Hole spans take // precedence, otherwise keep the one with the most-dominated BCB. // (Ideally we should try to preserve _all_ non-dominating BCBs, but that // requires a lot more complexity in the span refiner, for little benefit.) @@ -78,8 +78,8 @@ pub(super) fn mir_to_initial_sorted_coverage_spans( fn remove_unwanted_macro_spans(initial_spans: &mut Vec) { let mut seen_macro_spans = FxHashSet::default(); initial_spans.retain(|covspan| { - // Ignore (retain) closure spans and non-macro-expansion spans. - if covspan.is_closure || covspan.visible_macro.is_none() { + // Ignore (retain) hole spans and non-macro-expansion spans. + if covspan.is_hole || covspan.visible_macro.is_none() { return true; } @@ -96,7 +96,7 @@ fn split_visible_macro_spans(initial_spans: &mut Vec) { let mut extra_spans = vec![]; initial_spans.retain(|covspan| { - if covspan.is_closure { + if covspan.is_hole { return true; } @@ -112,7 +112,7 @@ fn split_visible_macro_spans(initial_spans: &mut Vec) { return true; } - assert!(!covspan.is_closure); + assert!(!covspan.is_hole); extra_spans.push(SpanFromMir::new(before, covspan.visible_macro, covspan.bcb, false)); extra_spans.push(SpanFromMir::new(after, covspan.visible_macro, covspan.bcb, false)); false // Discard the original covspan that we just split. @@ -336,7 +336,10 @@ pub(super) struct SpanFromMir { pub(super) span: Span, visible_macro: Option, pub(super) bcb: BasicCoverageBlock, - pub(super) is_closure: bool, + /// If true, this covspan represents a "hole" that should be carved out + /// from other spans, e.g. because it represents a closure expression that + /// will be instrumented separately as its own function. + pub(super) is_hole: bool, } impl SpanFromMir { @@ -348,8 +351,8 @@ impl SpanFromMir { span: Span, visible_macro: Option, bcb: BasicCoverageBlock, - is_closure: bool, + is_hole: bool, ) -> Self { - Self { span, visible_macro, bcb, is_closure } + Self { span, visible_macro, bcb, is_hole } } }