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.

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

Ensure io::Error's bitpacked repr doesn't accidentally impl UnwindSafe #95256

Merged
merged 3 commits into from Mar 29, 2022

Conversation

thomcc
Copy link
Member

@thomcc thomcc commented Mar 24, 2022

Sadly, I'm not sure how to easily test that we don't impl a trait, though (or can libstd use where io::Error: !UnwindSafe or something).

Fixes #95203

@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 24, 2022
@thomcc
Copy link
Member Author

thomcc commented Mar 24, 2022

Oh, maybe a compile_fail doctest.

@csmoe
Copy link
Member

csmoe commented Mar 24, 2022

Maybe you can introduce the static_assertion::assert_no_impl_all!(std::io::Error: UnwindSafe) or write the const item yourself like:

 const _: fn() = || {

            trait AmbiguousIfImpl<A> {
                fn some_item() {}
            }

            impl<T: ?Sized> AmbiguousIfImpl<()> for T {}

            impl<T: ?Sized + UnwindSafe> AmbiguousIfImpl<i32> for T {}

            let _ = <io::Error as AmbiguousIfImpl<_>>::some_item;
        };
    };

@thomcc
Copy link
Member Author

thomcc commented Mar 24, 2022

Up to the reviewer. I'd rather not add a dependency to fix this if we aren't already depending on it.

As it is, the compile_fail doctest does cover it, I've tested that it ./x.py test library/std fails without the corresponding phantomdata change.

@KamilaBorowska
Copy link
Contributor

KamilaBorowska commented Mar 24, 2022

I think this could be tested with UI test, see tests in src/test/ui/not-panic directory.

@thomcc
Copy link
Member Author

thomcc commented Mar 24, 2022

A downside of using UI tests (compared to the compile_fail doctest) would be that ./x.py test library/std wouldn't test it.

This is not a dealbreaker, but FWIW, I almost never run the full tests, and just let CI get them, but do run the tests for a specific area.

Co-authored-by: Mara Bos <m-ou.se@m-ou.se>
@m-ou-se
Copy link
Member

m-ou-se commented Mar 29, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Mar 29, 2022

📌 Commit 3ac93ab has been approved by m-ou-se

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 29, 2022
@m-ou-se
Copy link
Member

m-ou-se commented Mar 29, 2022

(FWIW: We have compile_fail doctests on private types/fields in more places in core and std, for similar things.)

bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 29, 2022
Rollup of 4 pull requests

Successful merges:

 - rust-lang#93840 (Stabilize Termination and ExitCode)
 - rust-lang#95256 (Ensure io::Error's bitpacked repr doesn't accidentally impl UnwindSafe)
 - rust-lang#95386 (Suggest wrapping patterns in enum variants)
 - rust-lang#95437 (diagnostics: regression test for derive bounds)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@cuviper cuviper added beta-nominated Nominated for backporting to the compiler in the beta channel. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 29, 2022
@bors bors merged commit 3208ed7 into rust-lang:master Mar 29, 2022
@rustbot rustbot added this to the 1.61.0 milestone Mar 29, 2022
@thomcc thomcc deleted the fix-unwind-safe branch March 29, 2022 23:55
@m-ou-se m-ou-se added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Mar 30, 2022
@pietroalbini pietroalbini removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Apr 4, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 4, 2022
…roalbini

Prepare 1.60.0 stable release

This PR bumps the channel to stable, and backports the following PRs:

* Miscellaneous release notes updates.
* rust-lang#94817
* rust-lang#95256

r? `@ghost`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

io::Error is UnwindSafe, but only on 64-bit platforms
10 participants