Rollup merge of rust-lang#111179 - Zalathar:sort-groups, r=Mark-Simulacrum

Fix instrument-coverage tests by using Python to sort instantiation groups

rust-lang#110942 was intended to fix a set of `-Cinstrument-coverage` tests, but it ended up silently *breaking* those tests on Linux, for annoying reasons detailed at rust-lang#111171.

Dealing with `diff --ignore-matching-lines` across multiple platforms has been such a hassle that I've instead written a simple Python script that can detect instantiation groups in the output of `llvm-cov show`, and sort them in a predictable order so that they can be used as snapshots for an ordinary invocation of `diff`.

This approach should be much less error-prone, because it can't accidentally ignore the wrong lines, and any unforeseen problems will tend to result in a Python exception or a failing diff.

Author: matthiaskrgr

tests/run-make/coverage-reports/Makefile

@@ -138,6 +138,7 @@ endif
 			) \
 		2> "$(TMPDIR)"/show_coverage_stderr.$@.txt \
 		| "$(PYTHON)" $(BASEDIR)/normalize_paths.py \
+		| "$(PYTHON)" $(BASEDIR)/sort_subviews.py \
 		> "$(TMPDIR)"/actual_show_coverage.$@.txt || \
 	( status=$$? ; \
 		>&2 cat "$(TMPDIR)"/show_coverage_stderr.$@.txt ; \
@@ -158,28 +159,10 @@ ifdef RUSTC_BLESS_TEST
 else
 	# Compare the show coverage output (`--bless` refreshes `typical` files).
 	#
-	# FIXME(richkadel): None of the Rust test source samples have the
-	# `// ignore-llvm-cov-show-diffs` anymore. This directive exists to work around a limitation
-	# with `llvm-cov show`. When reporting coverage for multiple instantiations of a generic function,
-	# with different type substitutions, `llvm-cov show` prints these in a non-deterministic order,
-	# breaking the `diff` comparison.
-	#
-	# A partial workaround is implemented below, with `diff --ignore-matching-lines=RE`
-	# to ignore each line prefixing each generic instantiation coverage code region.
-	#
-	# This workaround only works if the coverage counts are identical across all reported
-	# instantiations. If there is no way to ensure this, you may need to apply the
-	# `// ignore-llvm-cov-show-diffs` directive, and check for differences using the
-	# `.json` files to validate that results have not changed. (Until then, the JSON
-	# files are redundant, so there is no need to generate `expected_*.json` files or
-	# compare actual JSON results.)
-
-	$(DIFF) --ignore-matching-lines='^  \| .*::<.*>.*:$$' --ignore-matching-lines='^  \| <.*>::.*:$$' \
-		expected_show_coverage.$@.txt "$(TMPDIR)"/actual_show_coverage.$@.txt || \
-		( grep -q '^\/\/ ignore-llvm-cov-show-diffs' $(SOURCEDIR)/$@.rs && \
-			>&2 echo 'diff failed, but suppressed with `// ignore-llvm-cov-show-diffs` in $(SOURCEDIR)/$@.rs' \
-		) || \
-		( >&2 echo 'diff failed, and not suppressed without `// ignore-llvm-cov-show-diffs` in $(SOURCEDIR)/$@.rs'; \
-			false \
-		)
+	# `llvm-cov show` normally prints instantiation groups in an unpredictable
+	# order, but we have used `sort_subviews.py` to sort them, so we can still
+	# check the output directly with `diff`.
+
+	$(DIFF) expected_show_coverage.$@.txt "$(TMPDIR)"/actual_show_coverage.$@.txt || \
+		( >&2 echo 'diff failed in $(SOURCEDIR)/$@.rs'; false)
 endif

tests/run-make/coverage-reports/expected_show_coverage.async.txt

@@ -41,9 +41,9 @@
    41|      1|                    // executed asynchronously.
    42|      1|    match x {
    43|      1|        y if c(x).await == y + 1 => { d().await; }
-                      ^0       ^0                   ^0 ^0
+                      ^0        ^0                  ^0  ^0
    44|      1|        y if f().await == y + 1 => (),
-                      ^0      ^0                 ^0
+                      ^0       ^0                ^0
    45|      1|        _ => (),
    46|       |    }
    47|      1|}

tests/run-make/coverage-reports/expected_show_coverage.sort_groups.txt

@@ -0,0 +1,49 @@
+    1|       |// compile-flags: --edition=2021
+    2|       |
+    3|       |// Demonstrate that `sort_subviews.py` can sort instantiation groups into a
+    4|       |// predictable order, while preserving their heterogeneous contents.
+    5|       |
+    6|      1|fn main() {
+    7|      1|    let cond = std::env::args().len() > 1;
+    8|      1|    generic_fn::<()>(cond);
+    9|      1|    generic_fn::<&'static str>(!cond);
+   10|      1|    if false {
+   11|      0|        generic_fn::<char>(cond);
+   12|      1|    }
+   13|      1|    generic_fn::<i32>(cond);
+   14|      1|    other_fn();
+   15|      1|}
+   16|       |
+   17|      3|fn generic_fn<T>(cond: bool) {
+   18|      3|    if cond {
+   19|      1|        println!("{}", std::any::type_name::<T>());
+   20|      2|    }
+   21|      3|}
+  ------------------
+  | Unexecuted instantiation: sort_groups::generic_fn::<char>
+  ------------------
+  | sort_groups::generic_fn::<&str>:
+  |   17|      1|fn generic_fn<T>(cond: bool) {
+  |   18|      1|    if cond {
+  |   19|      1|        println!("{}", std::any::type_name::<T>());
+  |   20|      1|    }
+  |                   ^0
+  |   21|      1|}
+  ------------------
+  | sort_groups::generic_fn::<()>:
+  |   17|      1|fn generic_fn<T>(cond: bool) {
+  |   18|      1|    if cond {
+  |   19|      0|        println!("{}", std::any::type_name::<T>());
+  |   20|      1|    }
+  |   21|      1|}
+  ------------------
+  | sort_groups::generic_fn::<i32>:
+  |   17|      1|fn generic_fn<T>(cond: bool) {
+  |   18|      1|    if cond {
+  |   19|      0|        println!("{}", std::any::type_name::<T>());
+  |   20|      1|    }
+  |   21|      1|}
+  ------------------
+   22|       |
+   23|      1|fn other_fn() {}
+

tests/run-make/coverage-reports/expected_show_coverage.uses_crate.txt

@@ -19,29 +19,29 @@
    18|      2|    println!("used_only_from_bin_crate_generic_function with {:?}", arg);
    19|      2|}
   ------------------
-  | used_crate::used_only_from_bin_crate_generic_function::<&str>:
+  | Unexecuted instantiation: used_crate::used_only_from_bin_crate_generic_function::<_>
+  ------------------
+  | used_crate::used_only_from_bin_crate_generic_function::<&alloc::vec::Vec<i32>>:
   |   17|      1|pub fn used_only_from_bin_crate_generic_function<T: Debug>(arg: T) {
   |   18|      1|    println!("used_only_from_bin_crate_generic_function with {:?}", arg);
   |   19|      1|}
   ------------------
-  | used_crate::used_only_from_bin_crate_generic_function::<&alloc::vec::Vec<i32>>:
+  | used_crate::used_only_from_bin_crate_generic_function::<&str>:
   |   17|      1|pub fn used_only_from_bin_crate_generic_function<T: Debug>(arg: T) {
   |   18|      1|    println!("used_only_from_bin_crate_generic_function with {:?}", arg);
   |   19|      1|}
-  ------------------
-  | Unexecuted instantiation: used_crate::used_only_from_bin_crate_generic_function::<_>
   ------------------
    20|       |// Expect for above function: `Unexecuted instantiation` (see below)
    21|      2|pub fn used_only_from_this_lib_crate_generic_function<T: Debug>(arg: T) {
    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::<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|}
   ------------------
-  | 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|}
@@ -51,12 +51,12 @@
    26|      2|    println!("used_from_bin_crate_and_lib_crate_generic_function with {:?}", arg);
    27|      2|}
   ------------------
-  | used_crate::used_from_bin_crate_and_lib_crate_generic_function::<alloc::vec::Vec<i32>>:
+  | used_crate::used_from_bin_crate_and_lib_crate_generic_function::<&str>:
   |   25|      1|pub fn used_from_bin_crate_and_lib_crate_generic_function<T: Debug>(arg: T) {
   |   26|      1|    println!("used_from_bin_crate_and_lib_crate_generic_function with {:?}", arg);
   |   27|      1|}
   ------------------
-  | used_crate::used_from_bin_crate_and_lib_crate_generic_function::<&str>:
+  | used_crate::used_from_bin_crate_and_lib_crate_generic_function::<alloc::vec::Vec<i32>>:
   |   25|      1|pub fn used_from_bin_crate_and_lib_crate_generic_function<T: Debug>(arg: T) {
   |   26|      1|    println!("used_from_bin_crate_and_lib_crate_generic_function with {:?}", arg);
   |   27|      1|}

tests/run-make/coverage-reports/expected_show_coverage.uses_inline_crate.txt

@@ -42,6 +42,8 @@
    40|      2|    println!("used_only_from_bin_crate_generic_function with {:?}", arg);
    41|      2|}
   ------------------
+  | Unexecuted instantiation: used_inline_crate::used_only_from_bin_crate_generic_function::<_>
+  ------------------
   | used_inline_crate::used_only_from_bin_crate_generic_function::<&alloc::vec::Vec<i32>>:
   |   39|      1|pub fn used_only_from_bin_crate_generic_function<T: Debug>(arg: T) {
   |   40|      1|    println!("used_only_from_bin_crate_generic_function with {:?}", arg);
@@ -51,8 +53,6 @@
   |   39|      1|pub fn used_only_from_bin_crate_generic_function<T: Debug>(arg: T) {
   |   40|      1|    println!("used_only_from_bin_crate_generic_function with {:?}", arg);
   |   41|      1|}
-  ------------------
-  | Unexecuted instantiation: used_inline_crate::used_only_from_bin_crate_generic_function::<_>
   ------------------
    42|       |// Expect for above function: `Unexecuted instantiation` (see notes in `used_crate.rs`)
    43|       |
@@ -77,15 +77,15 @@
    51|      3|    println!("used_from_bin_crate_and_lib_crate_generic_function with {:?}", arg);
    52|      3|}
   ------------------
-  | used_inline_crate::used_from_bin_crate_and_lib_crate_generic_function::<alloc::vec::Vec<i32>>:
-  |   50|      1|pub fn used_from_bin_crate_and_lib_crate_generic_function<T: Debug>(arg: T) {
-  |   51|      1|    println!("used_from_bin_crate_and_lib_crate_generic_function with {:?}", arg);
-  |   52|      1|}
-  ------------------
   | used_inline_crate::used_from_bin_crate_and_lib_crate_generic_function::<&str>:
   |   50|      2|pub fn used_from_bin_crate_and_lib_crate_generic_function<T: Debug>(arg: T) {
   |   51|      2|    println!("used_from_bin_crate_and_lib_crate_generic_function with {:?}", arg);
   |   52|      2|}
+  ------------------
+  | used_inline_crate::used_from_bin_crate_and_lib_crate_generic_function::<alloc::vec::Vec<i32>>:
+  |   50|      1|pub fn used_from_bin_crate_and_lib_crate_generic_function<T: Debug>(arg: T) {
+  |   51|      1|    println!("used_from_bin_crate_and_lib_crate_generic_function with {:?}", arg);
+  |   52|      1|}
   ------------------
    53|       |
    54|       |#[inline(always)]

tests/run-make/coverage-reports/sort_subviews.py

@@ -0,0 +1,50 @@
+#!/usr/bin/env python3
+
+# `llvm-cov show` prints grouped subviews (e.g. for generic functions) in an
+# unstable order, which is inconvenient when checking output snapshots with
+# `diff`. To work around that, this script detects consecutive subviews in its
+# piped input, and sorts them while preserving their contents.
+
+from __future__ import print_function
+
+import sys
+
+
+def main():
+    subviews = []
+
+    def flush_subviews():
+        if not subviews:
+            return
+
+        # The last "subview" should be just a boundary line on its own, so
+        # temporarily remove it before sorting the accumulated subviews.
+        terminator = subviews.pop()
+        subviews.sort()
+        subviews.append(terminator)
+
+        for view in subviews:
+            for line in view:
+                print(line, end="")
+
+        subviews.clear()
+
+    for line in sys.stdin:
+        if line.startswith("  ------------------"):
+            # This is a subview boundary line, so start a new subview.
+            subviews.append([line])
+        elif line.startswith("  |"):
+            # Add this line to the current subview.
+            subviews[-1].append(line)
+        else:
+            # This line is not part of a subview, so sort and print any
+            # accumulated subviews, and then print the line as-is.
+            flush_subviews()
+            print(line, end="")
+
+    flush_subviews()
+    assert not subviews
+
+
+if __name__ == "__main__":
+    main()

tests/run-make/coverage/sort_groups.rs

@@ -0,0 +1,23 @@
+// compile-flags: --edition=2021
+
+// Demonstrate that `sort_subviews.py` can sort instantiation groups into a
+// predictable order, while preserving their heterogeneous contents.
+
+fn main() {
+    let cond = std::env::args().len() > 1;
+    generic_fn::<()>(cond);
+    generic_fn::<&'static str>(!cond);
+    if false {
+        generic_fn::<char>(cond);
+    }
+    generic_fn::<i32>(cond);
+    other_fn();
+}
+
+fn generic_fn<T>(cond: bool) {
+    if cond {
+        println!("{}", std::any::type_name::<T>());
+    }
+}
+
+fn other_fn() {}

tests/run-make/coverage/uses_crate.rs

@@ -1,6 +1,3 @@
-// FIXME #110395
-// ignore-llvm-cov-show-diffs
-
 #![allow(unused_assignments, unused_variables)]
 // compile-flags: -C opt-level=3 # validates coverage now works with optimizations
 extern crate used_crate;

tests/run-make/coverage/uses_inline_crate.rs

@@ -1,6 +1,3 @@
-// FIXME #110395
-// ignore-llvm-cov-show-diffs
-
 #![allow(unused_assignments, unused_variables)]
 
 // compile-flags: -C opt-level=3 # validates coverage now works with optimizations