Add #[must_use] to process::Command, process::Child and process::ExitStatus

library/std/src/process.rs

@@ -132,15 +132,25 @@ use crate::sys_common::{AsInner, AsInnerMut, FromInner, IntoInner};
 ///
 /// # Warning
 ///
+/// A `Child` which is simply dropped may continue running in parallel,
+/// possibly even after the program which launched it exits.
+/// If it inherited stdin/stdout, it may still read and write to them
+/// later, causing confusion and disruption.
+///
 /// On some systems, calling [`wait`] or similar is necessary for the OS to
 /// release resources. A process that terminated but has not been waited on is
 /// still around as a "zombie". Leaving too many zombies around may exhaust
 /// global resources (for example process IDs).
 ///
+/// Unless [`wait`] is called, any failure of the child will not be
+/// visible.
+///
 /// The standard library does *not* automatically wait on child processes (not
 /// even if the `Child` is dropped), it is up to the application developer to do
-/// so. As a consequence, dropping `Child` handles without waiting on them first
-/// is not recommended in long-running applications.
+/// so.
+///
+/// For these reasons, dropping `Child` handles without waiting on them first
+/// is usually incorrect.
 ///
 /// # Examples
 ///
@@ -160,6 +170,7 @@ use crate::sys_common::{AsInner, AsInnerMut, FromInner, IntoInner};
 ///
 /// [`wait`]: Child::wait
 #[stable(feature = "process", since = "1.0.0")]
+#[must_use = "this Child should probably be `wait`ed, or `status` used instead of `spawn`"]
 pub struct Child {
     handle: imp::Process,
 
@@ -483,17 +494,20 @@ impl fmt::Debug for ChildStderr {
 /// let mut list_dir = Command::new("ls");
 ///
 /// // Execute `ls` in the current directory of the program.
-/// list_dir.status().expect("process failed to execute");
+/// let status = list_dir.status().expect("process failed to execute");
+/// assert!(status.success());
 ///
 /// println!();
 ///
 /// // Change `ls` to execute in the root directory.
 /// list_dir.current_dir("/");
 ///
 /// // And then execute `ls` again but in the root directory.
-/// list_dir.status().expect("process failed to execute");
+/// let status = list_dir.status().expect("process failed to execute");
+/// assert!(status.success());
 /// ```
 #[stable(feature = "process", since = "1.0.0")]
+#[must_use = "this Command does not do anything unless it is run, eg using `status` or `spawn`"]
 pub struct Command {
     inner: imp::Command,
 }
@@ -525,9 +539,11 @@ impl Command {
     /// ```no_run
     /// use std::process::Command;
     ///
-    /// Command::new("sh")
-    ///         .spawn()
+    /// let status = Command::new("sh")
+    ///         .status()
     ///         .expect("sh command failed to start");
+    ///
+    /// assert!(status.success());
     /// ```
     #[stable(feature = "process", since = "1.0.0")]
     pub fn new<S: AsRef<OsStr>>(program: S) -> Command {
@@ -569,11 +585,13 @@ impl Command {
     /// ```no_run
     /// use std::process::Command;
     ///
-    /// Command::new("ls")
+    /// let status = Command::new("ls")
     ///         .arg("-l")
     ///         .arg("-a")
-    ///         .spawn()
+    ///         .status()
     ///         .expect("ls command failed to start");
+    ///
+    /// assert!(status.success());
     /// ```
     #[stable(feature = "process", since = "1.0.0")]
     pub fn arg<S: AsRef<OsStr>>(&mut self, arg: S) -> &mut Command {
@@ -599,10 +617,12 @@ impl Command {
     /// ```no_run
     /// use std::process::Command;
     ///
-    /// Command::new("ls")
+    /// let status = Command::new("ls")
     ///         .args(&["-l", "-a"])
-    ///         .spawn()
+    ///         .status()
     ///         .expect("ls command failed to start");
+    ///
+    /// assert!(status.success());
     /// ```
     #[stable(feature = "process", since = "1.0.0")]
     pub fn args<I, S>(&mut self, args: I) -> &mut Command
@@ -628,10 +648,12 @@ impl Command {
     /// ```no_run
     /// use std::process::Command;
     ///
-    /// Command::new("ls")
+    /// let status = Command::new("ls")
     ///         .env("PATH", "/bin")
-    ///         .spawn()
+    ///         .status()
     ///         .expect("ls command failed to start");
+    ///
+    /// assert!(status.success());
     /// ```
     #[stable(feature = "process", since = "1.0.0")]
     pub fn env<K, V>(&mut self, key: K, val: V) -> &mut Command
@@ -659,13 +681,15 @@ impl Command {
     ///         k == "TERM" || k == "TZ" || k == "LANG" || k == "PATH"
     ///     ).collect();
     ///
-    /// Command::new("printenv")
+    /// let status = Command::new("printenv")
     ///         .stdin(Stdio::null())
     ///         .stdout(Stdio::inherit())
     ///         .env_clear()
     ///         .envs(&filtered_env)
-    ///         .spawn()
+    ///         .status()
     ///         .expect("printenv failed to start");
+    ///
+    /// assert!(status.success());
     /// ```
     #[stable(feature = "command_envs", since = "1.19.0")]
     pub fn envs<I, K, V>(&mut self, vars: I) -> &mut Command
@@ -689,10 +713,12 @@ impl Command {
     /// ```no_run
     /// use std::process::Command;
     ///
-    /// Command::new("ls")
+    /// let status = Command::new("ls")
     ///         .env_remove("PATH")
-    ///         .spawn()
+    ///         .status()
     ///         .expect("ls command failed to start");
+    ///
+    /// assert!(status.success());
     /// ```
     #[stable(feature = "process", since = "1.0.0")]
     pub fn env_remove<K: AsRef<OsStr>>(&mut self, key: K) -> &mut Command {
@@ -709,10 +735,12 @@ impl Command {
     /// ```no_run
     /// use std::process::Command;
     ///
-    /// Command::new("ls")
+    /// let status = Command::new("ls")
     ///         .env_clear()
-    ///         .spawn()
+    ///         .status()
     ///         .expect("ls command failed to start");
+    ///
+    /// assert!(status.success());
     /// ```
     #[stable(feature = "process", since = "1.0.0")]
     pub fn env_clear(&mut self) -> &mut Command {
@@ -737,10 +765,12 @@ impl Command {
     /// ```no_run
     /// use std::process::Command;
     ///
-    /// Command::new("ls")
+    /// let status = Command::new("ls")
     ///         .current_dir("/bin")
-    ///         .spawn()
+    ///         .status()
     ///         .expect("ls command failed to start");
+    ///
+    /// assert!(status.success());
     /// ```
     ///
     /// [`canonicalize`]: crate::fs::canonicalize
@@ -765,10 +795,12 @@ impl Command {
     /// ```no_run
     /// use std::process::{Command, Stdio};
     ///
-    /// Command::new("ls")
+    /// let status = Command::new("ls")
     ///         .stdin(Stdio::null())
-    ///         .spawn()
+    ///         .status()
     ///         .expect("ls command failed to start");
+    ///
+    /// assert!(status.success());
     /// ```
     #[stable(feature = "process", since = "1.0.0")]
     pub fn stdin<T: Into<Stdio>>(&mut self, cfg: T) -> &mut Command {
@@ -791,10 +823,12 @@ impl Command {
     /// ```no_run
     /// use std::process::{Command, Stdio};
     ///
-    /// Command::new("ls")
+    /// let status = Command::new("ls")
     ///         .stdout(Stdio::null())
-    ///         .spawn()
+    ///         .status()
     ///         .expect("ls command failed to start");
+    ///
+    /// assert!(status.success());
     /// ```
     #[stable(feature = "process", since = "1.0.0")]
     pub fn stdout<T: Into<Stdio>>(&mut self, cfg: T) -> &mut Command {
@@ -817,10 +851,12 @@ impl Command {
     /// ```no_run
     /// use std::process::{Command, Stdio};
     ///
-    /// Command::new("ls")
+    /// let status = Command::new("ls")
     ///         .stderr(Stdio::null())
-    ///         .spawn()
+    ///         .status()
     ///         .expect("ls command failed to start");
+    ///
+    /// assert!(status.success());
     /// ```
     #[stable(feature = "process", since = "1.0.0")]
     pub fn stderr<T: Into<Stdio>>(&mut self, cfg: T) -> &mut Command {
@@ -832,16 +868,23 @@ impl Command {
     ///
     /// By default, stdin, stdout and stderr are inherited from the parent.
     ///
+    /// If you just want to run the command and wait for it to complete, consider using
+    /// [`Command::status`] instead.
+    ///
     /// # Examples
     ///
     /// Basic usage:
     ///
     /// ```no_run
     /// use std::process::Command;
     ///
-    /// Command::new("ls")
+    /// let status = Command::new("ls")
     ///         .spawn()
-    ///         .expect("ls command failed to start");
+    ///         .expect("ls command failed to start")
+    ///         .wait()
+    ///         .expect("failed to wait for child");
+    ///
+    /// assert!(status.success());
     /// ```
     #[stable(feature = "process", since = "1.0.0")]
     pub fn spawn(&mut self) -> io::Result<Child> {
@@ -1373,6 +1416,7 @@ impl From<fs::File> for Stdio {
 /// [`wait`]: Child::wait
 #[derive(PartialEq, Eq, Clone, Copy, Debug)]
 #[stable(feature = "process", since = "1.0.0")]
+#[must_use = "this ExitStatus might represent an error that should be checked and handled"]
 pub struct ExitStatus(imp::ExitStatus);
 
 impl ExitStatus {
@@ -1557,8 +1601,8 @@ impl Child {
     ///
     /// let mut command = Command::new("ls");
     /// if let Ok(mut child) = command.spawn() {
-    ///     child.wait().expect("command wasn't running");
-    ///     println!("Child has finished its execution!");
+    ///     let status = child.wait().expect("command wasn't running");
+    ///     println!("Child has finished its execution, status = {}!", status);
     /// } else {
     ///     println!("ls command didn't start");
     /// }

src/bootstrap/bin/rustc.rs

@@ -179,7 +179,10 @@ fn main() {
     }
 
     if let Some(mut on_fail) = on_fail {
-        on_fail.status().expect("Could not run the on_fail command");
+        let status = on_fail.status().expect("Could not run the on_fail command");
+        if !status.success() {
+            eprintln!("\nwarning: the on-fail command also failed: {}\n", status);
+        }
     }
 
     // Preserve the exit code. In case of signal, exit with 0xfe since it's

src/test/ui/fds-are-cloexec.rs

@@ -66,7 +66,9 @@ fn parent() {
                         .status()
                         .unwrap();
     assert!(status.success());
-    child.wait().unwrap();
+
+    let status = child.wait().unwrap();
+    assert!(status.success());
 }
 
 fn child(args: &[String]) {

src/test/ui/try-wait.rs

@@ -31,7 +31,8 @@ fn main() {
     assert!(maybe_status.is_none());
 
     me.kill().unwrap();
-    me.wait().unwrap();
+    let status = me.wait().unwrap();
+    assert!(!status.success());
 
     let status = me.try_wait().unwrap().unwrap();
     assert!(!status.success());

src/tools/compiletest/src/runtest.rs

@@ -869,17 +869,23 @@ impl<'test> TestCx<'test> {
 
             let adb_path = &self.config.adb_path;
 
-            Command::new(adb_path)
+            let status = Command::new(adb_path)
                 .arg("push")
                 .arg(&exe_file)
                 .arg(&self.config.adb_test_dir)
                 .status()
                 .unwrap_or_else(|_| panic!("failed to exec `{:?}`", adb_path));
+            if !status.success() {
+                panic!("Program failed `{:}`", adb_path);
+            }
 
-            Command::new(adb_path)
+            let status = Command::new(adb_path)
                 .args(&["forward", "tcp:5039", "tcp:5039"])
                 .status()
                 .unwrap_or_else(|_| panic!("failed to exec `{:?}`", adb_path));
+            if !status.success() {
+                panic!("Program failed `{:}`", adb_path);
+            }
 
             let adb_arg = format!(
                 "export LD_LIBRARY_PATH={}; \