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

Provide ExitStatusError

Closes #73125

In MR #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 https://github.com/rust-lang/rust/issues/73125#issuecomment-771092741

6 files changed, 94 insertions, 10 deletions

diff --git a/library/std/src/sys/unix/process/mod.rs b/library/std/src/sys/unix/process/mod.rs
index f67c70c0177..b5a19ed54a2 100644
--- a/library/std/src/sys/unix/process/mod.rs
+++ b/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;
 
diff --git a/library/std/src/sys/unix/process/process_fuchsia.rs b/library/std/src/sys/unix/process/process_fuchsia.rs
index b19ad4ccdc7..507abb27871 100644
--- a/library/std/src/sys/unix/process/process_fuchsia.rs
+++ b/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())
+    }
+}
diff --git a/library/std/src/sys/unix/process/process_unix.rs b/library/std/src/sys/unix/process/process_unix.rs
index 08b500b9c82..7f8065e7500 100644
--- a/library/std/src/sys/unix/process/process_unix.rs
+++ b/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;
diff --git a/library/std/src/sys/unsupported/process.rs b/library/std/src/sys/unsupported/process.rs
index 38ac0a1ddd5..7846e43cfb5 100644
--- a/library/std/src/sys/unsupported/process.rs
+++ b/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
     }
 
@@ -135,6 +136,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);
 
 impl ExitCode {
diff --git a/library/std/src/sys/windows/c.rs b/library/std/src/sys/windows/c.rs
index 7ea6048e94a..e91c489361e 100644
--- a/library/std/src/sys/windows/c.rs
+++ b/library/std/src/sys/windows/c.rs
@@ -4,6 +4,7 @@
 #![cfg_attr(test, allow(dead_code))]
 #![unstable(issue = "none", feature = "windows_c")]
 
+use crate::os::raw::NonZero_c_ulong;
 use crate::os::raw::{c_char, c_int, c_long, c_longlong, c_uint, c_ulong, c_ushort};
 use crate::ptr;
 
@@ -13,6 +14,7 @@ pub use self::EXCEPTION_DISPOSITION::*;
 pub use self::FILE_INFO_BY_HANDLE_CLASS::*;
 
 pub type DWORD = c_ulong;
+pub type NonZeroDWORD = NonZero_c_ulong;
 pub type HANDLE = LPVOID;
 pub type HINSTANCE = HANDLE;
 pub type HMODULE = HINSTANCE;
diff --git a/library/std/src/sys/windows/process.rs b/library/std/src/sys/windows/process.rs
index a5799606142..81dbea4a067 100644
--- a/library/std/src/sys/windows/process.rs
+++ b/library/std/src/sys/windows/process.rs
@@ -5,6 +5,7 @@ mod tests;
 
 use crate::borrow::Borrow;
 use crate::collections::BTreeMap;
+use crate::convert::{TryFrom, TryInto};
 use crate::env;
 use crate::env::split_paths;
 use crate::ffi::{OsStr, OsString};
@@ -12,10 +13,12 @@ use crate::fmt;
 use crate::fs;
 use crate::io::{self, Error, ErrorKind};
 use crate::mem;
+use crate::num::NonZeroI32;
 use crate::os::windows::ffi::OsStrExt;
 use crate::path::Path;
 use crate::ptr;
 use crate::sys::c;
+use crate::sys::c::NonZeroDWORD;
 use crate::sys::cvt;
 use crate::sys::fs::{File, OpenOptions};
 use crate::sys::handle::Handle;
@@ -376,8 +379,11 @@ impl Process {
 pub struct ExitStatus(c::DWORD);
 
 impl ExitStatus {
-    pub fn success(&self) -> bool {
-        self.0 == 0
+    pub fn exit_ok(&self) -> Result<(), ExitStatusError> {
+        match NonZeroDWORD::try_from(self.0) {
+            /* was nonzero */ Ok(failure) => Err(ExitStatusError(failure)),
+            /* was zero, couldn't convert */ Err(_) => Ok(()),
+        }
     }
     pub fn code(&self) -> Option<i32> {
         Some(self.0 as i32)
@@ -407,6 +413,21 @@ impl fmt::Display for ExitStatus {
 }
 
 #[derive(PartialEq, Eq, Clone, Copy, Debug)]
+pub struct ExitStatusError(c::NonZeroDWORD);
+
+impl Into<ExitStatus> for ExitStatusError {
+    fn into(self) -> ExitStatus {
+        ExitStatus(self.0.into())
+    }
+}
+
+impl ExitStatusError {
+    pub fn code(self) -> Option<NonZeroI32> {
+        Some((u32::from(self.0) as i32).try_into().unwrap())
+    }
+}
+
+#[derive(PartialEq, Eq, Clone, Copy, Debug)]
 pub struct ExitCode(c::DWORD);
 
 impl ExitCode {