-
-
Notifications
You must be signed in to change notification settings - Fork 14.3k
Replace -Zon-broken-pipe=... with Externally Implementable Item #[std::io::on_broken_pipe]
#150591
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
base: main
Are you sure you want to change the base?
Conversation
#![feature(on_broken_pipe)] code
This comment has been minimized.
This comment has been minimized.
7ce0046 to
5825fb6
Compare
So that the name is semantically correct for `#[feature(on_broken_pipe)]` (and actually also `-Zon-broken-pipe` if we end up keeping that).
…position So we don't need to update the test when the implementation changes. The default will work fine. No need to set it explicitly to Error. That is the default.
5825fb6 to
7b52018
Compare
#![feature(on_broken_pipe)] code-Zon-broken-pipe=... with Externally Implementable Item #[std::io::on_broken_pipe() -> std::io::OnBrokenPipe
-Zon-broken-pipe=... with Externally Implementable Item #[std::io::on_broken_pipe() -> std::io::OnBrokenPipe-Zon-broken-pipe=... with Externally Implementable Item #[std::io::on_broken_pipe]
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| // `std::io::on_broken_pipe` is implemented via EII and becomes an `extern "Rust"` | ||
| // function without a body. Under v0 mangling this shows up with a mangled symbol name | ||
| // (e.g. `_RNv..._3std2io14on_broken_pipe`). | ||
| // | ||
| // Miri does not support the real behavior (changing host SIGPIPE disposition), so we | ||
| // treat this as a no-op and request inheriting SIGPIPE. | ||
| name if name == "on_broken_pipe" || name.contains("std2io14on_broken_pipe") => { |
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.
I assume this is just for testing for now, but this hack is definitely not something we'd want to land.
The proper fix here is to implement EII in Miri, so that the real implementation gets invoked. We already have hacks in place to ignore std trying to change the host SIGPIPE disposition.
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.
Yep that was indeed a hack and not intended for review. I just wanted to see how far away I was from getting PR CI to pass.
Your comment was still helpful however. I have removed the code and replaced it with
#[cfg(not(miri))]
let on_broken_pipe = crate::io::on_broken_pipe();
#[cfg(miri)] // FIXME: Add proper miri support. See https://github.com/rust-lang/rust/pull/150591#discussion_r2665432118.
let on_broken_pipe = OnBrokenPipe::BackwardsCompatible;and will probably create a proper issue for it later.
(Note that this code is still not ready for review.)
rustc book: fix grammar It was added in ddee45e ([here](https://github.com/rust-lang/rust/pull/97802/changes#diff-85dc642a7bdad363a9e37d2491230280fcd977391ba7a242bda2da9ddb55f654) to be specific) when SIGPIPE was controlled with an attribute on `fn main()` which meant it could also be combined with `#[rustc_main]`: #[unix_sigpipe = "sig_dfl"] #[rustc_main] fn rustc_main() { The test stopped being needed in cde0cde when `#[unix_sigpipe = "..."]` was replaced by `-Zon-broken-pipe=...`. And it will not be needed when `-Zon-broken-pipe=...` is replaced by an Externally Implementable Item (see rust-lang#150591). Let's remove this test. Tracking issue: - rust-lang#150588
tests/ui/runtime/on-broken-pipe/with-rustc_main.rs: Not needed so remove It was added in ddee45e ([here](https://github.com/rust-lang/rust/pull/97802/changes#diff-85dc642a7bdad363a9e37d2491230280fcd977391ba7a242bda2da9ddb55f654) to be specific) when SIGPIPE was controlled with an attribute on `fn main()` which meant it could also be combined with `#[rustc_main]`: #[unix_sigpipe = "sig_dfl"] #[rustc_main] fn rustc_main() { The test stopped being needed in cde0cde when `#[unix_sigpipe = "..."]` was replaced by `-Zon-broken-pipe=...`. And it will not be needed when `-Zon-broken-pipe=...` is replaced by an Externally Implementable Item (see rust-lang#150591). Let's remove this test. Tracking issue: - rust-lang#150588
tests/ui/runtime/on-broken-pipe/with-rustc_main.rs: Not needed so remove It was added in ddee45e ([here](https://github.com/rust-lang/rust/pull/97802/changes#diff-85dc642a7bdad363a9e37d2491230280fcd977391ba7a242bda2da9ddb55f654) to be specific) when SIGPIPE was controlled with an attribute on `fn main()` which meant it could also be combined with `#[rustc_main]`: #[unix_sigpipe = "sig_dfl"] #[rustc_main] fn rustc_main() { The test stopped being needed in cde0cde when `#[unix_sigpipe = "..."]` was replaced by `-Zon-broken-pipe=...`. And it will not be needed when `-Zon-broken-pipe=...` is replaced by an Externally Implementable Item (see rust-lang#150591). Let's remove this test. Tracking issue: - rust-lang#150588
tests/ui/runtime/on-broken-pipe/with-rustc_main.rs: Not needed so remove It was added in ddee45e ([here](https://github.com/rust-lang/rust/pull/97802/changes#diff-85dc642a7bdad363a9e37d2491230280fcd977391ba7a242bda2da9ddb55f654) to be specific) when SIGPIPE was controlled with an attribute on `fn main()` which meant it could also be combined with `#[rustc_main]`: #[unix_sigpipe = "sig_dfl"] #[rustc_main] fn rustc_main() { The test stopped being needed in cde0cde when `#[unix_sigpipe = "..."]` was replaced by `-Zon-broken-pipe=...`. And it will not be needed when `-Zon-broken-pipe=...` is replaced by an Externally Implementable Item (see rust-lang#150591). Let's remove this test. Tracking issue: - rust-lang#150588
Fix `alloc_error_handler` signature mismatch It was added in ddee45e ([here](https://github.com/rust-lang/rust/pull/97802/changes#diff-85dc642a7bdad363a9e37d2491230280fcd977391ba7a242bda2da9ddb55f654) to be specific) when SIGPIPE was controlled with an attribute on `fn main()` which meant it could also be combined with `#[rustc_main]`: #[unix_sigpipe = "sig_dfl"] #[rustc_main] fn rustc_main() { The test stopped being needed in cde0cde when `#[unix_sigpipe = "..."]` was replaced by `-Zon-broken-pipe=...`. And it will not be needed when `-Zon-broken-pipe=...` is replaced by an Externally Implementable Item (see rust-lang#150591). Let's remove this test. Tracking issue: - rust-lang#150588
Rollup merge of #150757 - rm-unneded-test, r=bjorn3 Fix `alloc_error_handler` signature mismatch It was added in ddee45e ([here](https://github.com/rust-lang/rust/pull/97802/changes#diff-85dc642a7bdad363a9e37d2491230280fcd977391ba7a242bda2da9ddb55f654) to be specific) when SIGPIPE was controlled with an attribute on `fn main()` which meant it could also be combined with `#[rustc_main]`: #[unix_sigpipe = "sig_dfl"] #[rustc_main] fn rustc_main() { The test stopped being needed in cde0cde when `#[unix_sigpipe = "..."]` was replaced by `-Zon-broken-pipe=...`. And it will not be needed when `-Zon-broken-pipe=...` is replaced by an Externally Implementable Item (see #150591). Let's remove this test. Tracking issue: - #150588
76f940b to
fc9e23b
Compare
|
The job Click to see the possible cause of the failure (guessed by this bot) |
The code in this PR also acts as a regression test for the fix to #150588 which I will split out in a separate PR.
TODO
missing stability attributeerrors with trivial Externally Implementable Item (eii) function inlibrary/std#150514 first in a separate PR (see compiler: Make Externally Implementable Items (eii) in std build #150592)Tracking Issues:
#[std::io::on_broken_pipe]#150588