Auto merge of rust-lang#125752 - jieyouxu:kaboom, r=<try>

run-make: enforce `#[must_use]` and arm command wrappers with drop bombs

This PR is one in a series of cleanups to run-make tests and the run-make-support library.

### Summary

It's easy to forget to actually executed constructed command wrappers, e.g. `rustc().input("foo.rs")` but forget the `run()`, so to help catch these mistakes, we:

- Add `#[must_use]` annotations to functions where suitable and compile rmake.rs recipes with `-Dunused_must_use`.
- Arm command wrappers with drop bombs on construction to force them to be executed by test code.

### Details

Especially for command wrappers like `Rustc`, it's very easy to build up
a command invocation but forget to actually execute it, e.g. by using
`run()`. This commit adds "drop bombs" to command wrappers, which are
armed on command wrapper construction, and only defused if the command
is executed (through `run`, `run_fail` or `run_fail_assert_exit_code`).
If the test writer forgets to execute the command, the drop bomb will
"explode" and panic with an error message. This is so that tests don't
silently pass with constructed-but-not-executed command wrappers.

We don't add `#[must_use]` for command wrapper helper methods because
they return `&mut Self` and can be annoying e.g. if a helper method is
conditionally called, such as

```
if condition {
    cmd.arg("-Cprefer-dynamic"); // <- unused_must_use fires
}
cmd.run(); // <- even though cmd is eventually executed
```

This PR is best reviewed commit-by-commit.

Fixes rust-lang#125703.

Because `command_output()` doesn't defuse the drop bomb, it also fixes rust-lang#125617.

try-job: x86_64-msvc

Author: bors

src/tools/compiletest/src/runtest.rs

@@ -3530,6 +3530,10 @@ impl<'test> TestCx<'test> {
             .env_remove("MFLAGS")
             .env_remove("CARGO_MAKEFLAGS");
 
+        // In test code we want to be very pedantic about values being silently discarded that are
+        // annotated with `#[must_use]`.
+        cmd.arg("-Dunused_must_use");
+
         if std::env::var_os("COMPILETEST_FORCE_STAGE0").is_some() {
             let mut stage0_sysroot = build_root.clone();
             stage0_sysroot.push("stage0-sysroot");

src/tools/run-make-support/src/cc.rs

@@ -2,6 +2,7 @@ use std::env;
 use std::path::Path;
 use std::process::Command;
 
+use crate::drop_bomb::DropBomb;
 use crate::{bin_name, cygpath_windows, handle_failed_output, is_msvc, is_windows, tmp_dir, uname};
 
 /// Construct a new platform-specific C compiler invocation.
@@ -14,9 +15,11 @@ pub fn cc() -> Cc {
 
 /// A platform-specific C compiler invocation builder. The specific C compiler used is
 /// passed down from compiletest.
+#[must_use]
 #[derive(Debug)]
 pub struct Cc {
     cmd: Command,
+    drop_bomb: DropBomb,
 }
 
 crate::impl_common_helpers!(Cc);
@@ -36,7 +39,7 @@ impl Cc {
             cmd.arg(flag);
         }
 
-        Self { cmd }
+        Self { cmd, drop_bomb: DropBomb::arm("cc invocation must be executed") }
     }
 
     /// Specify path of the input file.
@@ -94,6 +97,7 @@ impl Cc {
 }
 
 /// `EXTRACFLAGS`
+#[must_use]
 pub fn extra_c_flags() -> Vec<&'static str> {
     // Adapted from tools.mk (trimmed):
     //
@@ -152,6 +156,7 @@ pub fn extra_c_flags() -> Vec<&'static str> {
 }
 
 /// `EXTRACXXFLAGS`
+#[must_use]
 pub fn extra_cxx_flags() -> Vec<&'static str> {
     // Adapted from tools.mk (trimmed):
     //

src/tools/run-make-support/src/clang.rs

@@ -2,6 +2,7 @@ use std::env;
 use std::path::Path;
 use std::process::Command;
 
+use crate::drop_bomb::DropBomb;
 use crate::{bin_name, handle_failed_output, tmp_dir};
 
 /// Construct a new `clang` invocation. `clang` is not always available for all targets.
@@ -10,62 +11,72 @@ pub fn clang() -> Clang {
 }
 
 /// A `clang` invocation builder.
+#[must_use]
 #[derive(Debug)]
 pub struct Clang {
     cmd: Command,
+    drop_bomb: DropBomb,
 }
 
 crate::impl_common_helpers!(Clang);
 
 impl Clang {
     /// Construct a new `clang` invocation. `clang` is not always available for all targets.
+    #[must_use]
     pub fn new() -> Self {
         let clang =
             env::var("CLANG").expect("`CLANG` not specified, but this is required to find `clang`");
         let cmd = Command::new(clang);
-        Self { cmd }
+        Self { cmd, drop_bomb: DropBomb::arm("clang invocation must be executed") }
     }
 
     /// Provide an input file.
+    #[must_use]
     pub fn input<P: AsRef<Path>>(&mut self, path: P) -> &mut Self {
         self.cmd.arg(path.as_ref());
         self
     }
 
     /// Specify the name of the executable. The executable will be placed under `$TMPDIR`, and the
     /// extension will be determined by [`bin_name`].
+    #[must_use]
     pub fn out_exe(&mut self, name: &str) -> &mut Self {
         self.cmd.arg("-o");
         self.cmd.arg(tmp_dir().join(bin_name(name)));
         self
     }
 
     /// Specify which target triple clang should target.
+    #[must_use]
     pub fn target(&mut self, target_triple: &str) -> &mut Self {
         self.cmd.arg("-target");
         self.cmd.arg(target_triple);
         self
     }
 
     /// Pass `-nostdlib` to disable linking the C standard library.
+    #[must_use]
     pub fn no_stdlib(&mut self) -> &mut Self {
         self.cmd.arg("-nostdlib");
         self
     }
 
     /// Specify architecture.
+    #[must_use]
     pub fn arch(&mut self, arch: &str) -> &mut Self {
         self.cmd.arg(format!("-march={arch}"));
         self
     }
 
     /// Specify LTO settings.
+    #[must_use]
     pub fn lto(&mut self, lto: &str) -> &mut Self {
         self.cmd.arg(format!("-flto={lto}"));
         self
     }
 
     /// Specify which ld to use.
+    #[must_use]
     pub fn use_ld(&mut self, ld: &str) -> &mut Self {
         self.cmd.arg(format!("-fuse-ld={ld}"));
         self

src/tools/run-make-support/src/diff/mod.rs

@@ -2,31 +2,37 @@ use regex::Regex;
 use similar::TextDiff;
 use std::path::Path;
 
+use crate::drop_bomb::DropBomb;
+
 #[cfg(test)]
 mod tests;
 
 pub fn diff() -> Diff {
     Diff::new()
 }
 
+#[must_use]
 #[derive(Debug)]
 pub struct Diff {
     expected: Option<String>,
     expected_name: Option<String>,
     actual: Option<String>,
     actual_name: Option<String>,
     normalizers: Vec<(String, String)>,
+    drop_bomb: DropBomb,
 }
 
 impl Diff {
     /// Construct a bare `diff` invocation.
+    #[must_use]
     pub fn new() -> Self {
         Self {
             expected: None,
             expected_name: None,
             actual: None,
             actual_name: None,
             normalizers: Vec::new(),
+            drop_bomb: DropBomb::arm("diff invocation must be executed"),
         }
     }
 
@@ -79,9 +85,9 @@ impl Diff {
         self
     }
 
-    /// Executes the diff process, prints any differences to the standard error.
     #[track_caller]
-    pub fn run(&self) {
+    pub fn run(&mut self) {
+        self.drop_bomb.defuse();
         let expected = self.expected.as_ref().expect("expected text not set");
         let mut actual = self.actual.as_ref().expect("actual text not set").to_string();
         let expected_name = self.expected_name.as_ref().unwrap();

src/tools/run-make-support/src/drop_bomb/mod.rs

@@ -0,0 +1,37 @@
+//! This module implements "drop bombs" intended for use by command wrappers to ensure that the
+//! constructed commands are *eventually* executed. This is exactly like `rustc_errors::Diag`
+//! where we force every `Diag` to be consumed or we emit a bug, but we panic instead.
+//!
+//! This is inspired by <https://docs.rs/drop_bomb/latest/drop_bomb/>.
+
+use std::borrow::Cow;
+
+#[cfg(test)]
+mod tests;
+
+#[derive(Debug)]
+pub(crate) struct DropBomb {
+    msg: Cow<'static, str>,
+    defused: bool,
+}
+
+impl DropBomb {
+    /// Arm a [`DropBomb`]. If the value is dropped without being [`defused`][Self::defused], then
+    /// it will panic.
+    pub(crate) fn arm<S: Into<Cow<'static, str>>>(message: S) -> DropBomb {
+        DropBomb { msg: message.into(), defused: false }
+    }
+
+    /// Defuse the [`DropBomb`]. This will prevent the drop bomb from panicking when dropped.
+    pub(crate) fn defuse(&mut self) {
+        self.defused = true;
+    }
+}
+
+impl Drop for DropBomb {
+    fn drop(&mut self) {
+        if !self.defused && !std::thread::panicking() {
+            panic!("{}", self.msg)
+        }
+    }
+}

src/tools/run-make-support/src/drop_bomb/tests.rs

@@ -0,0 +1,15 @@
+use super::DropBomb;
+
+#[test]
+#[should_panic]
+fn test_arm() {
+    let bomb = DropBomb::arm("hi :3");
+    drop(bomb); // <- armed bomb should explode when not defused
+}
+
+#[test]
+fn test_defuse() {
+    let mut bomb = DropBomb::arm("hi :3");
+    bomb.defuse();
+    drop(bomb); // <- defused bomb should not explode
+}