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
This commit is contained in:
bors 2024-06-13 07:26:21 +00:00
commit 921645c737
12 changed files with 48 additions and 2 deletions

View File

@ -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",

View File

@ -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");

View File

@ -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

View File

@ -1,6 +1,6 @@
[package]
name = "run_make_support"
version = "0.1.0"
version = "0.2.0"
edition = "2021"
[dependencies]

View File

@ -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,
}

View File

@ -11,6 +11,7 @@ pub fn clang() -> Clang {
/// A `clang` invocation builder.
#[derive(Debug)]
#[must_use]
pub struct Clang {
cmd: Command,
}

View File

@ -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
}

View File

@ -13,6 +13,7 @@ pub fn diff() -> Diff {
}
#[derive(Debug)]
#[must_use]
pub struct Diff {
expected: Option<String>,
expected_name: Option<String>,

View File

@ -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");

View File

@ -12,6 +12,7 @@ pub fn llvm_readobj() -> LlvmReadobj {
/// A `llvm-readobj` invocation builder.
#[derive(Debug)]
#[must_use]
pub struct LlvmReadobj {
cmd: Command,
}

View File

@ -18,6 +18,7 @@ pub fn aux_build() -> Rustc {
/// A `rustc` invocation builder.
#[derive(Debug)]
#[must_use]
pub struct Rustc {
cmd: Command,
}

View File

@ -17,6 +17,7 @@ pub fn rustdoc() -> Rustdoc {
}
#[derive(Debug)]
#[must_use]
pub struct Rustdoc {
cmd: Command,
}