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

E0603 is misleading on non-exhaustive enums #82788

Open
jhpratt opened this issue Mar 5, 2021 · 2 comments
Open

E0603 is misleading on non-exhaustive enums #82788

jhpratt opened this issue Mar 5, 2021 · 2 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints D-confusing Diagnostics: Confusing error or lint that should be reworked. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jhpratt
Copy link
Member

jhpratt commented Mar 5, 2021

Given the following code: (no link, as not reproducible in a single file)

Library:

pub enum Foo {
    Alpha,
    #[non_exhaustive]
    Beta,
}

impl Foo {
    pub fn beta() -> Self {
        Self::Beta
    }
}

External user (in this case a test):

use example::Foo;

#[test]
fn demo() {
    assert!(matches!(
        Foo::beta(),
        Foo::Beta
    ));
}

The current output is:

error[E0603]: unit variant `Beta` is private
 --> tests/demo.rs:7:14
  |
7 |         Foo::Beta
  |              ^^^^ private unit variant
  |
note: the unit variant `Beta` is defined here
 --> /home/jhpratt/example/src/lib.rs:4:5
  |
4 |     Beta,
  |     ^^^^

Ideally the output should look like:

error[E0638]: `..` required with variant marked as non-exhaustive
 --> tests/demo.rs:7:9
  |
7 |         Foo::Beta
  |         ^^^^^^^^^
  |
help: add `..` at the end of the field list to ignore all other fields
  |
7 |         Foo::Beta { .. }

Right now, the cause of the error is misleading, as it seems to imply that an enum variant is private (which is impossible). The error message should be updated to match the error provided when there are some fields present, but not the .. as required. The exact wording might vary a bit, but I copied it verbatim here. Given the explanation of E0603, I believe that this is the wrong error code altogether.

@rustbot modify labels to +A-diagnostics +T-compiler +D-confusing +D-incorrect

@jhpratt jhpratt added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 5, 2021
@rustbot rustbot added D-confusing Diagnostics: Confusing error or lint that should be reworked. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. labels Mar 5, 2021
@1c3t3a
Copy link
Contributor

1c3t3a commented Mar 9, 2021

Ahh looking at the tests for the corresponding RFC, this seems like desired behavior for Unit like enum Variants:

fn main() {
let variant_struct = NonExhaustiveVariants::Struct { field: 640 };
//~^ ERROR cannot create non-exhaustive variant
let variant_tuple = NonExhaustiveVariants::Tuple(640);
//~^ ERROR tuple variant `Tuple` is private [E0603]
let variant_unit = NonExhaustiveVariants::Unit;
//~^ ERROR unit variant `Unit` is private [E0603]
match variant_struct {
NonExhaustiveVariants::Unit => "",
//~^ ERROR unit variant `Unit` is private [E0603]
NonExhaustiveVariants::Tuple(fe_tpl) => "",
//~^ ERROR tuple variant `Tuple` is private [E0603]
NonExhaustiveVariants::Struct { field } => ""
//~^ ERROR `..` required with variant marked as non-exhaustive
};
if let NonExhaustiveVariants::Tuple(fe_tpl) = variant_struct {
//~^ ERROR tuple variant `Tuple` is private [E0603]
}
if let NonExhaustiveVariants::Struct { field } = variant_struct {
//~^ ERROR `..` required with variant marked as non-exhaustive
}
}

But I agree that this behavior is kind of odd. A potential fix would involve not setting the unit variants to private but treating them as a struct-like variants. If this is the desired behavior I would be happy to implement it!

@WaffleLapkin
Copy link
Member

Note that the same happens with structs too:

/// ```
/// let lib::Foo = lib::Foo::default();
/// ```
#[non_exhaustive]
#[derive(Default)]
pub struct Foo;
error[E0603]: unit struct `Foo` is private
   --> src/lib.rs:117:20
    |
4   | let lib::Foo = lib::Foo::default();
    |          ^^^ private unit struct
    |
note: the unit struct `Foo` is defined here
   --> path
    |
121 | pub struct Foo;
    | ^^^^^^^^^^^^^^^

bors added a commit to rust-lang/cargo that referenced this issue May 1, 2023
Document that adding `#[non_exhaustive]` on existing items is breaking.

### What does this PR try to resolve?

Adding `#[non_exhaustive]` to an existing struct, enum, or variant is almost always a breaking change and requires a major version bump for semver purposes. This PR adds a section to the semver reference page that describes this and provides examples showing how `#[non_exhaustive]` can break code.

### Additional information

Adding `#[non_exhaustive]` to a unit struct currently has no effect on whether that struct can be constructed in downstream crates. This is inconsistent with the behavior of `#[non_exhaustive]` on unit enum variants, which may not be constructed outside their own crate. This might be due to a similar underlying cause as: rust-lang/rust#78586

The confusing "variant is private" error messages for non-exhaustive unit and tuple variants are a known issue tracked in: rust-lang/rust#82788

Checking for the struct portion of this semver rule is done in: obi1kenobi/cargo-semver-checks#4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints D-confusing Diagnostics: Confusing error or lint that should be reworked. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants