Rollup merge of #134463 - jieyouxu:filecheck-prefix, r=Zalathar

compiletest: don't register predefined `MSVC`/`NONMSVC` FileCheck prefixes

This was fragile as it was based on host target passed to compiletest,
but the user could cross-compile and run test for a different target
(e.g. cross from linux to msvc, but msvc won't be set on the target).
Furthermore, it was also very surprising as normally revision names
(other than `CHECK`) was accepted as FileCheck prefixes.

This partially reverts the `MSVC`/`NONMSVC` predefined FileCheck
prefix registration introduced historically for some codegen tests.

This makes some codegen tests more verbose since they now need to
explicitly introduce `MSVC`/`NONMSVC` revisions, but I think that's
less surprising, e.g.:

```rs
//@ revisions: MSVC NONMSVC
//`@[MSVC]` only-msvc
//`@[NONMSVC]` ignore-msvc
```

Note that revisions are not *only* FileCheck prefixes in
FileCheck-based test suites, as they also can be used
to conditionally apply certain compiletest directives.

r? `@Zalathar` (or reroll a `r/? compiletest` reviewer)

try-job: x86_64-msvc
try-job: i686-msvc
try-job: x86_64-mingw-1
try-job: i686-mingw

src/tools/compiletest/src/runtest.rs

@@ -1958,23 +1958,23 @@ impl<'test> TestCx<'test> {
         let mut filecheck = Command::new(self.config.llvm_filecheck.as_ref().unwrap());
         filecheck.arg("--input-file").arg(output).arg(&self.testpaths.file);
 
-        // FIXME: Consider making some of these prefix flags opt-in per test,
-        // via `filecheck-flags` or by adding new header directives.
-
         // Because we use custom prefixes, we also have to register the default prefix.
         filecheck.arg("--check-prefix=CHECK");
 
-        // Some tests use the current revision name as a check prefix.
+        // FIXME(#134510): auto-registering revision names as check prefix is a bit sketchy, and
+        // that having to pass `--allow-unused-prefix` is an unfortunate side-effect of not knowing
+        // whether the test author actually wanted revision-specific check prefixes or not.
+        //
+        // TL;DR We may not want to conflate `compiletest` revisions and `FileCheck` prefixes.
+
+        // HACK: tests are allowed to use a revision name as a check prefix.
         if let Some(rev) = self.revision {
             filecheck.arg("--check-prefix").arg(rev);
         }
 
-        // Some tests also expect either the MSVC or NONMSVC prefix to be defined.
-        let msvc_or_not = if self.config.target.contains("msvc") { "MSVC" } else { "NONMSVC" };
-        filecheck.arg("--check-prefix").arg(msvc_or_not);
-
-        // The filecheck tool normally fails if a prefix is defined but not used.
-        // However, we define several prefixes globally for all tests.
+        // HACK: the filecheck tool normally fails if a prefix is defined but not used. However,
+        // sometimes revisions are used to specify *compiletest* directives which are not FileCheck
+        // concerns.
         filecheck.arg("--allow-unused-prefixes");
 
         // Provide more context on failures.

tests/codegen/async-fn-debug-awaitee-field.rs

@@ -1,8 +1,12 @@
-// This test makes sure that the coroutine field capturing the awaitee in a `.await` expression
-// is called "__awaitee" in debuginfo. This name must not be changed since debuggers and debugger
-// extensions rely on the field having this name.
-
 // ignore-tidy-linelength
+//! This test makes sure that the coroutine field capturing the awaitee in a `.await` expression
+//! is called `__awaitee` in debuginfo. This name must not be changed since debuggers and debugger
+//! extensions rely on the field having this name.
+
+//@ revisions: MSVC NONMSVC
+//@[MSVC] only-msvc
+//@[NONMSVC] ignore-msvc
+
 //@ compile-flags: -C debuginfo=2 --edition=2018 -Copt-level=0
 
 #![crate_type = "lib"]

tests/codegen/debug-accessibility/crate-enum.rs

@@ -1,9 +1,11 @@
-//@ compile-flags: -C debuginfo=2
 // ignore-tidy-linelength
+//! Checks that visibility information is present in the debuginfo for crate-visibility enums.
 
-#![allow(dead_code)]
+//@ revisions: MSVC NONMSVC
+//@[MSVC] only-msvc
+//@[NONMSVC] ignore-msvc
 
-// Checks that visibility information is present in the debuginfo for crate-visibility enums.
+//@ compile-flags: -C debuginfo=2
 
 mod module {
     use std::hint::black_box;

tests/codegen/debug-accessibility/private-enum.rs

@@ -1,9 +1,10 @@
-//@ compile-flags: -C debuginfo=2
 // ignore-tidy-linelength
+//! Checks that visibility information is present in the debuginfo for private enums.
 
-#![allow(dead_code)]
-
-// Checks that visibility information is present in the debuginfo for private enums.
+//@ revisions: MSVC NONMSVC
+//@[MSVC] only-msvc
+//@[NONMSVC] ignore-msvc
+//@ compile-flags: -C debuginfo=2
 
 use std::hint::black_box;
 

tests/codegen/debug-accessibility/public-enum.rs

@@ -1,9 +1,11 @@
-//@ compile-flags: -C debuginfo=2
 // ignore-tidy-linelength
+//! Checks that visibility information is present in the debuginfo for types and their fields.
 
-#![allow(dead_code)]
+//@ revisions: MSVC NONMSVC
+//@[MSVC] only-msvc
+//@[NONMSVC] ignore-msvc
 
-// Checks that visibility information is present in the debuginfo for types and their fields.
+//@ compile-flags: -C debuginfo=2
 
 use std::hint::black_box;
 

tests/codegen/debug-accessibility/super-enum.rs

@@ -1,9 +1,10 @@
-//@ compile-flags: -C debuginfo=2
 // ignore-tidy-linelength
+//! Checks that visibility information is present in the debuginfo for super-visibility enums.
 
-#![allow(dead_code)]
-
-// Checks that visibility information is present in the debuginfo for super-visibility enums.
+//@ revisions: MSVC NONMSVC
+//@[MSVC] only-msvc
+//@[NONMSVC] ignore-msvc
+//@ compile-flags: -C debuginfo=2
 
 mod module {
     use std::hint::black_box;

tests/codegen/debug-vtable.rs

@@ -1,5 +1,10 @@
-// This test checks the debuginfo for the expected 3 vtables is generated for correct names and number
-// of entries.
+// ignore-tidy-linelength
+//! This test checks the debuginfo for the expected 3 vtables is generated for correct names and
+//! number of entries.
+
+//@ revisions: MSVC NONMSVC
+//@[MSVC] only-msvc
+//@[NONMSVC] ignore-msvc
 
 // Use the v0 symbol mangling scheme to codegen order independent of rustc version.
 // Unnamed items like shims are generated in lexicographical order of their symbol name and in the
@@ -7,7 +12,6 @@
 // of the name, thus randomizing item order with respect to rustc version.
 
 //@ compile-flags: -Cdebuginfo=2 -Copt-level=0 -Csymbol-mangling-version=v0
-// ignore-tidy-linelength
 
 // Make sure that vtables don't have the unnamed_addr attribute when debuginfo is enabled.
 // This helps debuggers more reliably map from dyn pointer to concrete type.

tests/codegen/debuginfo-generic-closure-env-names.rs

@@ -1,14 +1,17 @@
-// This test checks that we get proper type names for closure environments and
-// async-fn environments in debuginfo, especially making sure that generic arguments
-// of the enclosing functions don't get lost.
-//
-// Unfortunately, the order that debuginfo gets emitted into LLVM IR becomes a bit hard
-// to predict once async fns are involved, so DAG allows any order.
-//
-// Note that the test does not check async-fns when targeting MSVC because debuginfo for
-// those does not follow the enum-fallback encoding yet and thus is incomplete.
-
 // ignore-tidy-linelength
+//! This test checks that we get proper type names for closure environments and
+//! async-fn environments in debuginfo, especially making sure that generic arguments
+//! of the enclosing functions don't get lost.
+//!
+//! Unfortunately, the order that debuginfo gets emitted into LLVM IR becomes a bit hard
+//! to predict once async fns are involved, so DAG allows any order.
+//!
+//! Note that the test does not check async-fns when targeting MSVC because debuginfo for
+//! those does not follow the enum-fallback encoding yet and thus is incomplete.
+
+//@ revisions: MSVC NONMSVC
+//@[MSVC] only-msvc
+//@[NONMSVC] ignore-msvc
 
 // Use the v0 symbol mangling scheme to codegen order independent of rustc version.
 // Unnamed items like shims are generated in lexicographical order of their symbol name and in the

tests/codegen/issues/issue-98678-async.rs

@@ -1,11 +1,13 @@
-// This test verifies the accuracy of emitted file and line debuginfo metadata for async blocks and
-// async functions.
-//
+// ignore-tidy-linelength
+//! This test verifies the accuracy of emitted file and line debuginfo metadata for async blocks and
+//! async functions.
+
+//@ revisions: MSVC NONMSVC
+//@[MSVC] only-msvc
+//@[NONMSVC] ignore-msvc
 //@ edition:2021
 //@ compile-flags: --crate-type=lib -Copt-level=0 -Cdebuginfo=2 -Zdebug-info-type-line-numbers=true
 
-// ignore-tidy-linelength
-
 // NONMSVC-DAG: ![[#FILE:]] = !DIFile({{.*}}filename:{{.*[/\\]}}issue-98678-async.rs{{".*}})
 // MSVC: ![[#FILE:]] = !DIFile({{.*}}filename:{{.*}}\\issue-98678-async.rs{{".*}})
 

tests/codegen/issues/issue-98678-closure-coroutine.rs

@@ -1,10 +1,13 @@
-// This test verifies the accuracy of emitted file and line debuginfo metadata for closures and
-// coroutines.
-//
-//@ compile-flags: --crate-type=lib -Copt-level=0 -Cdebuginfo=2 -Zdebug-info-type-line-numbers=true
+// ignore-tidy-linelength
+//! This test verifies the accuracy of emitted file and line debuginfo metadata for closures and
+//! coroutines.
+
 #![feature(coroutines, stmt_expr_attributes)]
 
-// ignore-tidy-linelength
+//@ revisions: MSVC NONMSVC
+//@[MSVC] only-msvc
+//@[NONMSVC] ignore-msvc
+//@ compile-flags: --crate-type=lib -Copt-level=0 -Cdebuginfo=2 -Zdebug-info-type-line-numbers=true
 
 // NONMSVC-DAG: ![[#FILE:]] = !DIFile({{.*}}filename:{{.*[/\\]}}issue-98678-closure-coroutine.rs{{".*}})
 // MSVC-DAG: ![[#FILE:]] = !DIFile({{.*}}filename:{{.*}}\\issue-98678-closure-coroutine.rs{{".*}})

tests/codegen/issues/issue-98678-enum.rs

@@ -1,8 +1,10 @@
-// This test verifies the accuracy of emitted file and line debuginfo metadata enums.
-//
-//@ compile-flags: --crate-type=lib -Copt-level=0 -Cdebuginfo=2 -Zdebug-info-type-line-numbers=true
-
 // ignore-tidy-linelength
+//! This test verifies the accuracy of emitted file and line debuginfo metadata enums.
+
+//@ revisions: MSVC NONMSVC
+//@[MSVC] only-msvc
+//@[NONMSVC] ignore-msvc
+//@ compile-flags: --crate-type=lib -Copt-level=0 -Cdebuginfo=2 -Zdebug-info-type-line-numbers=true
 
 // NONMSVC: ![[#FILE:]] = !DIFile({{.*}}filename:{{.*[/\\]}}issue-98678-enum.rs{{".*}})
 // MSVC: ![[#FILE:]] = !DIFile({{.*}}filename:{{.*}}\\issue-98678-enum.rs{{".*}})

tests/codegen/issues/issue-98678-struct-union.rs

@@ -1,9 +1,11 @@
-// This test verifies the accuracy of emitted file and line debuginfo metadata for structs and
-// unions.
-//
-//@ compile-flags: --crate-type=lib -Copt-level=0 -Cdebuginfo=2 -Zdebug-info-type-line-numbers=true
-
 // ignore-tidy-linelength
+//! This test verifies the accuracy of emitted file and line debuginfo metadata for structs and
+//! unions.
+
+//@ revisions: MSVC NONMSVC
+//@[MSVC] only-msvc
+//@[NONMSVC] ignore-msvc
+//@ compile-flags: --crate-type=lib -Copt-level=0 -Cdebuginfo=2 -Zdebug-info-type-line-numbers=true
 
 // NONMSVC: ![[#FILE:]] = !DIFile({{.*}}filename:{{.*[/\\]}}issue-98678-struct-union.rs{{".*}})
 // MSVC: ![[#FILE:]] = !DIFile({{.*}}filename:{{.*}}\\issue-98678-struct-union.rs{{".*}})

tests/codegen/meta-filecheck/msvc-prefix-good.rs

@@ -1,7 +0,0 @@
-// One of MSVC or NONMSVC should always be defined, so this test should pass.
-
-// (one of these should always be present)
-
-// MSVC: main
-// NONMSVC: main
-fn main() {}