about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2024-06-13 07:26:21 +0000
committerbors <bors@rust-lang.org>2024-06-13 07:26:21 +0000
commit921645c737f1d6d107a0a10ca5ee129d364dcd7a (patch)
tree0a6893f08ea9007b677b95f140eeff76ee5670c0
parent56e112afb6bd92698fca44a83b4f116b3e51552c (diff)
parent5b126ed35857154274fe7f2e69c19b7c74a5fc30 (diff)
downloadrust-921645c737f1d6d107a0a10ca5ee129d364dcd7a.tar.gz
rust-921645c737f1d6d107a0a10ca5ee129d364dcd7a.zip
Auto merge of #126197 - jieyouxu:rmake-must-use, r=Kobzol
run-make: annotate library with `#[must_use]` and enforce `unused_must_use` in rmake.rs

This PR adds `#[must_use]` annotations to functions of the `run_make_support` library where it makes sense, and adjusts compiletest to compile rmake.rs with `-Dunused_must_use`.

The rationale is that it's highly likely that unused `#[must_use]` values in rmake.rs test files are bugs. For example, unused fs/io results are often load-bearing to the correctness of the test and often unchecked fs/io results allow the test to silently pass where it would've failed if the result was checked.

This PR is best reviewed commit-by-commit.

try-job: test-various
try-job: x86_64-msvc
-rw-r--r--Cargo.lock2
-rw-r--r--src/tools/compiletest/src/runtest.rs4
-rw-r--r--src/tools/run-make-support/CHANGELOG.md16
-rw-r--r--src/tools/run-make-support/Cargo.toml2
-rw-r--r--src/tools/run-make-support/src/cc.rs1
-rw-r--r--src/tools/run-make-support/src/clang.rs1
-rw-r--r--src/tools/run-make-support/src/command.rs3
-rw-r--r--src/tools/run-make-support/src/diff/mod.rs1
-rw-r--r--src/tools/run-make-support/src/lib.rs17
-rw-r--r--src/tools/run-make-support/src/llvm_readobj.rs1
-rw-r--r--src/tools/run-make-support/src/rustc.rs1
-rw-r--r--src/tools/run-make-support/src/rustdoc.rs1
12 files changed, 48 insertions, 2 deletions
diff --git a/Cargo.lock b/Cargo.lock
index e5ea9fee8a6..9d3eada2810 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -3452,7 +3452,7 @@ dependencies = [
 
 [[package]]
 name = "run_make_support"
-version = "0.1.0"
+version = "0.2.0"
 dependencies = [
  "gimli 0.28.1",
  "object 0.34.0",
diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs
index dbe016b8305..cc15961b46e 100644
--- a/src/tools/compiletest/src/runtest.rs
+++ b/src/tools/compiletest/src/runtest.rs
@@ -3551,6 +3551,10 @@ impl<'test> TestCx<'test> {
             .env("TARGET_RPATH_DIR", cwd.join(&self.config.run_lib_path))
             .env("LLVM_COMPONENTS", &self.config.llvm_components);
 
+        // 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");
diff --git a/src/tools/run-make-support/CHANGELOG.md b/src/tools/run-make-support/CHANGELOG.md
index a37de9fda80..c1b7b618a92 100644
--- a/src/tools/run-make-support/CHANGELOG.md
+++ b/src/tools/run-make-support/CHANGELOG.md
@@ -10,6 +10,22 @@ changes to the support library).
 This support library will probably never reach 1.0. Please bump the minor version in `Cargo.toml` if
 you make any breaking changes or other significant changes, or bump the patch version for bug fixes.
 
+## [0.2.0] - 2024-06-11
+
+### Added
+
+- Added `fs_wrapper` module which provides panic-on-fail helpers for their respective `std::fs`
+  counterparts, the motivation is to:
+    - Reduce littering `.unwrap()` or `.expect()` everywhere for fs operations
+    - Help the test writer avoid forgetting to check fs results (even though enforced by
+      `-Dunused_must_use`)
+    - Provide better panic messages by default
+- Added `path()` helper which creates a `Path` relative to `cwd()` (but is less noisy).
+
+### Changed
+
+- Marked many functions with `#[must_use]`, and rmake.rs are now compiled with `-Dunused_must_use`.
+
 ## [0.1.0] - 2024-06-09
 
 ### Changed
diff --git a/src/tools/run-make-support/Cargo.toml b/src/tools/run-make-support/Cargo.toml
index 5450620e7f0..2f7f51442f1 100644
--- a/src/tools/run-make-support/Cargo.toml
+++ b/src/tools/run-make-support/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "run_make_support"
-version = "0.1.0"
+version = "0.2.0"
 edition = "2021"
 
 [dependencies]
diff --git a/src/tools/run-make-support/src/cc.rs b/src/tools/run-make-support/src/cc.rs
index 8694740cfc3..31b9e8a23b8 100644
--- a/src/tools/run-make-support/src/cc.rs
+++ b/src/tools/run-make-support/src/cc.rs
@@ -15,6 +15,7 @@ pub fn cc() -> Cc {
 /// A platform-specific C compiler invocation builder. The specific C compiler used is
 /// passed down from compiletest.
 #[derive(Debug)]
+#[must_use]
 pub struct Cc {
     cmd: Command,
 }
diff --git a/src/tools/run-make-support/src/clang.rs b/src/tools/run-make-support/src/clang.rs
index a31004659c1..c23e41ebe21 100644
--- a/src/tools/run-make-support/src/clang.rs
+++ b/src/tools/run-make-support/src/clang.rs
@@ -11,6 +11,7 @@ pub fn clang() -> Clang {
 
 /// A `clang` invocation builder.
 #[derive(Debug)]
+#[must_use]
 pub struct Clang {
     cmd: Command,
 }
diff --git a/src/tools/run-make-support/src/command.rs b/src/tools/run-make-support/src/command.rs
index d689dc2f979..7cbc61bdf33 100644
--- a/src/tools/run-make-support/src/command.rs
+++ b/src/tools/run-make-support/src/command.rs
@@ -148,14 +148,17 @@ pub struct CompletedProcess {
 }
 
 impl CompletedProcess {
+    #[must_use]
     pub fn stdout_utf8(&self) -> String {
         String::from_utf8(self.output.stdout.clone()).expect("stdout is not valid UTF-8")
     }
 
+    #[must_use]
     pub fn stderr_utf8(&self) -> String {
         String::from_utf8(self.output.stderr.clone()).expect("stderr is not valid UTF-8")
     }
 
+    #[must_use]
     pub fn status(&self) -> ExitStatus {
         self.output.status
     }
diff --git a/src/tools/run-make-support/src/diff/mod.rs b/src/tools/run-make-support/src/diff/mod.rs
index 0fb6fec9d58..105cdbc7b32 100644
--- a/src/tools/run-make-support/src/diff/mod.rs
+++ b/src/tools/run-make-support/src/diff/mod.rs
@@ -13,6 +13,7 @@ pub fn diff() -> Diff {
 }
 
 #[derive(Debug)]
+#[must_use]
 pub struct Diff {
     expected: Option<String>,
     expected_name: Option<String>,
diff --git a/src/tools/run-make-support/src/lib.rs b/src/tools/run-make-support/src/lib.rs
index 0238255a53f..d077c500c5d 100644
--- a/src/tools/run-make-support/src/lib.rs
+++ b/src/tools/run-make-support/src/lib.rs
@@ -37,6 +37,7 @@ pub use rustc::{aux_build, rustc, Rustc};
 pub use rustdoc::{bare_rustdoc, rustdoc, Rustdoc};
 
 #[track_caller]
+#[must_use]
 pub fn env_var(name: &str) -> String {
     match env::var(name) {
         Ok(v) => v,
@@ -45,6 +46,7 @@ pub fn env_var(name: &str) -> String {
 }
 
 #[track_caller]
+#[must_use]
 pub fn env_var_os(name: &str) -> OsString {
     match env::var_os(name) {
         Some(v) => v,
@@ -53,32 +55,38 @@ pub fn env_var_os(name: &str) -> OsString {
 }
 
 /// `TARGET`
+#[must_use]
 pub fn target() -> String {
     env_var("TARGET")
 }
 
 /// Check if target is windows-like.
+#[must_use]
 pub fn is_windows() -> bool {
     target().contains("windows")
 }
 
 /// Check if target uses msvc.
+#[must_use]
 pub fn is_msvc() -> bool {
     target().contains("msvc")
 }
 
 /// Check if target uses macOS.
+#[must_use]
 pub fn is_darwin() -> bool {
     target().contains("darwin")
 }
 
 #[track_caller]
+#[must_use]
 pub fn python_command() -> Command {
     let python_path = env_var("PYTHON");
     Command::new(python_path)
 }
 
 #[track_caller]
+#[must_use]
 pub fn htmldocck() -> Command {
     let mut python = python_command();
     python.arg(source_root().join("src/etc/htmldocck.py"));
@@ -91,6 +99,7 @@ pub fn path<P: AsRef<Path>>(p: P) -> PathBuf {
 }
 
 /// Path to the root rust-lang/rust source checkout.
+#[must_use]
 pub fn source_root() -> PathBuf {
     env_var("SOURCE_ROOT").into()
 }
@@ -124,6 +133,7 @@ pub fn create_symlink<P: AsRef<Path>, Q: AsRef<Path>>(original: P, link: Q) {
 }
 
 /// Construct the static library name based on the platform.
+#[must_use]
 pub fn static_lib_name(name: &str) -> String {
     // See tools.mk (irrelevant lines omitted):
     //
@@ -148,6 +158,7 @@ pub fn static_lib_name(name: &str) -> String {
 }
 
 /// Construct the dynamic library name based on the platform.
+#[must_use]
 pub fn dynamic_lib_name(name: &str) -> String {
     // See tools.mk (irrelevant lines omitted):
     //
@@ -174,6 +185,7 @@ pub fn dynamic_lib_name(name: &str) -> String {
     }
 }
 
+#[must_use]
 pub fn dynamic_lib_extension() -> &'static str {
     if is_darwin() {
         "dylib"
@@ -185,16 +197,19 @@ pub fn dynamic_lib_extension() -> &'static str {
 }
 
 /// Generate the name a rust library (rlib) would have.
+#[must_use]
 pub fn rust_lib_name(name: &str) -> String {
     format!("lib{name}.rlib")
 }
 
 /// Construct the binary name based on platform.
+#[must_use]
 pub fn bin_name(name: &str) -> String {
     if is_windows() { format!("{name}.exe") } else { name.to_string() }
 }
 
 /// Return the current working directory.
+#[must_use]
 pub fn cwd() -> PathBuf {
     env::current_dir().unwrap()
 }
@@ -202,6 +217,7 @@ pub fn cwd() -> PathBuf {
 /// Use `cygpath -w` on a path to get a Windows path string back. This assumes that `cygpath` is
 /// available on the platform!
 #[track_caller]
+#[must_use]
 pub fn cygpath_windows<P: AsRef<Path>>(path: P) -> String {
     let caller = panic::Location::caller();
     let mut cygpath = Command::new("cygpath");
@@ -217,6 +233,7 @@ pub fn cygpath_windows<P: AsRef<Path>>(path: P) -> String {
 
 /// Run `uname`. This assumes that `uname` is available on the platform!
 #[track_caller]
+#[must_use]
 pub fn uname() -> String {
     let caller = panic::Location::caller();
     let mut uname = Command::new("uname");
diff --git a/src/tools/run-make-support/src/llvm_readobj.rs b/src/tools/run-make-support/src/llvm_readobj.rs
index 57ddfc205e6..3c719356e8f 100644
--- a/src/tools/run-make-support/src/llvm_readobj.rs
+++ b/src/tools/run-make-support/src/llvm_readobj.rs
@@ -12,6 +12,7 @@ pub fn llvm_readobj() -> LlvmReadobj {
 
 /// A `llvm-readobj` invocation builder.
 #[derive(Debug)]
+#[must_use]
 pub struct LlvmReadobj {
     cmd: Command,
 }
diff --git a/src/tools/run-make-support/src/rustc.rs b/src/tools/run-make-support/src/rustc.rs
index c27af966f39..331e1a5ab8b 100644
--- a/src/tools/run-make-support/src/rustc.rs
+++ b/src/tools/run-make-support/src/rustc.rs
@@ -18,6 +18,7 @@ pub fn aux_build() -> Rustc {
 
 /// A `rustc` invocation builder.
 #[derive(Debug)]
+#[must_use]
 pub struct Rustc {
     cmd: Command,
 }
diff --git a/src/tools/run-make-support/src/rustdoc.rs b/src/tools/run-make-support/src/rustdoc.rs
index 332906f739a..93078561254 100644
--- a/src/tools/run-make-support/src/rustdoc.rs
+++ b/src/tools/run-make-support/src/rustdoc.rs
@@ -17,6 +17,7 @@ pub fn rustdoc() -> Rustdoc {
 }
 
 #[derive(Debug)]
+#[must_use]
 pub struct Rustdoc {
     cmd: Command,
 }