Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Suggestion: Add ExitCodeExt for Windows #97914

Closed
AronParker opened this issue Jun 9, 2022 · 3 comments · Fixed by #97917
Closed

Suggestion: Add ExitCodeExt for Windows #97914

AronParker opened this issue Jun 9, 2022 · 3 comments · Fixed by #97917

Comments

@AronParker
Copy link
Contributor

I welcome the newly stabilized ExitCode which allows Termination with an exit code.

I am facing some difficulties using it on Windows however, most programs on Windows if they use the exit code at all return an HRESULT or DWORD, neither of which are constructible from the current only available impl From<u8> for ExitCode. Right now I resort to using pointers to access the internals of ExitCode on Windows, which is undesirable. I suggest adding the following windows-specific trait:

impl From<i32> for ExitCode {
    fn from(code: i32) -> Self { ... }
}

Alternatively, a trait named ExitCodeExt could also be added for Windows which allows generation of i32 based error codes.
@eggyal
Copy link
Contributor

eggyal commented Jun 9, 2022

I think this is already understood/desired (see #93445 (comment)) but what's been stabilised so far is an MVP?

@eggyal
Copy link
Contributor

eggyal commented Jun 9, 2022

See also #93840 (comment) (emphasis added):

Custom exit codes: We resolved this by adding From<u8> for ExitCode. We choose to only support u8 initially because it is the least common denominator between the sets of exit codes supported by our current platforms. In the future we anticipate adding platform specific extension traits to ExitCode for constructors from larger or negative numbers, as needed.

@AronParker AronParker mentioned this issue Jun 9, 2022
Merged
@AronParker
Copy link
Contributor Author

AronParker commented Jun 9, 2022 •
edited
Loading

See also #93840 (comment) (emphasis added):

Custom exit codes: We resolved this by adding From<u8> for ExitCode. We choose to only support u8 initially because it is the least common denominator between the sets of exit codes supported by our current platforms. In the future we anticipate adding platform specific extension traits to ExitCode for constructors from larger or negative numbers, as needed.

Thank you! That comment is a very nice find, I wrote a pull request and referenced that comment in it. Feel free to voice your thoughts on it if you'd like.

@AronParker AronParker mentioned this issue Jun 10, 2022
Closed
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jul 7, 2022
@matthiaskrgr
Rollup merge of rust-lang#97917 - AronParker:master, r=ChrisDenton
8d00ca2 
Implement ExitCodeExt for Windows

Fixes rust-lang#97914

### Motivation:

On Windows it is common for applications to return `HRESULT` (`i32`) or `DWORD` (`u32`) values. These stem from COM based components ([HRESULTS](https://docs.microsoft.com/en-us/windows/win32/api/objbase/nf-objbase-coinitialize)), Win32 errors ([GetLastError](https://docs.microsoft.com/en-us/windows/win32/api/errhandlingapi/nf-errhandlingapi-getlasterror)), GUI applications ([WM_QUIT](https://docs.microsoft.com/en-us/windows/win32/winmsg/wm-quit)) and more. The newly stabilized `ExitCode` provides an excellent fit for propagating these values, because `std::process::exit` does not run deconstructors which can result in errors. However, `ExitCode` currently only implements `From<u8> for ExitCode`, which disallows the full range of `i32`/`u32` values. This pull requests attempts to address that shortcoming by providing windows specific extensions that accept a `u32` value (which covers all possible `HRESULTS` and Win32 errors) analog to [ExitStatusExt::from_raw](https://doc.rust-lang.org/std/os/windows/process/trait.ExitStatusExt.html#tymethod.from_raw).

This was also intended by the original Stabilization rust-lang#93840 (comment)  as pointed out by `@eggyal` in rust-lang#97914 (comment):

> Issues around platform specific representations: We resolved this issue by changing the return type of report from i32 to the opaque type ExitCode. __That way we can change the underlying representation without affecting the API, letting us offer full support for platform specific exit code APIs in the future.__

[Emphasis added]

### API

```rust
/// Windows-specific extensions to [`process::ExitCode`].
///
/// 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 = "windows_process_exit_code_from", since = "1.63.0")]
pub trait ExitCodeExt: Sealed {
    /// Creates a new `ExitCode` from the raw underlying `u32` return value of
    /// a process.
    #[stable(feature = "windows_process_exit_code_from", since = "1.63.0")]
    fn from_raw(raw: u32) -> Self;
}

#[stable(feature = "windows_process_exit_code_from", since = "1.63.0")]
impl ExitCodeExt for process::ExitCode {
    fn from_raw(raw: u32) -> Self {
        process::ExitCode::from_inner(From::from(raw))
    }
}
```

### Misc

I apologize in advance if I misplaced any attributes regarding stabilzation, as far as I learned traits are insta-stable so I chose to make them stable. If this is an error, please let me know and I'll correct it. I also added some additional machinery to make it work, analog to [ExitStatus](https://doc.rust-lang.org/std/process/struct.ExitStatus.html#).

EDIT: Proposal: rust-lang/libs-team#48
@bors bors closed this as completed in 6826f33 Jul 7, 2022
workingjubilee pushed a commit to tcdi/postgrestd that referenced this issue Sep 15, 2022
@matthiaskrgr
Rollup merge of #97917 - AronParker:master, r=ChrisDenton
52b2fa9 
Implement ExitCodeExt for Windows

Fixes #97914

### Motivation:

On Windows it is common for applications to return `HRESULT` (`i32`) or `DWORD` (`u32`) values. These stem from COM based components ([HRESULTS](https://docs.microsoft.com/en-us/windows/win32/api/objbase/nf-objbase-coinitialize)), Win32 errors ([GetLastError](https://docs.microsoft.com/en-us/windows/win32/api/errhandlingapi/nf-errhandlingapi-getlasterror)), GUI applications ([WM_QUIT](https://docs.microsoft.com/en-us/windows/win32/winmsg/wm-quit)) and more. The newly stabilized `ExitCode` provides an excellent fit for propagating these values, because `std::process::exit` does not run deconstructors which can result in errors. However, `ExitCode` currently only implements `From<u8> for ExitCode`, which disallows the full range of `i32`/`u32` values. This pull requests attempts to address that shortcoming by providing windows specific extensions that accept a `u32` value (which covers all possible `HRESULTS` and Win32 errors) analog to [ExitStatusExt::from_raw](https://doc.rust-lang.org/std/os/windows/process/trait.ExitStatusExt.html#tymethod.from_raw).

This was also intended by the original Stabilization rust-lang/rust#93840 (comment)  as pointed out by ``@eggyal`` in rust-lang/rust#97914 (comment):

> Issues around platform specific representations: We resolved this issue by changing the return type of report from i32 to the opaque type ExitCode. __That way we can change the underlying representation without affecting the API, letting us offer full support for platform specific exit code APIs in the future.__

[Emphasis added]

### API

```rust
/// Windows-specific extensions to [`process::ExitCode`].
///
/// 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 = "windows_process_exit_code_from", since = "1.63.0")]
pub trait ExitCodeExt: Sealed {
    /// Creates a new `ExitCode` from the raw underlying `u32` return value of
    /// a process.
    #[stable(feature = "windows_process_exit_code_from", since = "1.63.0")]
    fn from_raw(raw: u32) -> Self;
}

#[stable(feature = "windows_process_exit_code_from", since = "1.63.0")]
impl ExitCodeExt for process::ExitCode {
    fn from_raw(raw: u32) -> Self {
        process::ExitCode::from_inner(From::from(raw))
    }
}
```

### Misc

I apologize in advance if I misplaced any attributes regarding stabilzation, as far as I learned traits are insta-stable so I chose to make them stable. If this is an error, please let me know and I'll correct it. I also added some additional machinery to make it work, analog to [ExitStatus](https://doc.rust-lang.org/std/process/struct.ExitStatus.html#).

EDIT: Proposal: rust-lang/libs-team#48
@AronParker AronParker mentioned this issue May 17, 2023
Open
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Implement ExitCodeExt for Windows
2 participants
@AronParker @eggyal