Rebuilt out of date tests and fixed an old bug now exposed

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

@@ -243,14 +243,18 @@ pub struct CoverageSpans<'a, 'tcx> {
     /// iteration.
     some_curr: Option<CoverageSpan>,
 
-    /// The original `span` for `curr`, in case the `curr` span is modified.
+    /// The original `span` for `curr`, in case `curr.span()` is modified. The `curr_original_span`
+    /// **must not be mutated** (except when advancing to the next `curr`), even if `curr.span()`
+    /// is mutated.
     curr_original_span: Span,
 
     /// The CoverageSpan from a prior iteration; typically assigned from that iteration's `curr`.
     /// If that `curr` was discarded, `prev` retains its value from the previous iteration.
     some_prev: Option<CoverageSpan>,
 
-    /// Assigned from `curr_original_span` from the previous iteration.
+    /// Assigned from `curr_original_span` from the previous iteration. The `prev_original_span`
+    /// **must not be mutated** (except when advancing to the next `prev`), even if `prev.span()`
+    /// is mutated.
     prev_original_span: Span,
 
     /// A copy of the expn_span from the prior iteration.
@@ -400,8 +404,12 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
             } else if self.curr().is_closure {
                 self.carve_out_span_for_closure();
             } else if self.prev_original_span == self.curr().span {
-                // Note that this compares the new span to `prev_original_span`, which may not
-                // be the full `prev.span` (if merged during the previous iteration).
+                // 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 self.prev().is_macro_expansion() && self.curr().is_macro_expansion() {
                     // Macros that expand to include branching (such as
                     // `assert_eq!()`, `assert_ne!()`, `info!()`, `debug!()`, or
@@ -663,10 +671,13 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
             self.push_refined_span(pre_closure);
         }
         if has_post_closure_span {
-            // Update prev.span to start after the closure (and discard curr)
+            // Mutate `prev.span()` to start after the closure (and discard curr).
+            // (**NEVER** update `prev_original_span` because it affects the assumptions
+            // about how the `CoverageSpan`s are ordered.)
             self.prev_mut().span = self.prev().span.with_lo(right_cutoff);
-            self.prev_original_span = self.prev().span;
+            debug!("  Mutated prev.span to start after the closure. prev={:?}", self.prev());
             for dup in pending_dups.iter_mut() {
+                debug!("    ...and at least one overlapping dup={:?}", dup);
                 dup.span = dup.span.with_lo(right_cutoff);
             }
             self.pending_dups.append(&mut pending_dups);
@@ -678,8 +689,14 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
     }
 
     /// Called if `curr.span` equals `prev_original_span` (and potentially equal to all
-    /// `pending_dups` spans, if any); but keep in mind, `prev.span` may start at a `Span.lo()` that
-    /// is less than (further left of) `prev_original_span.lo()`.
+    /// `pending_dups` spans, if any). Keep in mind, `prev.span()` may have been changed.
+    /// If prev.span() was merged into other spans (with matching BCB, for instance),
+    /// `prev.span.hi()` will be greater than (further right of) `prev_original_span.hi()`.
+    /// If prev.span() was split off to the right of a closure, prev.span().lo() will be
+    /// greater than prev_original_span.lo(). The actual span of `prev_original_span` is
+    /// not as important as knowing that `prev()` **used to have the same span** as `curr(),
+    /// which means their sort order is still meaningful for determinating the dominator
+    /// relationship.
     ///
     /// When two `CoverageSpan`s have the same `Span`, dominated spans can be discarded; but if
     /// 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

@@ -0,0 +1,42 @@
+    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|      0|        $value.or_else(|e| {
+   18|      0|            let message = format!($error_message, e);
+   19|      0|            if message.len() > 0 {
+   20|      0|                println!("{}", message);
+   21|      0|                Ok(String::from("ok"))
+   22|       |            } else {
+   23|      0|                bail!("error");
+   24|       |            }
+   25|      0|        })
+   26|       |    };
+   27|       |}
+   28|       |
+   29|      1|fn load_configuration_files() -> Result<String, String> {
+   30|      1|    Ok(String::from("config"))
+   31|      1|}
+   32|       |
+   33|      1|pub fn main() -> Result<(), String> {
+   34|      1|    println!("Starting service");
+   35|      1|    let config = on_error!(load_configuration_files(), "Error loading configs: {}")?;
+                                                                                                 ^0
+   36|       |
+   37|      1|    let startup_delay_duration = String::from("arg");
+   38|      1|    let _ = (config, startup_delay_duration);
+   39|      1|    Ok(())
+   40|      1|}
+

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

@@ -0,0 +1,83 @@
+    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|      0|        $value.or_else(|e| {
+   18|      0|            let message = format!($error_message, e);
+   19|      0|            if message.len() > 0 {
+   20|      0|                println!("{}", message);
+   21|      0|                Ok(String::from("ok"))
+   22|       |            } else {
+   23|      0|                bail!("error");
+   24|       |            }
+   25|      0|        })
+   26|       |    };
+   27|       |}
+   28|       |
+   29|      1|fn load_configuration_files() -> Result<String, String> {
+   30|      1|    Ok(String::from("config"))
+   31|      1|}
+   32|       |
+   33|      1|pub async fn test() -> Result<(), String> {
+   34|      1|    println!("Starting service");
+   35|      1|    let config = on_error!(load_configuration_files(), "Error loading configs: {}")?;
+                                                                                                 ^0
+   36|       |
+   37|      1|    let startup_delay_duration = String::from("arg");
+   38|      1|    let _ = (config, startup_delay_duration);
+   39|      1|    Ok(())
+   40|      1|}
+   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|       |}
+

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

@@ -12,9 +12,9 @@
     5|       |
     6|      1|fn main() {
     7|      1|    let bar = Foo(1);
-    8|      0|    assert_eq!(bar, Foo(1));
+    8|      1|    assert_eq!(bar, Foo(1));
     9|      1|    let baz = Foo(0);
-   10|      0|    assert_ne!(baz, Foo(1));
+   10|      1|    assert_ne!(baz, Foo(1));
    11|      1|    println!("{:?}", Foo(1));
    12|      1|    println!("{:?}", bar);
    13|      1|    println!("{:?}", baz);

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

@@ -2,7 +2,6 @@
     2|       |
     3|       |// expect-exit-status-101
     4|     21|#[derive(PartialEq, Eq)]
-                                  ^0
   ------------------
   | <issue_84561::Foo as core::cmp::PartialEq>::eq:
   |    4|     21|#[derive(PartialEq, Eq)]

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

@@ -36,12 +36,12 @@
    22|      2|    println!("used_only_from_this_lib_crate_generic_function with {:?}", arg);
    23|      2|}
   ------------------
-  | used_crate::used_only_from_this_lib_crate_generic_function::<&str>:
+  | used_crate::used_only_from_this_lib_crate_generic_function::<alloc::vec::Vec<i32>>:
   |   21|      1|pub fn used_only_from_this_lib_crate_generic_function<T: Debug>(arg: T) {
   |   22|      1|    println!("used_only_from_this_lib_crate_generic_function with {:?}", arg);
   |   23|      1|}
   ------------------
-  | used_crate::used_only_from_this_lib_crate_generic_function::<alloc::vec::Vec<i32>>:
+  | used_crate::used_only_from_this_lib_crate_generic_function::<&str>:
   |   21|      1|pub fn used_only_from_this_lib_crate_generic_function<T: Debug>(arg: T) {
   |   22|      1|    println!("used_only_from_this_lib_crate_generic_function with {:?}", arg);
   |   23|      1|}

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

@@ -0,0 +1,40 @@
+// compile-flags: --edition=2018
+#![feature(no_coverage)]
+
+macro_rules! bail {
+    ($msg:literal $(,)?) => {
+        if $msg.len() > 0 {
+            println!("no msg");
+        } else {
+            println!($msg);
+        }
+        return Err(String::from($msg));
+    };
+}
+
+macro_rules! on_error {
+    ($value:expr, $error_message:expr) => {
+        $value.or_else(|e| {
+            let message = format!($error_message, e);
+            if message.len() > 0 {
+                println!("{}", message);
+                Ok(String::from("ok"))
+            } else {
+                bail!("error");
+            }
+        })
+    };
+}
+
+fn load_configuration_files() -> Result<String, String> {
+    Ok(String::from("config"))
+}
+
+pub fn main() -> Result<(), String> {
+    println!("Starting service");
+    let config = on_error!(load_configuration_files(), "Error loading configs: {}")?;
+
+    let startup_delay_duration = String::from("arg");
+    let _ = (config, startup_delay_duration);
+    Ok(())
+}

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

@@ -0,0 +1,81 @@
+// compile-flags: --edition=2018
+#![feature(no_coverage)]
+
+macro_rules! bail {
+    ($msg:literal $(,)?) => {
+        if $msg.len() > 0 {
+            println!("no msg");
+        } else {
+            println!($msg);
+        }
+        return Err(String::from($msg));
+    };
+}
+
+macro_rules! on_error {
+    ($value:expr, $error_message:expr) => {
+        $value.or_else(|e| {
+            let message = format!($error_message, e);
+            if message.len() > 0 {
+                println!("{}", message);
+                Ok(String::from("ok"))
+            } else {
+                bail!("error");
+            }
+        })
+    };
+}
+
+fn load_configuration_files() -> Result<String, String> {
+    Ok(String::from("config"))
+}
+
+pub async fn test() -> Result<(), String> {
+    println!("Starting service");
+    let config = on_error!(load_configuration_files(), "Error loading configs: {}")?;
+
+    let startup_delay_duration = String::from("arg");
+    let _ = (config, startup_delay_duration);
+    Ok(())
+}
+
+#[no_coverage]
+fn main() {
+    executor::block_on(test());
+}
+
+mod executor {
+    use core::{
+        future::Future,
+        pin::Pin,
+        task::{Context, Poll, RawWaker, RawWakerVTable, Waker},
+    };
+
+    #[no_coverage]
+    pub fn block_on<F: Future>(mut future: F) -> F::Output {
+        let mut future = unsafe { Pin::new_unchecked(&mut future) };
+        use std::hint::unreachable_unchecked;
+        static VTABLE: RawWakerVTable = RawWakerVTable::new(
+
+            #[no_coverage]
+            |_| unsafe { unreachable_unchecked() }, // clone
+
+            #[no_coverage]
+            |_| unsafe { unreachable_unchecked() }, // wake
+
+            #[no_coverage]
+            |_| unsafe { unreachable_unchecked() }, // wake_by_ref
+
+            #[no_coverage]
+            |_| (),
+        );
+        let waker = unsafe { Waker::from_raw(RawWaker::new(core::ptr::null(), &VTABLE)) };
+        let mut context = Context::from_waker(&waker);
+
+        loop {
+            if let Poll::Ready(val) = future.as_mut().poll(&mut context) {
+                break val;
+            }
+        }
+    }
+}