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

Adding non_exhaustive should disable missing_copy_implementations #116766

Closed
milliams opened this issue Oct 15, 2023 · 1 comment · Fixed by #116812
Closed

Adding non_exhaustive should disable missing_copy_implementations #116766

milliams opened this issue Oct 15, 2023 · 1 comment · Fixed by #116812
Assignees
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@milliams
Copy link

milliams commented Oct 15, 2023

I have the following minimal example:

#![deny(missing_copy_implementations)]

#[derive(Clone)]
pub enum MyEnum {
    A,
}

It does as I expect and gives the error:

error: type could implement `Copy`; consider adding `impl Copy`
 --> src/lib.rs:4:1
  |
4 | / pub enum MyEnum {
5 | |     A,
6 | | }
  | |_^
  |
note: the lint level is defined here
 --> src/lib.rs:1:9
  |
1 | #![deny(missing_copy_implementations)]
  |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

However, since this enum is going to have more options added to it in the future, for example, B(String), I go ahead and label it as non_exhaustive:

#![deny(missing_copy_implementations)]

#[derive(Clone)]
#[non_exhaustive]
pub enum MyEnum {
    A,
}

I would expect that this would disable the missing_copy_implementations lint in this case. This is because, otherwise adding a Copy implementation, e.g.:

#![deny(missing_copy_implementations)]

#[derive(Clone, Copy)]
#[non_exhaustive]
pub enum MyEnum {
    A,
}

would then break when, in the future, I add my B variant:

#![deny(missing_copy_implementations)]

#[derive(Clone, Copy)]
#[non_exhaustive]
pub enum MyEnum {
    A,
    B(String),
}

with:

error[E0204]: the trait `Copy` cannot be implemented for this type
 --> src/lib.rs:3:17
  |
3 | #[derive(Clone, Copy)]
  |                 ^^^^
...
7 |     B(String),
  |       ------ this field does not implement `Copy`
  |

In short, I would expect that adding non_exhaustive would disable the missing_copy_implementations lint as it's not future-proof, and the whole point of the non_exhaustive attribute is to say "that a type or variant may have more fields or variants added in the future".

@milliams milliams added the C-bug Category: This is a bug. label Oct 15, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Oct 15, 2023
@Noratrieb Noratrieb added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Oct 16, 2023
@rmehri01
Copy link
Contributor

@rustbot claim

@bors bors closed this as completed in 0653d7e Oct 18, 2023
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Oct 18, 2023
Rollup merge of rust-lang#116812 - rmehri01:missing_copy_implementations_non_exhaustive, r=petrochenkov

Disable missing_copy_implementations lint on non_exhaustive types

Fixes rust-lang#116766
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants