From be572a8010cd3223896afb09d84dab95e3e570fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= Date: Thu, 30 May 2024 00:50:39 +0000 Subject: [PATCH 1/4] run-make-support: add `#[must_use]` and drop bombs for command wrappers 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 ``` We also add `#[must_use]` attributes to functions in the support library where suitable to help catch unintentionally discarded values. --- src/tools/run-make-support/src/cc.rs | 7 ++- src/tools/run-make-support/src/clang.rs | 13 ++++- src/tools/run-make-support/src/diff/mod.rs | 10 +++- .../run-make-support/src/drop_bomb/mod.rs | 37 ++++++++++++ .../run-make-support/src/drop_bomb/tests.rs | 15 +++++ src/tools/run-make-support/src/lib.rs | 57 +++++++++++++++---- .../run-make-support/src/llvm_readobj.rs | 5 +- src/tools/run-make-support/src/rustc.rs | 28 ++++----- src/tools/run-make-support/src/rustdoc.rs | 29 ++++------ 9 files changed, 149 insertions(+), 52 deletions(-) create mode 100644 src/tools/run-make-support/src/drop_bomb/mod.rs create mode 100644 src/tools/run-make-support/src/drop_bomb/tests.rs diff --git a/src/tools/run-make-support/src/cc.rs b/src/tools/run-make-support/src/cc.rs index 799c36b104999..f0551f054ef91 100644 --- a/src/tools/run-make-support/src/cc.rs +++ b/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. @@ -87,6 +90,7 @@ impl Cc { } /// `EXTRACFLAGS` +#[must_use] pub fn extra_c_flags() -> Vec<&'static str> { // Adapted from tools.mk (trimmed): // @@ -145,6 +149,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): // diff --git a/src/tools/run-make-support/src/clang.rs b/src/tools/run-make-support/src/clang.rs index 6ccce67b250db..53dd8a3d6aaec 100644 --- a/src/tools/run-make-support/src/clang.rs +++ b/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,23 +11,27 @@ 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>(&mut self, path: P) -> &mut Self { self.cmd.arg(path.as_ref()); self @@ -34,6 +39,7 @@ impl Clang { /// 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))); @@ -41,6 +47,7 @@ impl Clang { } /// 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); @@ -48,24 +55,28 @@ impl Clang { } /// 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 diff --git a/src/tools/run-make-support/src/diff/mod.rs b/src/tools/run-make-support/src/diff/mod.rs index d864ddf4eb175..05b93b573c18d 100644 --- a/src/tools/run-make-support/src/diff/mod.rs +++ b/src/tools/run-make-support/src/diff/mod.rs @@ -2,6 +2,8 @@ use regex::Regex; use similar::TextDiff; use std::path::Path; +use crate::drop_bomb::DropBomb; + #[cfg(test)] mod tests; @@ -9,6 +11,7 @@ pub fn diff() -> Diff { Diff::new() } +#[must_use] #[derive(Debug)] pub struct Diff { expected: Option, @@ -16,10 +19,12 @@ pub struct Diff { actual: Option, actual_name: Option, normalizers: Vec<(String, String)>, + drop_bomb: DropBomb, } impl Diff { /// Construct a bare `diff` invocation. + #[must_use] pub fn new() -> Self { Self { expected: None, @@ -27,6 +32,7 @@ impl Diff { 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(); diff --git a/src/tools/run-make-support/src/drop_bomb/mod.rs b/src/tools/run-make-support/src/drop_bomb/mod.rs new file mode 100644 index 0000000000000..57dba70f2631f --- /dev/null +++ b/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 . + +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>>(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) + } + } +} diff --git a/src/tools/run-make-support/src/drop_bomb/tests.rs b/src/tools/run-make-support/src/drop_bomb/tests.rs new file mode 100644 index 0000000000000..4a488c0f67080 --- /dev/null +++ b/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 +} diff --git a/src/tools/run-make-support/src/lib.rs b/src/tools/run-make-support/src/lib.rs index 0cf64db6ac9c0..0ef7b7ae6a551 100644 --- a/src/tools/run-make-support/src/lib.rs +++ b/src/tools/run-make-support/src/lib.rs @@ -6,6 +6,7 @@ pub mod cc; pub mod clang; pub mod diff; +mod drop_bomb; pub mod llvm_readobj; pub mod run; pub mod rustc; @@ -31,52 +32,62 @@ pub use rustc::{aux_build, rustc, Rustc}; pub use rustdoc::{bare_rustdoc, rustdoc, Rustdoc}; /// Path of `TMPDIR` (a temporary build directory, not under `/tmp`). +#[must_use] pub fn tmp_dir() -> PathBuf { env::var_os("TMPDIR").unwrap().into() } /// `TARGET` +#[must_use] pub fn target() -> String { env::var("TARGET").unwrap() } /// 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") } /// Construct a path to a static library under `$TMPDIR` given the library name. This will return a /// path with `$TMPDIR` joined with platform-and-compiler-specific library name. +#[must_use] pub fn static_lib(name: &str) -> PathBuf { tmp_dir().join(static_lib_name(name)) } +#[must_use] pub fn python_command() -> Command { let python_path = std::env::var("PYTHON").expect("PYTHON environment variable does not exist"); Command::new(python_path) } +#[must_use] pub fn htmldocck() -> Command { let mut python = python_command(); python.arg(source_path().join("src/etc/htmldocck.py")); python } +#[must_use] pub fn source_path() -> PathBuf { std::env::var("S").expect("S variable does not exist").into() } /// 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): // @@ -102,11 +113,13 @@ pub fn static_lib_name(name: &str) -> String { /// Construct a path to a dynamic library under `$TMPDIR` given the library name. This will return a /// path with `$TMPDIR` joined with platform-and-compiler-specific library name. +#[must_use] pub fn dynamic_lib(name: &str) -> PathBuf { tmp_dir().join(dynamic_lib_name(name)) } /// 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): // @@ -134,6 +147,7 @@ pub fn dynamic_lib_name(name: &str) -> String { /// Construct a path to a rust library (rlib) under `$TMPDIR` given the library name. This will return a /// path with `$TMPDIR` joined with the library name. +#[must_use] pub fn rust_lib(name: &str) -> PathBuf { tmp_dir().join(rust_lib_name(name)) } @@ -145,12 +159,14 @@ pub fn rust_lib_name(name: &str) -> String { } /// 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() } } /// Use `cygpath -w` on a path to get a Windows path string back. This assumes that `cygpath` is /// available on the platform! +#[must_use] #[track_caller] pub fn cygpath_windows>(path: P) -> String { let caller_location = std::panic::Location::caller(); @@ -169,6 +185,7 @@ pub fn cygpath_windows>(path: P) -> String { } /// Run `uname`. This assumes that `uname` is available on the platform! +#[must_use] #[track_caller] pub fn uname() -> String { let caller_location = std::panic::Location::caller(); @@ -285,19 +302,21 @@ pub fn assert_not_contains(haystack: &str, needle: &str) { } } +// FIXME(jieyouxu): convert this macro to become a command wrapper trait. /// Implement common helpers for command wrappers. This assumes that the command wrapper is a struct -/// containing a `cmd: Command` field and a `output` function. The provided helpers are: +/// containing a `cmd: Command` field, a `command_output()` function, and a `drop_bomb` field. The +/// provided helpers can be grouped into a few categories: /// /// 1. Generic argument acceptors: `arg` and `args` (delegated to [`Command`]). These are intended /// to be *fallback* argument acceptors, when specific helpers don't make sense. Prefer to add /// new specific helper methods over relying on these generic argument providers. /// 2. Environment manipulation methods: `env`, `env_remove` and `env_clear`: these delegate to /// methods of the same name on [`Command`]. -/// 3. Output and execution: `output`, `run` and `run_fail` are provided. `output` waits for the -/// command to finish running and returns the process's [`Output`]. `run` and `run_fail` are -/// higher-level convenience methods which waits for the command to finish running and assert -/// that the command successfully ran or failed as expected. Prefer `run` and `run_fail` when -/// possible. +/// 3. Output and execution: `run`, `run_fail` and `run_fail_assert_exit_code` are provided. `run*` +/// are higher-level convenience methods which waits for the command to finish running and assert +/// that the command successfully ran or failed as expected. +/// +/// `command_output()` should almost never be used in test code. /// /// Example usage: /// @@ -378,9 +397,16 @@ macro_rules! impl_common_helpers { self } + /// Set the path where the command will be run. + pub fn current_dir>(&mut self, path: P) -> &mut Self { + self.cmd.current_dir(path); + self + } + /// Run the constructed command and assert that it is successfully run. #[track_caller] pub fn run(&mut self) -> ::std::process::Output { + self.drop_bomb.defuse(); let caller_location = ::std::panic::Location::caller(); let caller_line_number = caller_location.line(); @@ -394,6 +420,7 @@ macro_rules! impl_common_helpers { /// Run the constructed command and assert that it does not successfully run. #[track_caller] pub fn run_fail(&mut self) -> ::std::process::Output { + self.drop_bomb.defuse(); let caller_location = ::std::panic::Location::caller(); let caller_line_number = caller_location.line(); @@ -404,10 +431,20 @@ macro_rules! impl_common_helpers { output } - /// Set the path where the command will be run. - pub fn current_dir>(&mut self, path: P) -> &mut Self { - self.cmd.current_dir(path); - self + /// Run the constructed command and assert that it does not successfully run and + /// exits with the expected exit code. + // FIXME(jieyouxu): we should probably *always* assert the expected exit code? + #[track_caller] + pub fn run_fail_assert_exit_code(&mut self, code: i32) -> ::std::process::Output { + self.drop_bomb.defuse(); + let caller_location = ::std::panic::Location::caller(); + let caller_line_number = caller_location.line(); + + let output = self.command_output(); + if output.status.code().unwrap() != code { + handle_failed_output(&self.cmd, output, caller_line_number); + } + output } } }; diff --git a/src/tools/run-make-support/src/llvm_readobj.rs b/src/tools/run-make-support/src/llvm_readobj.rs index f114aacfa3fc7..9460fa12d2240 100644 --- a/src/tools/run-make-support/src/llvm_readobj.rs +++ b/src/tools/run-make-support/src/llvm_readobj.rs @@ -2,6 +2,7 @@ use std::env; use std::path::{Path, PathBuf}; use std::process::Command; +use crate::drop_bomb::DropBomb; use crate::handle_failed_output; /// Construct a new `llvm-readobj` invocation. This assumes that `llvm-readobj` is available @@ -11,9 +12,11 @@ pub fn llvm_readobj() -> LlvmReadobj { } /// A `llvm-readobj` invocation builder. +#[must_use] #[derive(Debug)] pub struct LlvmReadobj { cmd: Command, + drop_bomb: DropBomb, } crate::impl_common_helpers!(LlvmReadobj); @@ -27,7 +30,7 @@ impl LlvmReadobj { let llvm_bin_dir = PathBuf::from(llvm_bin_dir); let llvm_readobj = llvm_bin_dir.join("llvm-readobj"); let cmd = Command::new(llvm_readobj); - Self { cmd } + Self { cmd, drop_bomb: DropBomb::arm("llvm-readobj invocation must be executed") } } /// Provide an input file. diff --git a/src/tools/run-make-support/src/rustc.rs b/src/tools/run-make-support/src/rustc.rs index 8b0252b8f04c4..840ff1aef7afe 100644 --- a/src/tools/run-make-support/src/rustc.rs +++ b/src/tools/run-make-support/src/rustc.rs @@ -4,6 +4,7 @@ use std::io::Write; use std::path::Path; use std::process::{Command, Output, Stdio}; +use crate::drop_bomb::DropBomb; use crate::{handle_failed_output, set_host_rpath, tmp_dir}; /// Construct a new `rustc` invocation. @@ -17,20 +18,23 @@ pub fn aux_build() -> Rustc { } /// A `rustc` invocation builder. +#[must_use] #[derive(Debug)] pub struct Rustc { cmd: Command, stdin: Option>, + drop_bomb: DropBomb, } crate::impl_common_helpers!(Rustc); -fn setup_common() -> Command { +fn setup_common() -> (Command, DropBomb) { let rustc = env::var("RUSTC").unwrap(); let mut cmd = Command::new(rustc); set_host_rpath(&mut cmd); cmd.arg("--out-dir").arg(tmp_dir()).arg("-L").arg(tmp_dir()); - cmd + let drop_bomb = DropBomb::arm("rustc invocation must be executed"); + (cmd, drop_bomb) } impl Rustc { @@ -38,15 +42,15 @@ impl Rustc { /// Construct a new `rustc` invocation. pub fn new() -> Self { - let cmd = setup_common(); - Self { cmd, stdin: None } + let (cmd, drop_bomb) = setup_common(); + Self { cmd, stdin: None, drop_bomb } } /// Construct a new `rustc` invocation with `aux_build` preset (setting `--crate-type=lib`). pub fn new_aux_build() -> Self { - let mut cmd = setup_common(); + let (mut cmd, drop_bomb) = setup_common(); cmd.arg("--crate-type=lib"); - Self { cmd, stdin: None } + Self { cmd, stdin: None, drop_bomb } } // Argument provider methods @@ -230,16 +234,4 @@ impl Rustc { self.cmd.output().expect("failed to get output of finished process") } } - - #[track_caller] - pub fn run_fail_assert_exit_code(&mut self, code: i32) -> Output { - let caller_location = std::panic::Location::caller(); - let caller_line_number = caller_location.line(); - - let output = self.command_output(); - if output.status.code().unwrap() != code { - handle_failed_output(&self.cmd, output, caller_line_number); - } - output - } } diff --git a/src/tools/run-make-support/src/rustdoc.rs b/src/tools/run-make-support/src/rustdoc.rs index 61d7448a6bfe7..69925287b3e6e 100644 --- a/src/tools/run-make-support/src/rustdoc.rs +++ b/src/tools/run-make-support/src/rustdoc.rs @@ -4,6 +4,7 @@ use std::io::Write; use std::path::Path; use std::process::{Command, Output, Stdio}; +use crate::drop_bomb::DropBomb; use crate::{handle_failed_output, set_host_rpath}; /// Construct a plain `rustdoc` invocation with no flags set. @@ -16,34 +17,36 @@ pub fn rustdoc() -> Rustdoc { Rustdoc::new() } +#[must_use] #[derive(Debug)] pub struct Rustdoc { cmd: Command, stdin: Option>, + drop_bomb: DropBomb, } crate::impl_common_helpers!(Rustdoc); -fn setup_common() -> Command { +fn setup_common() -> (Command, DropBomb) { let rustdoc = env::var("RUSTDOC").unwrap(); let mut cmd = Command::new(rustdoc); set_host_rpath(&mut cmd); - cmd + (cmd, DropBomb::arm("rustdoc invocation must be executed")) } impl Rustdoc { /// Construct a bare `rustdoc` invocation. pub fn bare() -> Self { - let cmd = setup_common(); - Self { cmd, stdin: None } + let (cmd, drop_bomb) = setup_common(); + Self { cmd, stdin: None, drop_bomb } } /// Construct a `rustdoc` invocation with `-L $(TARGET_RPATH_DIR)` set. pub fn new() -> Self { - let mut cmd = setup_common(); + let (mut cmd, drop_bomb) = setup_common(); let target_rpath_dir = env::var_os("TARGET_RPATH_DIR").unwrap(); cmd.arg(format!("-L{}", target_rpath_dir.to_string_lossy())); - Self { cmd, stdin: None } + Self { cmd, stdin: None, drop_bomb } } /// Specify where an external library is located. @@ -96,7 +99,7 @@ impl Rustdoc { /// Get the [`Output`] of the finished process. #[track_caller] - pub fn command_output(&mut self) -> ::std::process::Output { + pub fn command_output(&mut self) -> Output { // let's make sure we piped all the input and outputs self.cmd.stdin(Stdio::piped()); self.cmd.stdout(Stdio::piped()); @@ -157,16 +160,4 @@ impl Rustdoc { self.cmd.arg(format); self } - - #[track_caller] - pub fn run_fail_assert_exit_code(&mut self, code: i32) -> Output { - let caller_location = std::panic::Location::caller(); - let caller_line_number = caller_location.line(); - - let output = self.command_output(); - if output.status.code().unwrap() != code { - handle_failed_output(&self.cmd, output, caller_line_number); - } - output - } } From 288c2deeeaadaf2d6c9c2af96feca711181146fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= Date: Thu, 30 May 2024 00:59:32 +0000 Subject: [PATCH 2/4] compiletest: compile rmake.rs with -Dunused_must_use --- src/tools/compiletest/src/runtest.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 4ea12a0f9e480..f1bc6d0982570 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -3507,6 +3507,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"); From a77a5f440ca92d3400290273aa3774354d483b68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= Date: Thu, 30 May 2024 03:14:55 +0000 Subject: [PATCH 3/4] run-make-support: extract rust_lib_name helper --- src/tools/run-make-support/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tools/run-make-support/src/lib.rs b/src/tools/run-make-support/src/lib.rs index 0ef7b7ae6a551..50776b2d1ea8c 100644 --- a/src/tools/run-make-support/src/lib.rs +++ b/src/tools/run-make-support/src/lib.rs @@ -145,8 +145,8 @@ pub fn dynamic_lib_name(name: &str) -> String { } } -/// Construct a path to a rust library (rlib) under `$TMPDIR` given the library name. This will return a -/// path with `$TMPDIR` joined with the library name. +/// Construct a path to a rust library (rlib) under `$TMPDIR` given the library name. This will +/// return a path with `$TMPDIR` joined with the library name. #[must_use] pub fn rust_lib(name: &str) -> PathBuf { tmp_dir().join(rust_lib_name(name)) From 7867fb75a377fb7105fc0679e50a05add842ba25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= Date: Thu, 30 May 2024 02:40:27 +0000 Subject: [PATCH 4/4] tests/run-make: fix unused_must_use issues and other test failures --- tests/run-make/CURRENT_RUSTC_VERSION/rmake.rs | 3 +-- .../c-link-to-rust-staticlib/rmake.rs | 2 +- .../run-make/doctests-keep-binaries/rmake.rs | 4 ++-- tests/run-make/doctests-runtool/rmake.rs | 4 ++-- tests/run-make/reset-codegen-1/rmake.rs | 5 +++-- tests/run-make/rustdoc-error-lines/rmake.rs | 5 ++--- .../rustdoc-scrape-examples-macros/rmake.rs | 19 ++++++++----------- tests/run-make/rustdoc-shared-flags/rmake.rs | 4 ++-- .../same-lib-two-locations-no-panic/rmake.rs | 12 ++++++------ 9 files changed, 27 insertions(+), 31 deletions(-) diff --git a/tests/run-make/CURRENT_RUSTC_VERSION/rmake.rs b/tests/run-make/CURRENT_RUSTC_VERSION/rmake.rs index 1bdb634757120..f30f59d818690 100644 --- a/tests/run-make/CURRENT_RUSTC_VERSION/rmake.rs +++ b/tests/run-make/CURRENT_RUSTC_VERSION/rmake.rs @@ -13,8 +13,7 @@ fn main() { let mut stable_path = PathBuf::from(env!("TMPDIR")); stable_path.push("libstable.rmeta"); - let output = - rustc().input("main.rs").emit("metadata").extern_("stable", &stable_path).command_output(); + let output = rustc().input("main.rs").emit("metadata").extern_("stable", &stable_path).run(); let stderr = String::from_utf8_lossy(&output.stderr); let version = include_str!(concat!(env!("S"), "/src/version")); diff --git a/tests/run-make/c-link-to-rust-staticlib/rmake.rs b/tests/run-make/c-link-to-rust-staticlib/rmake.rs index 63d5eb78c6987..2f21c7356dfb8 100644 --- a/tests/run-make/c-link-to-rust-staticlib/rmake.rs +++ b/tests/run-make/c-link-to-rust-staticlib/rmake.rs @@ -10,6 +10,6 @@ fn main() { rustc().input("foo.rs").run(); cc().input("bar.c").input(static_lib("foo")).out_exe("bar").args(&extra_c_flags()).run(); run("bar"); - fs::remove_file(static_lib("foo")); + fs::remove_file(static_lib("foo")).unwrap(); run("bar"); } diff --git a/tests/run-make/doctests-keep-binaries/rmake.rs b/tests/run-make/doctests-keep-binaries/rmake.rs index 0613ef4839b14..137abc5ce672f 100644 --- a/tests/run-make/doctests-keep-binaries/rmake.rs +++ b/tests/run-make/doctests-keep-binaries/rmake.rs @@ -10,7 +10,7 @@ fn setup_test_env(callback: F) { create_dir(&out_dir).expect("failed to create doctests folder"); rustc().input("t.rs").crate_type("rlib").run(); callback(&out_dir, &tmp_dir().join("libt.rlib")); - remove_dir_all(out_dir); + remove_dir_all(out_dir).unwrap(); } fn check_generated_binaries() { @@ -60,6 +60,6 @@ fn main() { .extern_("t", "libt.rlib") .run(); - remove_dir_all(run_dir_path); + remove_dir_all(run_dir_path).unwrap(); }); } diff --git a/tests/run-make/doctests-runtool/rmake.rs b/tests/run-make/doctests-runtool/rmake.rs index 6cc7c6bbdafd2..1985f1f2e368f 100644 --- a/tests/run-make/doctests-runtool/rmake.rs +++ b/tests/run-make/doctests-runtool/rmake.rs @@ -33,6 +33,6 @@ fn main() { .current_dir(tmp_dir()) .run(); - remove_dir_all(run_dir); - remove_dir_all(run_tool); + remove_dir_all(run_dir).unwrap(); + remove_dir_all(run_tool).unwrap(); } diff --git a/tests/run-make/reset-codegen-1/rmake.rs b/tests/run-make/reset-codegen-1/rmake.rs index 4b91ba7df90bd..973636cec5bf7 100644 --- a/tests/run-make/reset-codegen-1/rmake.rs +++ b/tests/run-make/reset-codegen-1/rmake.rs @@ -31,8 +31,9 @@ fn main() { ("multi-output", Some("asm,obj")), ]; for (output_file, emit) in flags { - fs::remove_file(output_file).unwrap_or_default(); + fs::remove_dir_all(tmp_dir()).unwrap_or_default(); + fs::create_dir_all(tmp_dir()).unwrap(); compile(output_file, emit); - fs::remove_file(output_file); + fs::remove_dir_all(tmp_dir()).unwrap(); } } diff --git a/tests/run-make/rustdoc-error-lines/rmake.rs b/tests/run-make/rustdoc-error-lines/rmake.rs index 31536c78dd460..bba3aa7f96128 100644 --- a/tests/run-make/rustdoc-error-lines/rmake.rs +++ b/tests/run-make/rustdoc-error-lines/rmake.rs @@ -4,9 +4,8 @@ use run_make_support::rustdoc; fn main() { - let output = - String::from_utf8(rustdoc().input("input.rs").arg("--test").command_output().stdout) - .unwrap(); + let output = rustdoc().input("input.rs").arg("--test").run_fail(); + let output = String::from_utf8(output.stdout).unwrap(); let should_contain = &[ "input.rs - foo (line 5)", diff --git a/tests/run-make/rustdoc-scrape-examples-macros/rmake.rs b/tests/run-make/rustdoc-scrape-examples-macros/rmake.rs index 81b7defafc6c0..0fbdbc426b52d 100644 --- a/tests/run-make/rustdoc-scrape-examples-macros/rmake.rs +++ b/tests/run-make/rustdoc-scrape-examples-macros/rmake.rs @@ -9,17 +9,14 @@ fn main() { let proc_crate_name = "foobar_macro"; let crate_name = "foobar"; - let dylib_name = String::from_utf8( - rustc() - .crate_name(proc_crate_name) - .crate_type("dylib") - .arg("--print") - .arg("file-names") - .arg("-") - .command_output() - .stdout, - ) - .unwrap(); + let output = rustc() + .crate_name(proc_crate_name) + .crate_type("dylib") + .arg("--print") + .arg("file-names") + .arg("-") + .run(); + let dylib_name = String::from_utf8(output.stdout).unwrap(); rustc() .input("src/proc.rs") diff --git a/tests/run-make/rustdoc-shared-flags/rmake.rs b/tests/run-make/rustdoc-shared-flags/rmake.rs index 2db613f781764..105ca0b6dec59 100644 --- a/tests/run-make/rustdoc-shared-flags/rmake.rs +++ b/tests/run-make/rustdoc-shared-flags/rmake.rs @@ -1,8 +1,8 @@ use run_make_support::{rustc, rustdoc, Diff}; fn compare_outputs(args: &[&str]) { - let rustc_output = String::from_utf8(rustc().args(args).command_output().stdout).unwrap(); - let rustdoc_output = String::from_utf8(rustdoc().args(args).command_output().stdout).unwrap(); + let rustc_output = String::from_utf8(rustc().args(args).run().stdout).unwrap(); + let rustdoc_output = String::from_utf8(rustdoc().args(args).run().stdout).unwrap(); Diff::new().expected_text("rustc", rustc_output).actual_text("rustdoc", rustdoc_output).run(); } diff --git a/tests/run-make/same-lib-two-locations-no-panic/rmake.rs b/tests/run-make/same-lib-two-locations-no-panic/rmake.rs index 2900c3c8b749c..fb75300e90fa7 100644 --- a/tests/run-make/same-lib-two-locations-no-panic/rmake.rs +++ b/tests/run-make/same-lib-two-locations-no-panic/rmake.rs @@ -7,22 +7,22 @@ //@ ignore-cross-compile -use run_make_support::{dynamic_lib, rust_lib, rustc, tmp_dir}; +use run_make_support::{dynamic_lib, dynamic_lib_name, rust_lib, rust_lib_name, rustc, tmp_dir}; use std::fs; fn main() { let tmp_dir_other = tmp_dir().join("other"); - fs::create_dir(&tmp_dir_other); + fs::create_dir(&tmp_dir_other).unwrap(); rustc().input("foo.rs").crate_type("dylib").arg("-Cprefer-dynamic").run(); - fs::rename(dynamic_lib("foo"), &tmp_dir_other); + fs::rename(dynamic_lib("foo"), tmp_dir_other.join(dynamic_lib_name("foo"))).unwrap(); rustc().input("foo.rs").crate_type("dylib").arg("-Cprefer-dynamic").run(); rustc().input("bar.rs").library_search_path(&tmp_dir_other).run(); - fs::remove_dir_all(tmp_dir()); + fs::remove_dir_all(tmp_dir()).unwrap(); - fs::create_dir_all(&tmp_dir_other); + fs::create_dir_all(&tmp_dir_other).unwrap(); rustc().input("foo.rs").crate_type("rlib").run(); - fs::rename(rust_lib("foo"), &tmp_dir_other); + fs::rename(rust_lib("foo"), tmp_dir_other.join(rust_lib_name("foo"))).unwrap(); rustc().input("foo.rs").crate_type("rlib").run(); rustc().input("bar.rs").library_search_path(tmp_dir_other).run(); }