Skip to content

Commit 0312bf5

Browse files
richkadelrichkadel
committed
Rebuilt out of date tests and fixed an old bug now exposed
1 parent eef546a commit 0312bf5

File tree

8 files changed

+275
-13
lines changed

8 files changed

+275
-13
lines changed

‎compiler/rustc_mir/src/transform/coverage/spans.rs‎

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -243,14 +243,18 @@ pub struct CoverageSpans<'a, 'tcx> {
243243
/// iteration.
244244
some_curr: Option<CoverageSpan>,
245245

246-
/// The original `span` for `curr`, in case the `curr` span is modified.
246+
/// The original `span` for `curr`, in case `curr.span()` is modified. The `curr_original_span`
247+
/// **must not be mutated** (except when advancing to the next `curr`), even if `curr.span()`
248+
/// is mutated.
247249
curr_original_span: Span,
248250

249251
/// The CoverageSpan from a prior iteration; typically assigned from that iteration's `curr`.
250252
/// If that `curr` was discarded, `prev` retains its value from the previous iteration.
251253
some_prev: Option<CoverageSpan>,
252254

253-
/// Assigned from `curr_original_span` from the previous iteration.
255+
/// Assigned from `curr_original_span` from the previous iteration. The `prev_original_span`
256+
/// **must not be mutated** (except when advancing to the next `prev`), even if `prev.span()`
257+
/// is mutated.
254258
prev_original_span: Span,
255259

256260
/// A copy of the expn_span from the prior iteration.
@@ -400,8 +404,12 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
400404
} else if self.curr().is_closure {
401405
self.carve_out_span_for_closure();
402406
} else if self.prev_original_span == self.curr().span {
403-
// Note that this compares the new span to `prev_original_span`, which may not
404-
// be the full `prev.span` (if merged during the previous iteration).
407+
// Note that this compares the new (`curr`) span to `prev_original_span`.
408+
// In this branch, the actual span byte range of `prev_original_span` is not
409+
// important. What is important is knowing whether the new `curr` span was
410+
// **originally** the same as the original span of `prev()`. The original spans
411+
// reflect their original sort order, and for equal spans, conveys a partial
412+
// ordering based on CFG dominator priority.
405413
if self.prev().is_macro_expansion() && self.curr().is_macro_expansion() {
406414
// Macros that expand to include branching (such as
407415
// `assert_eq!()`, `assert_ne!()`, `info!()`, `debug!()`, or
@@ -663,10 +671,13 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
663671
self.push_refined_span(pre_closure);
664672
}
665673
if has_post_closure_span {
666-
// Update prev.span to start after the closure (and discard curr)
674+
// Mutate `prev.span()` to start after the closure (and discard curr).
675+
// (**NEVER** update `prev_original_span` because it affects the assumptions
676+
// about how the `CoverageSpan`s are ordered.)
667677
self.prev_mut().span = self.prev().span.with_lo(right_cutoff);
668-
self.prev_original_span = self.prev().span;
678+
debug!(" Mutated prev.span to start after the closure. prev={:?}", self.prev());
669679
for dup in pending_dups.iter_mut() {
680+
debug!(" ...and at least one overlapping dup={:?}", dup);
670681
dup.span = dup.span.with_lo(right_cutoff);
671682
}
672683
self.pending_dups.append(&mut pending_dups);
@@ -678,8 +689,14 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
678689
}
679690

680691
/// Called if `curr.span` equals `prev_original_span` (and potentially equal to all
681-
/// `pending_dups` spans, if any); but keep in mind, `prev.span` may start at a `Span.lo()` that
682-
/// is less than (further left of) `prev_original_span.lo()`.
692+
/// `pending_dups` spans, if any). Keep in mind, `prev.span()` may have been changed.
693+
/// If prev.span() was merged into other spans (with matching BCB, for instance),
694+
/// `prev.span.hi()` will be greater than (further right of) `prev_original_span.hi()`.
695+
/// If prev.span() was split off to the right of a closure, prev.span().lo() will be
696+
/// greater than prev_original_span.lo(). The actual span of `prev_original_span` is
697+
/// not as important as knowing that `prev()` **used to have the same span** as `curr(),
698+
/// which means their sort order is still meaningful for determinating the dominator
699+
/// relationship.
683700
///
684701
/// When two `CoverageSpan`s have the same `Span`, dominated spans can be discarded; but if
685702
/// neither `CoverageSpan` dominates the other, both (or possibly more than two) are held,

‎src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.closure_macro.txt‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
1| |// compile-flags: --edition=2018
2+
2| |#![feature(no_coverage)]
3+
3| |
4+
4| |macro_rules! bail {
5+
5| | ($msg:literal $(,)?) => {
6+
6| | if $msg.len() > 0 {
7+
7| | println!("no msg");
8+
8| | } else {
9+
9| | println!($msg);
10+
10| | }
11+
11| | return Err(String::from($msg));
12+
12| | };
13+
13| |}
14+
14| |
15+
15| |macro_rules! on_error {
16+
16| | ($value:expr, $error_message:expr) => {
17+
17| 0| $value.or_else(|e| {
18+
18| 0| let message = format!($error_message, e);
19+
19| 0| if message.len() > 0 {
20+
20| 0| println!("{}", message);
21+
21| 0| Ok(String::from("ok"))
22+
22| | } else {
23+
23| 0| bail!("error");
24+
24| | }
25+
25| 0| })
26+
26| | };
27+
27| |}
28+
28| |
29+
29| 1|fn load_configuration_files() -> Result<String, String> {
30+
30| 1| Ok(String::from("config"))
31+
31| 1|}
32+
32| |
33+
33| 1|pub fn main() -> Result<(), String> {
34+
34| 1| println!("Starting service");
35+
35| 1| let config = on_error!(load_configuration_files(), "Error loading configs: {}")?;
36+
^0
37+
36| |
38+
37| 1| let startup_delay_duration = String::from("arg");
39+
38| 1| let _ = (config, startup_delay_duration);
40+
39| 1| Ok(())
41+
40| 1|}
42+

‎src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.closure_macro_async.txt‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
1| |// compile-flags: --edition=2018
2+
2| |#![feature(no_coverage)]
3+
3| |
4+
4| |macro_rules! bail {
5+
5| | ($msg:literal $(,)?) => {
6+
6| | if $msg.len() > 0 {
7+
7| | println!("no msg");
8+
8| | } else {
9+
9| | println!($msg);
10+
10| | }
11+
11| | return Err(String::from($msg));
12+
12| | };
13+
13| |}
14+
14| |
15+
15| |macro_rules! on_error {
16+
16| | ($value:expr, $error_message:expr) => {
17+
17| 0| $value.or_else(|e| {
18+
18| 0| let message = format!($error_message, e);
19+
19| 0| if message.len() > 0 {
20+
20| 0| println!("{}", message);
21+
21| 0| Ok(String::from("ok"))
22+
22| | } else {
23+
23| 0| bail!("error");
24+
24| | }
25+
25| 0| })
26+
26| | };
27+
27| |}
28+
28| |
29+
29| 1|fn load_configuration_files() -> Result<String, String> {
30+
30| 1| Ok(String::from("config"))
31+
31| 1|}
32+
32| |
33+
33| 1|pub async fn test() -> Result<(), String> {
34+
34| 1| println!("Starting service");
35+
35| 1| let config = on_error!(load_configuration_files(), "Error loading configs: {}")?;
36+
^0
37+
36| |
38+
37| 1| let startup_delay_duration = String::from("arg");
39+
38| 1| let _ = (config, startup_delay_duration);
40+
39| 1| Ok(())
41+
40| 1|}
42+
41| |
43+
42| |#[no_coverage]
44+
43| |fn main() {
45+
44| | executor::block_on(test());
46+
45| |}
47+
46| |
48+
47| |mod executor {
49+
48| | use core::{
50+
49| | future::Future,
51+
50| | pin::Pin,
52+
51| | task::{Context, Poll, RawWaker, RawWakerVTable, Waker},
53+
52| | };
54+
53| |
55+
54| | #[no_coverage]
56+
55| | pub fn block_on<F: Future>(mut future: F) -> F::Output {
57+
56| | let mut future = unsafe { Pin::new_unchecked(&mut future) };
58+
57| | use std::hint::unreachable_unchecked;
59+
58| | static VTABLE: RawWakerVTable = RawWakerVTable::new(
60+
59| |
61+
60| | #[no_coverage]
62+
61| | |_| unsafe { unreachable_unchecked() }, // clone
63+
62| |
64+
63| | #[no_coverage]
65+
64| | |_| unsafe { unreachable_unchecked() }, // wake
66+
65| |
67+
66| | #[no_coverage]
68+
67| | |_| unsafe { unreachable_unchecked() }, // wake_by_ref
69+
68| |
70+
69| | #[no_coverage]
71+
70| | |_| (),
72+
71| | );
73+
72| | let waker = unsafe { Waker::from_raw(RawWaker::new(core::ptr::null(), &VTABLE)) };
74+
73| | let mut context = Context::from_waker(&waker);
75+
74| |
76+
75| | loop {
77+
76| | if let Poll::Ready(val) = future.as_mut().poll(&mut context) {
78+
77| | break val;
79+
78| | }
80+
79| | }
81+
80| | }
82+
81| |}
83+

‎src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-83601.txt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,9 @@
1212
5| |
1313
6| 1|fn main() {
1414
7| 1| let bar = Foo(1);
15-
8| 0| assert_eq!(bar, Foo(1));
15+
8| 1| assert_eq!(bar, Foo(1));
1616
9| 1| let baz = Foo(0);
17-
10| 0| assert_ne!(baz, Foo(1));
17+
10| 1| assert_ne!(baz, Foo(1));
1818
11| 1| println!("{:?}", Foo(1));
1919
12| 1| println!("{:?}", bar);
2020
13| 1| println!("{:?}", baz);

‎src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-84561.txt‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
2| |
33
3| |// expect-exit-status-101
44
4| 21|#[derive(PartialEq, Eq)]
5-
^0
65
------------------
76
| <issue_84561::Foo as core::cmp::PartialEq>::eq:
87
| 4| 21|#[derive(PartialEq, Eq)]

‎src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_crate.txt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,12 @@
3636
22| 2| println!("used_only_from_this_lib_crate_generic_function with {:?}", arg);
3737
23| 2|}
3838
------------------
39-
| used_crate::used_only_from_this_lib_crate_generic_function::<&str>:
39+
| used_crate::used_only_from_this_lib_crate_generic_function::<alloc::vec::Vec<i32>>:
4040
| 21| 1|pub fn used_only_from_this_lib_crate_generic_function<T: Debug>(arg: T) {
4141
| 22| 1| println!("used_only_from_this_lib_crate_generic_function with {:?}", arg);
4242
| 23| 1|}
4343
------------------
44-
| used_crate::used_only_from_this_lib_crate_generic_function::<alloc::vec::Vec<i32>>:
44+
| used_crate::used_only_from_this_lib_crate_generic_function::<&str>:
4545
| 21| 1|pub fn used_only_from_this_lib_crate_generic_function<T: Debug>(arg: T) {
4646
| 22| 1| println!("used_only_from_this_lib_crate_generic_function with {:?}", arg);
4747
| 23| 1|}

‎src/test/run-make-fulldeps/coverage/closure_macro.rs‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// compile-flags: --edition=2018
2+
#![feature(no_coverage)]
3+
4+
macro_rules! bail {
5+
($msg:literal $(,)?) => {
6+
if $msg.len() > 0 {
7+
println!("no msg");
8+
} else {
9+
println!($msg);
10+
}
11+
return Err(String::from($msg));
12+
};
13+
}
14+
15+
macro_rules! on_error {
16+
($value:expr, $error_message:expr) => {
17+
$value.or_else(|e| {
18+
let message = format!($error_message, e);
19+
if message.len() > 0 {
20+
println!("{}", message);
21+
Ok(String::from("ok"))
22+
} else {
23+
bail!("error");
24+
}
25+
})
26+
};
27+
}
28+
29+
fn load_configuration_files() -> Result<String, String> {
30+
Ok(String::from("config"))
31+
}
32+
33+
pub fn main() -> Result<(), String> {
34+
println!("Starting service");
35+
let config = on_error!(load_configuration_files(), "Error loading configs: {}")?;
36+
37+
let startup_delay_duration = String::from("arg");
38+
let _ = (config, startup_delay_duration);
39+
Ok(())
40+
}

‎src/test/run-make-fulldeps/coverage/closure_macro_async.rs‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
// compile-flags: --edition=2018
2+
#![feature(no_coverage)]
3+
4+
macro_rules! bail {
5+
($msg:literal $(,)?) => {
6+
if $msg.len() > 0 {
7+
println!("no msg");
8+
} else {
9+
println!($msg);
10+
}
11+
return Err(String::from($msg));
12+
};
13+
}
14+
15+
macro_rules! on_error {
16+
($value:expr, $error_message:expr) => {
17+
$value.or_else(|e| {
18+
let message = format!($error_message, e);
19+
if message.len() > 0 {
20+
println!("{}", message);
21+
Ok(String::from("ok"))
22+
} else {
23+
bail!("error");
24+
}
25+
})
26+
};
27+
}
28+
29+
fn load_configuration_files() -> Result<String, String> {
30+
Ok(String::from("config"))
31+
}
32+
33+
pub async fn test() -> Result<(), String> {
34+
println!("Starting service");
35+
let config = on_error!(load_configuration_files(), "Error loading configs: {}")?;
36+
37+
let startup_delay_duration = String::from("arg");
38+
let _ = (config, startup_delay_duration);
39+
Ok(())
40+
}
41+
42+
#[no_coverage]
43+
fn main() {
44+
executor::block_on(test());
45+
}
46+
47+
mod executor {
48+
use core::{
49+
future::Future,
50+
pin::Pin,
51+
task::{Context, Poll, RawWaker, RawWakerVTable, Waker},
52+
};
53+
54+
#[no_coverage]
55+
pub fn block_on<F: Future>(mut future: F) -> F::Output {
56+
let mut future = unsafe { Pin::new_unchecked(&mut future) };
57+
use std::hint::unreachable_unchecked;
58+
static VTABLE: RawWakerVTable = RawWakerVTable::new(
59+
60+
#[no_coverage]
61+
|_| unsafe { unreachable_unchecked() }, // clone
62+
63+
#[no_coverage]
64+
|_| unsafe { unreachable_unchecked() }, // wake
65+
66+
#[no_coverage]
67+
|_| unsafe { unreachable_unchecked() }, // wake_by_ref
68+
69+
#[no_coverage]
70+
|_| (),
71+
);
72+
let waker = unsafe { Waker::from_raw(RawWaker::new(core::ptr::null(), &VTABLE)) };
73+
let mut context = Context::from_waker(&waker);
74+
75+
loop {
76+
if let Poll::Ready(val) = future.as_mut().poll(&mut context) {
77+
break val;
78+
}
79+
}
80+
}
81+
}

0 commit comments

Comments
 (0)