unix: impl ExitStatusExt for ExitStatusError

It is unergnomic to have to say things like
   bad.into_status().signal()

Implementing `ExitStatusExt` for `ExitStatusError` fixes this.
Unfortunately it does mean making a previously-infallible method
capable of panicing, although of course the existing impl remains
infallible.

The alternative would be a whole new `ExitStatusErrorExt` trait.

`<ExitStatus as ExitStatusExt>::into_raw()` is not particularly
ergonomic to call because of the often-required type annotation.
See for example the code in the test case in
  library/std/src/sys/unix/process/process_unix/tests.rs

Perhaps we should provide equivalent free functions for `ExitStatus`
and `ExitStatusExt` in std::os::unix::process and maybe deprecate this
trait method.  But I think that is for the future.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>

diff --git a/library/std/src/os/unix/process.rs b/library/std/src/os/unix/process.rs
index 355855bcd10e203d6921f3560833bc3270fa4513..21da8ba15de0130db9660a8625e12592b73f7d8f 100644 (file)
--- a/library/std/src/os/unix/process.rs
+++ b/library/std/src/os/unix/process.rs
@@ -195,28 +195,62 @@ fn arg0<S>(&mut self, arg: S) -> &mut 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` and `ExitStatusError` **do not necessarily represent an exit status**, as
+/// passed to the `exit` system call or returned by
+/// [`ExitStatus::code()`](crate::process::ExitStatus::code).  They represents **any wait status**
+/// (or any nonzero wait status, respectively), 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;
 
     /// If the process was terminated by a signal, returns that signal.
     ///
     /// In other words, if `WIFSIGNALED`, this returns `WTERMSIG`.
+    ///
+    /// # Examples
+    /// ```
+    /// #![feature(exit_status_error)]
+    /// use std::process::{Command, ExitStatusError};
+    /// use std::os::unix::process::ExitStatusExt;
+    ///
+    /// fn run(script: &str) -> Result<(), ExitStatusError> {
+    ///     Command::new("sh").args(&["-ec",script])
+    ///         .status().expect("failed to fork/exec sh")
+    ///         .exit_ok()
+    ///         .or_else(|bad| {
+    ///             if bad.signal() == Some(13) /*PIPE*/ {
+    ///                 Ok(())
+    ///             } else {
+    ///                 Err(bad)
+    ///             }
+    ///         })
+    /// }
+    ///
+    /// run("exit").unwrap();
+    /// run("kill -PIPE $$").unwrap();
+    /// run("exit 42").unwrap_err();
+    /// ```
     #[stable(feature = "rust1", since = "1.0.0")]
     fn signal(&self) -> Option<i32>;
 
@@ -272,6 +306,35 @@ fn into_raw(self) -> i32 {
     }
 }
 
+#[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]

diff --git a/library/std/src/process.rs b/library/std/src/process.rs
index 6020ecf685366d7f5cd67d5dcc819f423ff07d0d..c55cc18b2cff5f982ed74fbbd710db6785898416 100644 (file)
--- a/library/std/src/process.rs
+++ b/library/std/src/process.rs
@@ -1500,6 +1500,10 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
     }
 }
 
+/// 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`].
@@ -1536,9 +1540,8 @@ impl ExitStatusError {
     /// 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
-    /// [`ExitStatusExt`](crate::os::unix::process::ExitStatusExt) (possibly after getting the
-    /// general `ExitStatus` by using [`status()`](ExitStatusError::status).
+    /// 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.