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

#[non_exhaustive] on variant should block cross-crate as casts #91161

Closed
scottmcm opened this issue Nov 23, 2021 · 5 comments · Fixed by #92744
Closed

#[non_exhaustive] on variant should block cross-crate as casts #91161

scottmcm opened this issue Nov 23, 2021 · 5 comments · Fixed by #92744
Assignees
Labels
C-bug Category: This is a bug. E-help-wanted Call for participation: Help is requested to fix this issue. F-non_exhaustive `#![feature(non_exhaustive)]` T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@scottmcm
Copy link
Member

scottmcm commented Nov 23, 2021

This currently compiles

// in crate 1
#[repr(u8)]
enum Foo {
    A,
    #[non_exhaustive]
    B,
}
// in crate 2
fn demo(f: Foo) -> u8 {
    f as u8
}

However, that as cast would be an erroneous "non-primitive cast" if the variant ever got any fields, as it's allowed to do by the #[non_exhaustive].

Thus we need to fix the compiler so that the cast is rejected outside the crate that owns Foo.

@scottmcm scottmcm added T-lang Relevant to the language team, which will review and decide on the PR/issue. F-non_exhaustive `#![feature(non_exhaustive)]` labels Nov 23, 2021
@Badel2
Copy link
Contributor

Badel2 commented Nov 23, 2021

Across different crates the code does not compile, the #[non_exhaustive] attribute seems to mark the enum variant as private:

error[E0603]: unit variant `B` is private
 --> src/main.rs:4:19
  |
4 |     let _x = Foo::B as u8;
  |                   ^ private unit variant
  |
note: the unit variant `B` is defined here
 --> /tmp/issue-91161-dep/src/lib.rs:5:5
  |
5 |     B,
  |     ^

@scottmcm scottmcm added the C-bug Category: This is a bug. label Nov 27, 2021
@scottmcm scottmcm changed the title Does #[non_exhaustive] on variants block cross-crate as casts? #[non_exhaustive] on variant should block cross-crate as casts Nov 27, 2021
scottmcm added a commit to scottmcm/rust that referenced this issue Nov 27, 2021
@scottmcm
Copy link
Member Author

Confirmed this is a problem: #91281

Nominating for lang to confirm that this should be fixed. Presumably via a forward-compat error at first.

@scottmcm scottmcm added the I-lang-nominated Nominated for discussion during a lang team meeting. label Nov 27, 2021
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 29, 2021
…Mark-Simulacrum

Add demonstration test for rust-lang#91161

Since cross-crate things are hard to demonstrate in playground, here's a test showing that something currently works that shouldn't.

cc rust-lang#91161 that tracks fixing the problem (and updating this test)
bors added a commit to rust-lang-ci/rust that referenced this issue Nov 29, 2021
…askrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#91049 (Add a caveat to std::os::windows::fs::symlink_file)
 - rust-lang#91281 (Add demonstration test for rust-lang#91161)
 - rust-lang#91327 (Delete an unreachable codepath from format_args implementation)
 - rust-lang#91336 (Remove unused root_parent.)
 - rust-lang#91349 (Accumulate all values of `-C remark` option)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@joshtriplett
Copy link
Member

We discussed this in today's @rust-lang/lang meeting, and we agree that this is a bug.

We should fix this if possible. We'll need a crater run to confirm that this won't break things.

@scottmcm
Copy link
Member Author

scottmcm commented Nov 30, 2021

And a clarification from the meeting:

This should error in the downstream crate, saying something along the lines of "enums with #[non_exhaustive] variants cannot be as cast (outside the crate which defined them)".

I don't know how hard this is; hopefully it should be relatively straight-forward following all the other non_exhaustive logic.

@scottmcm scottmcm added E-help-wanted Call for participation: Help is requested to fix this issue. and removed I-lang-nominated Nominated for discussion during a lang team meeting. labels Nov 30, 2021
@lambinoo
Copy link
Contributor

@rustbot claim

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Aug 11, 2022
…eign-variants, r=scottmcm

Check if enum from foreign crate has any non exhaustive variants when attempting a cast

Fixes rust-lang#91161

As stated in the issue, this will require a crater run as it might break other people's stuff.
@bors bors closed this as completed in dfddc2f Aug 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. E-help-wanted Call for participation: Help is requested to fix this issue. F-non_exhaustive `#![feature(non_exhaustive)]` T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
4 participants