Auto merge of rust-lang#82973 - ijackson:exitstatuserror, r=yaahc

Provide ExitStatusError

Closes rust-lang#73125

In MR rust-lang#81452 "Add #[must_use] to [...] process::ExitStatus" we concluded that the existing arrangements in are too awkward so adding that `#[must_use]` is blocked on improving the ergonomics.

I wrote a mini-RFC-style discusion of the approach in rust-lang#73125 (comment)

Author: bors

library/std/src/os/unix/process.rs

@@ -201,22 +201,32 @@ impl CommandExt for process::Command {
     }
 }
 
-/// Unix-specific extensions to [`process::ExitStatus`].
+/// Unix-specific extensions to [`process::ExitStatus`] and
+/// [`ExitStatusError`](process::ExitStatusError).
 ///
-/// On Unix, `ExitStatus` **does not necessarily represent an exit status**, as passed to the
-/// `exit` system call or returned by [`ExitStatus::code()`](crate::process::ExitStatus::code).
-/// It represents **any wait status**, as returned by one of the `wait` family of system calls.
+/// On Unix, `ExitStatus` **does not necessarily represent an exit status**, as
+/// passed to the `exit` system call or returned by
+/// [`ExitStatus::code()`](crate::process::ExitStatus::code).  It represents **any wait status**
+/// as returned by one of the `wait` family of system
+/// calls.
 ///
-/// This is because a Unix wait status (a Rust `ExitStatus`) can represent a Unix exit status, but
-/// can also represent other kinds of process event.
+/// A Unix wait status (a Rust `ExitStatus`) can represent a Unix exit status, but can also
+/// represent other kinds of process event.
 ///
 /// This trait is sealed: it cannot be implemented outside the standard library.
 /// This is so that future additional methods are not breaking changes.
 #[stable(feature = "rust1", since = "1.0.0")]
 pub trait ExitStatusExt: Sealed {
-    /// Creates a new `ExitStatus` from the raw underlying integer status value from `wait`
+    /// Creates a new `ExitStatus` or `ExitStatusError` from the raw underlying integer status
+    /// value from `wait`
     ///
     /// The value should be a **wait status, not an exit status**.
+    ///
+    /// # Panics
+    ///
+    /// Panics on an attempt to make an `ExitStatusError` from a wait status of `0`.
+    ///
+    /// Making an `ExitStatus` always succeds and never panics.
     #[stable(feature = "exit_status_from", since = "1.12.0")]
     fn from_raw(raw: i32) -> Self;
 
@@ -278,6 +288,35 @@ impl ExitStatusExt for process::ExitStatus {
     }
 }
 
+#[unstable(feature = "exit_status_error", issue = "84908")]
+impl ExitStatusExt for process::ExitStatusError {
+    fn from_raw(raw: i32) -> Self {
+        process::ExitStatus::from_raw(raw)
+            .exit_ok()
+            .expect_err("<ExitStatusError as ExitStatusExt>::from_raw(0) but zero is not an error")
+    }
+
+    fn signal(&self) -> Option<i32> {
+        self.into_status().signal()
+    }
+
+    fn core_dumped(&self) -> bool {
+        self.into_status().core_dumped()
+    }
+
+    fn stopped_signal(&self) -> Option<i32> {
+        self.into_status().stopped_signal()
+    }
+
+    fn continued(&self) -> bool {
+        self.into_status().continued()
+    }
+
+    fn into_raw(self) -> i32 {
+        self.into_status().into_raw()
+    }
+}
+
 #[stable(feature = "process_extensions", since = "1.2.0")]
 impl FromRawFd for process::Stdio {
     #[inline]

library/std/src/process.rs

@@ -110,6 +110,7 @@ use crate::ffi::OsStr;
 use crate::fmt;
 use crate::fs;
 use crate::io::{self, Initializer, IoSlice, IoSliceMut};
+use crate::num::NonZeroI32;
 use crate::path::Path;
 use crate::str;
 use crate::sys::pipe::{read2, AnonPipe};
@@ -1387,8 +1388,8 @@ impl From<fs::File> for Stdio {
 /// An `ExitStatus` represents every possible disposition of a process.  On Unix this
 /// is the **wait status**.  It is *not* simply an *exit status* (a value passed to `exit`).
 ///
-/// For proper error reporting of failed processes, print the value of `ExitStatus` using its
-/// implementation of [`Display`](crate::fmt::Display).
+/// For proper error reporting of failed processes, print the value of `ExitStatus` or
+/// `ExitStatusError` using their implementations of [`Display`](crate::fmt::Display).
 ///
 /// [`status`]: Command::status
 /// [`wait`]: Child::wait
@@ -1401,6 +1402,29 @@ pub struct ExitStatus(imp::ExitStatus);
 impl crate::sealed::Sealed for ExitStatus {}
 
 impl ExitStatus {
+    /// Was termination successful?  Returns a `Result`.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// #![feature(exit_status_error)]
+    /// # if cfg!(unix) {
+    /// use std::process::Command;
+    ///
+    /// let status = Command::new("ls")
+    ///                      .arg("/dev/nonexistent")
+    ///                      .status()
+    ///                      .expect("ls could not be executed");
+    ///
+    /// println!("ls: {}", status);
+    /// status.exit_ok().expect_err("/dev/nonexistent could be listed!");
+    /// # } // cfg!(unix)
+    /// ```
+    #[unstable(feature = "exit_status_error", issue = "84908")]
+    pub fn exit_ok(&self) -> Result<(), ExitStatusError> {
+        self.0.exit_ok().map_err(ExitStatusError)
+    }
+
     /// Was termination successful? Signal termination is not considered a
     /// success, and success is defined as a zero exit status.
     ///
@@ -1422,7 +1446,7 @@ impl ExitStatus {
     /// ```
     #[stable(feature = "process", since = "1.0.0")]
     pub fn success(&self) -> bool {
-        self.0.success()
+        self.0.exit_ok().is_ok()
     }
 
     /// Returns the exit code of the process, if any.
@@ -1476,6 +1500,120 @@ impl fmt::Display for ExitStatus {
     }
 }
 
+/// Allows extension traits within `std`.
+#[unstable(feature = "sealed", issue = "none")]
+impl crate::sealed::Sealed for ExitStatusError {}
+
+/// Describes the result of a process after it has failed
+///
+/// Produced by the [`.exit_ok`](ExitStatus::exit_ok) method on [`ExitStatus`].
+///
+/// # Examples
+///
+/// ```
+/// #![feature(exit_status_error)]
+/// # if cfg!(unix) {
+/// use std::process::{Command, ExitStatusError};
+///
+/// fn run(cmd: &str) -> Result<(),ExitStatusError> {
+///     Command::new(cmd).status().unwrap().exit_ok()?;
+///     Ok(())
+/// }
+///
+/// run("true").unwrap();
+/// run("false").unwrap_err();
+/// # } // cfg!(unix)
+/// ```
+#[derive(PartialEq, Eq, Clone, Copy, Debug)]
+#[unstable(feature = "exit_status_error", issue = "84908")]
+// The definition of imp::ExitStatusError should ideally be such that
+// Result<(), imp::ExitStatusError> has an identical representation to imp::ExitStatus.
+pub struct ExitStatusError(imp::ExitStatusError);
+
+#[unstable(feature = "exit_status_error", issue = "84908")]
+impl ExitStatusError {
+    /// Reports the exit code, if applicable, from an `ExitStatusError`.
+    ///
+    /// In Unix terms the return value is the **exit status**: the value passed to `exit`, if the
+    /// process finished by calling `exit`.  Note that on Unix the exit status is truncated to 8
+    /// bits, and that values that didn't come from a program's call to `exit` may be invented by the
+    /// runtime system (often, for example, 255, 254, 127 or 126).
+    ///
+    /// On Unix, this will return `None` if the process was terminated by a signal.  If you want to
+    /// handle such situations specially, consider using methods from
+    /// [`ExitStatusExt`](crate::os::unix::process::ExitStatusExt).
+    ///
+    /// If the process finished by calling `exit` with a nonzero value, this will return
+    /// that exit status.
+    ///
+    /// If the error was something else, it will return `None`.
+    ///
+    /// If the process exited successfully (ie, by calling `exit(0)`), there is no
+    /// `ExitStatusError`.  So the return value from `ExitStatusError::code()` is always nonzero.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// #![feature(exit_status_error)]
+    /// # #[cfg(unix)] {
+    /// use std::process::Command;
+    ///
+    /// let bad = Command::new("false").status().unwrap().exit_ok().unwrap_err();
+    /// assert_eq!(bad.code(), Some(1));
+    /// # } // #[cfg(unix)]
+    /// ```
+    pub fn code(&self) -> Option<i32> {
+        self.code_nonzero().map(Into::into)
+    }
+
+    /// Reports the exit code, if applicable, from an `ExitStatusError`, as a `NonZero`
+    ///
+    /// This is exaclty like [`code()`](Self::code), except that it returns a `NonZeroI32`.
+    ///
+    /// Plain `code`, returning a plain integer, is provided because is is often more convenient.
+    /// The returned value from `code()` is indeed also nonzero; use `code_nonzero()` when you want
+    /// a type-level guarantee of nonzeroness.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// #![feature(exit_status_error)]
+    /// # if cfg!(unix) {
+    /// use std::convert::TryFrom;
+    /// use std::num::NonZeroI32;
+    /// use std::process::Command;
+    ///
+    /// let bad = Command::new("false").status().unwrap().exit_ok().unwrap_err();
+    /// assert_eq!(bad.code_nonzero().unwrap(), NonZeroI32::try_from(1).unwrap());
+    /// # } // cfg!(unix)
+    /// ```
+    pub fn code_nonzero(&self) -> Option<NonZeroI32> {
+        self.0.code()
+    }
+
+    /// Converts an `ExitStatusError` (back) to an `ExitStatus`.
+    pub fn into_status(&self) -> ExitStatus {
+        ExitStatus(self.0.into())
+    }
+}
+
+#[unstable(feature = "exit_status_error", issue = "84908")]
+impl Into<ExitStatus> for ExitStatusError {
+    fn into(self) -> ExitStatus {
+        ExitStatus(self.0.into())
+    }
+}
+
+#[unstable(feature = "exit_status_error", issue = "84908")]
+impl fmt::Display for ExitStatusError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "process exited unsuccessfully: {}", self.into_status())
+    }
+}
+
+#[unstable(feature = "exit_status_error", issue = "84908")]
+impl crate::error::Error for ExitStatusError {}
+
 /// This type represents the status code a process can return to its
 /// parent under normal termination.
 ///

library/std/src/sys/unix/process/mod.rs

@@ -1,5 +1,5 @@
 pub use self::process_common::{Command, CommandArgs, ExitCode, Stdio, StdioPipes};
-pub use self::process_inner::{ExitStatus, Process};
+pub use self::process_inner::{ExitStatus, ExitStatusError, Process};
 pub use crate::ffi::OsString as EnvKey;
 pub use crate::sys_common::process::CommandEnvs;
 

library/std/src/sys/unix/process/process_fuchsia.rs

@@ -1,7 +1,8 @@
-use crate::convert::TryInto;
+use crate::convert::{TryFrom, TryInto};
 use crate::fmt;
 use crate::io;
 use crate::mem;
+use crate::num::{NonZeroI32, NonZeroI64};
 use crate::ptr;
 
 use crate::sys::process::process_common::*;
@@ -236,8 +237,11 @@ impl Process {
 pub struct ExitStatus(i64);
 
 impl ExitStatus {
-    pub fn success(&self) -> bool {
-        self.code() == Some(0)
+    pub fn exit_ok(&self) -> Result<(), ExitStatusError> {
+        match NonZeroI64::try_from(self.0) {
+            /* was nonzero */ Ok(failure) => Err(ExitStatusError(failure)),
+            /* was zero, couldn't convert */ Err(_) => Ok(()),
+        }
     }
 
     pub fn code(&self) -> Option<i32> {
@@ -306,3 +310,19 @@ impl fmt::Display for ExitStatus {
         write!(f, "exit code: {}", self.0)
     }
 }
+
+#[derive(PartialEq, Eq, Clone, Copy, Debug)]
+pub struct ExitStatusError(NonZeroI64);
+
+impl Into<ExitStatus> for ExitStatusError {
+    fn into(self) -> ExitStatus {
+        ExitStatus(self.0.into())
+    }
+}
+
+impl ExitStatusError {
+    pub fn code(self) -> Option<NonZeroI32> {
+        // fixme: affected by the same bug as ExitStatus::code()
+        ExitStatus(self.0.into()).code().map(|st| st.try_into().unwrap())
+    }
+}

library/std/src/sys/unix/process/process_unix.rs

@@ -1,7 +1,9 @@
-use crate::convert::TryInto;
+use crate::convert::{TryFrom, TryInto};
 use crate::fmt;
 use crate::io::{self, Error, ErrorKind};
 use crate::mem;
+use crate::num::NonZeroI32;
+use crate::os::raw::NonZero_c_int;
 use crate::ptr;
 use crate::sys;
 use crate::sys::cvt;
@@ -491,8 +493,16 @@ impl ExitStatus {
         libc::WIFEXITED(self.0)
     }
 
-    pub fn success(&self) -> bool {
-        self.code() == Some(0)
+    pub fn exit_ok(&self) -> Result<(), ExitStatusError> {
+        // This assumes that WIFEXITED(status) && WEXITSTATUS==0 corresponds to status==0.  This is
+        // true on all actual versios of Unix, is widely assumed, and is specified in SuS
+        // https://pubs.opengroup.org/onlinepubs/9699919799/functions/wait.html .  If it is not
+        // true for a platform pretending to be Unix, the tests (our doctests, and also
+        // procsss_unix/tests.rs) will spot it.  `ExitStatusError::code` assumes this too.
+        match NonZero_c_int::try_from(self.0) {
+            /* was nonzero */ Ok(failure) => Err(ExitStatusError(failure)),
+            /* was zero, couldn't convert */ Err(_) => Ok(()),
+        }
     }
 
     pub fn code(&self) -> Option<i32> {
@@ -547,6 +557,21 @@ impl fmt::Display for ExitStatus {
     }
 }
 
+#[derive(PartialEq, Eq, Clone, Copy, Debug)]
+pub struct ExitStatusError(NonZero_c_int);
+
+impl Into<ExitStatus> for ExitStatusError {
+    fn into(self) -> ExitStatus {
+        ExitStatus(self.0.into())
+    }
+}
+
+impl ExitStatusError {
+    pub fn code(self) -> Option<NonZeroI32> {
+        ExitStatus(self.0.into()).code().map(|st| st.try_into().unwrap())
+    }
+}
+
 #[cfg(test)]
 #[path = "process_unix/tests.rs"]
 mod tests;

library/std/src/sys/unsupported/process.rs

@@ -2,6 +2,7 @@ use crate::ffi::OsStr;
 use crate::fmt;
 use crate::io;
 use crate::marker::PhantomData;
+use crate::num::NonZeroI32;
 use crate::path::Path;
 use crate::sys::fs::File;
 use crate::sys::pipe::AnonPipe;
@@ -97,7 +98,7 @@ impl fmt::Debug for Command {
 pub struct ExitStatus(!);
 
 impl ExitStatus {
-    pub fn success(&self) -> bool {
+    pub fn exit_ok(&self) -> Result<(), ExitStatusError> {
         self.0
     }
 
@@ -134,6 +135,21 @@ impl fmt::Display for ExitStatus {
     }
 }
 
+#[derive(PartialEq, Eq, Clone, Copy, Debug)]
+pub struct ExitStatusError(ExitStatus);
+
+impl Into<ExitStatus> for ExitStatusError {
+    fn into(self) -> ExitStatus {
+        self.0.0
+    }
+}
+
+impl ExitStatusError {
+    pub fn code(self) -> Option<NonZeroI32> {
+        self.0.0
+    }
+}
+
 #[derive(PartialEq, Eq, Clone, Copy, Debug)]
 pub struct ExitCode(bool);