-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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.
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
Make ExitStatus implement Default #106425
Conversation
Even where actually running processes is not supported. Needed for the next commit. The manual trait implementations now belong on ExitStatusError, which still can't exist.
r? @thomcc (rustbot has picked a reviewer for you, use r? to override) |
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
@rustbot label +T-libs-api -T-libs |
This comment was marked as resolved.
This comment was marked as resolved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Impl looks fine, but will need API review.
This should probably have ACP, tbh |
Ah, sorry. Now done in rust-lang/libs-team#158 The rustbot comment says (emphasis added):
Maybe that isn't quite right? I'm not sure what it should say precisely (or where to edit it). |
@rfcbot merge (See rust-lang/libs-team#158 for motivation/details.) |
Team member @m-ou-se has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
I see that there's nothing in the PR that documents what the default is. I think it should at least say somewhere what that default is. Having it come from a |
It also feels like it make make sense to make |
@scottmcm I generally agree with your comments. In rust-lang/libs-team#158 @m-ou-se suggested In summary, I think we should
I have put this on my personal todo list. Items 1 and 2 are insta-stable. Should I make an ACP, or can we do the FCP in the MR? It seems to me that this is all relatively uncontroversial so maybe going straight to MR is best. I don't think any of those are ABI breaks from where this MR gets us, so this MR can go through as-is, even though it has room for improvement. |
Oh. I see I lost the review race there. I will rewind and do the followup as its own MR. |
dcbd04d
to
1f1d49a
Compare
As suggested here rust-lang#106425 (comment)
@workingjubilee I think my push and rewind have caused the bots to become confused. @bors said "this is now in the queue" but clicking through to the queue it isn't showing as Approved. Would you mind re-approving 1f1d49a ? Sorry for the inconvenience. |
@bors r+ |
…olnay Make ExitStatus implement Default And, necessarily, make it inhabited even on platforms without processes. I noticed while preparing rust-lang/rfcs#3362 that there was no way for anyone to construct an `ExitStatus`. This would be insta-stable so needs an FCP.
…iaskrgr Rollup of 11 pull requests Successful merges: - rust-lang#106425 (Make ExitStatus implement Default) - rust-lang#113480 (add aarch64-unknown-teeos target) - rust-lang#113586 (Mention style for new syntax in tracking issue template) - rust-lang#113593 (CFI: Fix error compiling core with LLVM CFI enabled) - rust-lang#114612 (update llvm-wrapper include to silence deprecation warning) - rust-lang#114613 (Prevent constant rebuilds of `rustc-main` (and thus everything else)) - rust-lang#114615 (interpret: remove incomplete protection against invalid where clauses) - rust-lang#114628 (Allowing re-implementation of mir_drops_elaborated query) - rust-lang#114629 (tests: Uncomment now valid GAT code behind FIXME) - rust-lang#114630 (Migrate GUI colors test to original CSS color format) - rust-lang#114631 (add provisional cache test for new solver) r? `@ghost` `@rustbot` modify labels: rollup
…m-ou-se Improve docs for impl Default for ExitStatus This addresses a review comment in rust-lang#106425 (which is on the way to being merged I think). Some of the other followup work is more complicated so I'm going to do individual MRs. ~~Note this branch is on top of rust-lang#106425~~
…m-ou-se Improve docs for impl Default for ExitStatus This addresses a review comment in rust-lang#106425 (which is on the way to being merged I think). Some of the other followup work is more complicated so I'm going to do individual MRs. ~~Note this branch is on top of rust-lang#106425~~
…m-ou-se Improve docs for impl Default for ExitStatus This addresses a review comment in rust-lang#106425 (which is on the way to being merged I think). Some of the other followup work is more complicated so I'm going to do individual MRs. ~~Note this branch is on top of rust-lang#106425~~
…m-ou-se Improve docs for impl Default for ExitStatus This addresses a review comment in rust-lang#106425 (which is on the way to being merged I think). Some of the other followup work is more complicated so I'm going to do individual MRs. ~~Note this branch is on top of rust-lang#106425~~
Language -------- - [Uplift `clippy::fn_null_check` lint as `useless_ptr_null_checks`.] (rust-lang/rust#111717) - [Make `noop_method_call` warn by default.] (rust-lang/rust#111916) - [Support interpolated block for `try` and `async` in macros.] (rust-lang/rust#112953) - [Make `unconditional_recursion` lint detect recursive drops.] (rust-lang/rust#113902) - [Future compatibility warning for some impls being incorrectly considered not overlapping.] (rust-lang/rust#114023) - [The `invalid_reference_casting` lint is now **deny-by-default** (instead of allow-by-default)] (rust-lang/rust#112431) Compiler -------- - [Write version information in a `.comment` section like GCC/Clang.] (rust-lang/rust#97550) - [Add documentation on v0 symbol mangling.] (rust-lang/rust#97571) - [Stabilize `extern "thiscall"` and `"thiscall-unwind"` ABIs.] (rust-lang/rust#114562) - [Only check outlives goals on impl compared to trait.] (rust-lang/rust#109356) - [Infer type in irrefutable slice patterns with fixed length as array.] (rust-lang/rust#113199) - [Discard default auto trait impls if explicit ones exist.] (rust-lang/rust#113312) - Add several new tier 3 targets: - [`aarch64-unknown-teeos`] (rust-lang/rust#113480) - [`csky-unknown-linux-gnuabiv2`] (rust-lang/rust#113658) - [`riscv64-linux-android`] (rust-lang/rust#112858) - [`riscv64gc-unknown-hermit`] (rust-lang/rust#114004) - [`x86_64-unikraft-linux-musl`] (rust-lang/rust#113411) - [`x86_64-unknown-linux-ohos`] (rust-lang/rust#113061) - [Add `wasm32-wasi-preview1-threads` as a tier 2 target.] (rust-lang/rust#112922) Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support. Libraries --------- - [Add `Read`, `Write` and `Seek` impls for `Arc<File>`.] (rust-lang/rust#94748) - [Merge functionality of `io::Sink` into `io::Empty`.] (rust-lang/rust#98154) - [Implement `RefUnwindSafe` for `Backtrace`] (rust-lang/rust#100455) - [Make `ExitStatus` implement `Default`] (rust-lang/rust#106425) - [`impl SliceIndex<str> for (Bound<usize>, Bound<usize>)`] (rust-lang/rust#111081) - [Change default panic handler message format.] (rust-lang/rust#112849) - [Cleaner `assert_eq!` & `assert_ne!` panic messages.] (rust-lang/rust#111071) - [Correct the (deprecated) Android `stat` struct definitions.] (rust-lang/rust#113130) Stabilized APIs --------------- - [Unsigned `{integer}::div_ceil`] (https://doc.rust-lang.org/stable/std/primitive.u32.html#method.div_ceil) - [Unsigned `{integer}::next_multiple_of`] (https://doc.rust-lang.org/stable/std/primitive.u32.html#method.next_multiple_of) - [Unsigned `{integer}::checked_next_multiple_of`] (https://doc.rust-lang.org/stable/std/primitive.u32.html#method.checked_next_multiple_of) - [`std::ffi::FromBytesUntilNulError`] (https://doc.rust-lang.org/stable/std/ffi/struct.FromBytesUntilNulError.html) - [`std::os::unix::fs::chown`] (https://doc.rust-lang.org/stable/std/os/unix/fs/fn.chown.html) - [`std::os::unix::fs::fchown`] (https://doc.rust-lang.org/stable/std/os/unix/fs/fn.fchown.html) - [`std::os::unix::fs::lfchown`] (https://doc.rust-lang.org/stable/std/os/unix/fs/fn.lchown.html) - [`LocalKey::<Cell<T>>::get`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.get) - [`LocalKey::<Cell<T>>::set`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.set) - [`LocalKey::<Cell<T>>::take`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.take) - [`LocalKey::<Cell<T>>::replace`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.replace) - [`LocalKey::<RefCell<T>>::with_borrow`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.with_borrow) - [`LocalKey::<RefCell<T>>::with_borrow_mut`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.with_borrow_mut) - [`LocalKey::<RefCell<T>>::set`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.set-1) - [`LocalKey::<RefCell<T>>::take`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.take-1) - [`LocalKey::<RefCell<T>>::replace`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.replace-1) These APIs are now stable in const contexts: - [`rc::Weak::new`] (https://doc.rust-lang.org/stable/alloc/rc/struct.Weak.html#method.new) - [`sync::Weak::new`] (https://doc.rust-lang.org/stable/alloc/sync/struct.Weak.html#method.new) - [`NonNull::as_ref`] (https://doc.rust-lang.org/stable/core/ptr/struct.NonNull.html#method.as_ref) Cargo ----- - [Encode URL params correctly for `SourceId` in `Cargo.lock`.] (rust-lang/cargo#12280) - [Bail out an error when using `cargo::` in custom build script.] (rust-lang/cargo#12332) Compatibility Notes ------------------- - [Update the minimum external LLVM to 15.] (rust-lang/rust#114148) - [Check for non-defining uses of return position `impl Trait`.] (rust-lang/rust#112842) Internal Changes ---------------- These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools. - [Remove LLVM pointee types, supporting only opaque pointers.] (rust-lang/rust#105545) - [Port PGO/LTO/BOLT optimized build pipeline to Rust.] (rust-lang/rust#112235) - [Replace in-tree `rustc_apfloat` with the new version of the crate.] (rust-lang/rust#113843) - [Update to LLVM 17.] (rust-lang/rust#114048) - [Add `internal_features` lint for internal unstable features.] (rust-lang/rust#108955) - [Mention style for new syntax in tracking issue template.] (rust-lang/rust#113586)
impl Default for ExitCode As suggested here rust-lang#106425 (comment) Needs FCP since this is an insta-stable impl. Ideally we would have `impl From<ExitCode> for ExitStatus` and implement the default `ExitStatus` using that. That is sadly not so easy because of the various strange confusions about `ExitCode` (unix: exit status) vs `ExitStatus` (unix: wait status) in the not-really-unix platforms in `library//src/sys/unix/process`. I'll try to follow that up.
As suggested here rust-lang#106425 (comment)
impl Default for ExitCode As suggested here rust-lang#106425 (comment) Needs FCP since this is an insta-stable impl. Ideally we would have `impl From<ExitCode> for ExitStatus` and implement the default `ExitStatus` using that. That is sadly not so easy because of the various strange confusions about `ExitCode` (unix: exit status) vs `ExitStatus` (unix: wait status) in the not-really-unix platforms in `library//src/sys/unix/process`. I'll try to follow that up.
Pkgsrc changes: * Adjust patches and cargo checksums to new versions. * For an external LLVM, set dependency of llvm >= 15, in accordance with the upstream changes. * Add a patch with a backport from LLVM 17.0.3 fixing codegen for PPC, ref. rust-lang/rust#116845 Upstream changes: Version 1.73.0 (2023-10-05) ========================== Language -------- - [Uplift `clippy::fn_null_check` lint as `useless_ptr_null_checks`.] (rust-lang/rust#111717) - [Make `noop_method_call` warn by default.] (rust-lang/rust#111916) - [Support interpolated block for `try` and `async` in macros.] (rust-lang/rust#112953) - [Make `unconditional_recursion` lint detect recursive drops.] (rust-lang/rust#113902) - [Future compatibility warning for some impls being incorrectly considered not overlapping.] (rust-lang/rust#114023) - [The `invalid_reference_casting` lint is now **deny-by-default** (instead of allow-by-default)] (rust-lang/rust#112431 Compiler -------- - [Write version information in a `.comment` section like GCC/Clang.] (rust-lang/rust#97550) - [Add documentation on v0 symbol mangling.] (rust-lang/rust#97571) - [Stabilize `extern "thiscall"` and `"thiscall-unwind"` ABIs.] (rust-lang/rust#114562) - [Only check outlives goals on impl compared to trait.] (rust-lang/rust#109356) - [Infer type in irrefutable slice patterns with fixed length as array.] (rust-lang/rust#113199) - [Discard default auto trait impls if explicit ones exist.] (rust-lang/rust#113312) - Add several new tier 3 targets: - [`aarch64-unknown-teeos`] (rust-lang/rust#113480) - [`csky-unknown-linux-gnuabiv2`] (rust-lang/rust#113658) - [`riscv64-linux-android`] (rust-lang/rust#112858) - [`riscv64gc-unknown-hermit`] (rust-lang/rust#114004) - [`x86_64-unikraft-linux-musl`] (rust-lang/rust#113411) - [`x86_64-unknown-linux-ohos`] (rust-lang/rust#113061) - [Add `wasm32-wasi-preview1-threads` as a tier 2 target.] (rust-lang/rust#112922) Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support. Libraries --------- - [Add `Read`, `Write` and `Seek` impls for `Arc<File>`.] (rust-lang/rust#94748) - [Merge functionality of `io::Sink` into `io::Empty`.] (rust-lang/rust#98154) - [Implement `RefUnwindSafe` for `Backtrace`] (rust-lang/rust#100455) - [Make `ExitStatus` implement `Default`] (rust-lang/rust#106425) - [`impl SliceIndex<str> for (Bound<usize>, Bound<usize>)`] (rust-lang/rust#111081) - [Change default panic handler message format.] (rust-lang/rust#112849) - [Cleaner `assert_eq!` & `assert_ne!` panic messages.] (rust-lang/rust#111071) - [Correct the (deprecated) Android `stat` struct definitions.] (rust-lang/rust#113130) Stabilized APIs --------------- - [Unsigned `{integer}::div_ceil`] (https://doc.rust-lang.org/stable/std/primitiv e.u32.html#method.div_ceil) - [Unsigned `{integer}::next_multiple_of`] (https://doc.rust-lang.org/stable/std/primitive.u32.html#method.next_multiple_of) - [Unsigned `{integer}::checked_next_multiple_of`] (https://doc.rust-lang.org/stable/std/primitive.u32.html#method.checked_next_multiple_of) - [`std::ffi::FromBytesUntilNulError`] (https://doc.rust-lang.org/stable/std/ffi/struct.FromBytesUntilNulError.html) - [`std::os::unix::fs::chown`] (https://doc.rust-lang.org/stable/std/os/unix/fs/fn.chown.html) - [`std::os::unix::fs::fchown`] (https://doc.rust-lang.org/stable/std/os/unix/fs/fn.fchown.html) - [`std::os::unix::fs::lfchown`] (https://doc.rust-lang.org/stable/std/os/unix/fs/fn.lchown.html) - [`LocalKey::<Cell<T>>::get`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.get) - [`LocalKey::<Cell<T>>::set`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.set) - [`LocalKey::<Cell<T>>::take`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.take) - [`LocalKey::<Cell<T>>::replace`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.replace) - [`LocalKey::<RefCell<T>>::with_borrow`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.with_borrow) - [`LocalKey::<RefCell<T>>::with_borrow_mut`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.with_borrow_mut) - [`LocalKey::<RefCell<T>>::set`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.set-1) - [`LocalKey::<RefCell<T>>::take`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.take-1) - [`LocalKey::<RefCell<T>>::replace`] (https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.replace-1) These APIs are now stable in const contexts: - [`rc::Weak::new`] (https://doc.rust-lang.org/stable/alloc/rc/struct.Weak.html#method.new) - [`sync::Weak::new`] (https://doc.rust-lang.org/stable/alloc/sync/struct.Weak.html#method.new) - [`NonNull::as_ref`] (https://doc.rust-lang.org/stable/core/ptr/struct.NonNull.html#method.as_ref) Cargo ----- - [Encode URL params correctly for `SourceId` in `Cargo.lock`.] (rust-lang/cargo#12280) - [Bail out an error when using `cargo::` in custom build script.] (rust-lang/cargo#12332) Misc ---- Compatibility Notes ------------------- - [Update the minimum external LLVM to 15.] (rust-lang/rust#114148) - [Check for non-defining uses of return position `impl Trait`.] (rust-lang/rust#112842) Internal Changes ---------------- These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools. - [Remove LLVM pointee types, supporting only opaque pointers.] (rust-lang/rust#105545) - [Port PGO/LTO/BOLT optimized build pipeline to Rust.] (rust-lang/rust#112235) - [Replace in-tree `rustc_apfloat` with the new version of the crate.] (rust-lang/rust#113843) - [Update to LLVM 17.] (rust-lang/rust#114048) - [Add `internal_features` lint for internal unstable features.] (rust-lang/rust#108955) - [Mention style for new syntax in tracking issue template.] (rust-lang/rust#113586)
And, necessarily, make it inhabited even on platforms without processes.
I noticed while preparing rust-lang/rfcs#3362 that there was no way for anyone to construct an
ExitStatus
.This would be insta-stable so needs an FCP.