-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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 structs may be empty #128934
Conversation
This comment has been minimized.
This comment has been minimized.
@rfcbot fcp merge We discussed this in the triage meeting today. This sounded good to us. Let's do it via FCP since there's a one-way door here. |
Hey @Nadrieril , I skimmed over the PR and noticed that all the test cases touched here that involve never-typed field also had that same field always marked as Is there a pre-existing test for the case of a non-exhaustive struct with a non-pub field that is empty? (A situation I infer to be a case which we wish to continue erroring on...) |
Team member @traviscross has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns. |
There isn't a test for |
The final comment period, with a disposition to merge, as per the review above, is now complete. As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed. This will be merged soon. |
ba231c5
to
6f6a6bc
Compare
@rustbot ready |
@bors r+ |
…, r=compiler-errors Non-exhaustive structs may be empty This is a follow-up to a discrepancy noticed in rust-lang#122792: today, the following struct is considered inhabited (non-empty) outside its defining crate: ```rust #[non_exhaustive] pub struct UninhabitedStruct { pub never: !, // other fields } ``` `#[non_exhaustive]` on a struct should mean that adding fields to it isn't a breaking change. There is no way that adding fields to this struct could make it non-empty since the `never` field must stay and is inconstructible. I suspect this was implemented this way due to confusion with `#[non_exhaustive]` enums, which indeed should be considered non-empty outside their defining crate. I propose that we consider such a struct uninhabited (empty), just like it would be without the `#[non_exhaustive]` annotation. Code that doesn't pass today and will pass after this: ```rust // In a different crate fn empty_match_on_empty_struct<T>(x: UninhabitedStruct) -> T { match x {} } ``` This is not a breaking change. r? `@compiler-errors`
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#127692 (Suggest `impl Trait` for References to Bare Trait in Function Header) - rust-lang#128701 (Don't Suggest Labeling `const` and `unsafe` Blocks ) - rust-lang#128934 (Non-exhaustive structs may be empty) - rust-lang#129630 (Document the broken C ABI of `wasm32-unknown-unknown`) - rust-lang#129706 (Rename dump of coroutine by-move-body to be more consistent, fix ICE in dump_mir) - rust-lang#129896 (do not attempt to prove unknowable goals) - rust-lang#129926 (Move `SanityCheck` and `MirPass`) - rust-lang#129928 (rustc_driver_impl: remove some old dead logic) - rust-lang#129930 (include 1.80.1 release notes on master) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#127692 (Suggest `impl Trait` for References to Bare Trait in Function Header) - rust-lang#128701 (Don't Suggest Labeling `const` and `unsafe` Blocks ) - rust-lang#128934 (Non-exhaustive structs may be empty) - rust-lang#129630 (Document the broken C ABI of `wasm32-unknown-unknown`) - rust-lang#129863 (update comment regarding TargetOptions.features) - rust-lang#129896 (do not attempt to prove unknowable goals) - rust-lang#129926 (Move `SanityCheck` and `MirPass`) - rust-lang#129928 (rustc_driver_impl: remove some old dead logic) - rust-lang#129930 (include 1.80.1 release notes on master) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#127692 (Suggest `impl Trait` for References to Bare Trait in Function Header) - rust-lang#128701 (Don't Suggest Labeling `const` and `unsafe` Blocks ) - rust-lang#128934 (Non-exhaustive structs may be empty) - rust-lang#129630 (Document the broken C ABI of `wasm32-unknown-unknown`) - rust-lang#129863 (update comment regarding TargetOptions.features) - rust-lang#129896 (do not attempt to prove unknowable goals) - rust-lang#129926 (Move `SanityCheck` and `MirPass`) - rust-lang#129928 (rustc_driver_impl: remove some old dead logic) - rust-lang#129930 (include 1.80.1 release notes on master) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#127692 (Suggest `impl Trait` for References to Bare Trait in Function Header) - rust-lang#128701 (Don't Suggest Labeling `const` and `unsafe` Blocks ) - rust-lang#128934 (Non-exhaustive structs may be empty) - rust-lang#129630 (Document the broken C ABI of `wasm32-unknown-unknown`) - rust-lang#129863 (update comment regarding TargetOptions.features) - rust-lang#129896 (do not attempt to prove unknowable goals) - rust-lang#129926 (Move `SanityCheck` and `MirPass`) - rust-lang#129928 (rustc_driver_impl: remove some old dead logic) - rust-lang#129930 (include 1.80.1 release notes on master) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#127692 (Suggest `impl Trait` for References to Bare Trait in Function Header) - rust-lang#128701 (Don't Suggest Labeling `const` and `unsafe` Blocks ) - rust-lang#128934 (Non-exhaustive structs may be empty) - rust-lang#129630 (Document the broken C ABI of `wasm32-unknown-unknown`) - rust-lang#129863 (update comment regarding TargetOptions.features) - rust-lang#129896 (do not attempt to prove unknowable goals) - rust-lang#129926 (Move `SanityCheck` and `MirPass`) - rust-lang#129928 (rustc_driver_impl: remove some old dead logic) - rust-lang#129930 (include 1.80.1 release notes on master) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#127692 (Suggest `impl Trait` for References to Bare Trait in Function Header) - rust-lang#128701 (Don't Suggest Labeling `const` and `unsafe` Blocks ) - rust-lang#128934 (Non-exhaustive structs may be empty) - rust-lang#129630 (Document the broken C ABI of `wasm32-unknown-unknown`) - rust-lang#129863 (update comment regarding TargetOptions.features) - rust-lang#129896 (do not attempt to prove unknowable goals) - rust-lang#129926 (Move `SanityCheck` and `MirPass`) - rust-lang#129928 (rustc_driver_impl: remove some old dead logic) - rust-lang#129930 (include 1.80.1 release notes on master) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#127692 (Suggest `impl Trait` for References to Bare Trait in Function Header) - rust-lang#128701 (Don't Suggest Labeling `const` and `unsafe` Blocks ) - rust-lang#128934 (Non-exhaustive structs may be empty) - rust-lang#129630 (Document the broken C ABI of `wasm32-unknown-unknown`) - rust-lang#129863 (update comment regarding TargetOptions.features) - rust-lang#129896 (do not attempt to prove unknowable goals) - rust-lang#129926 (Move `SanityCheck` and `MirPass`) - rust-lang#129928 (rustc_driver_impl: remove some old dead logic) - rust-lang#129930 (include 1.80.1 release notes on master) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#127692 (Suggest `impl Trait` for References to Bare Trait in Function Header) - rust-lang#128701 (Don't Suggest Labeling `const` and `unsafe` Blocks ) - rust-lang#128934 (Non-exhaustive structs may be empty) - rust-lang#129630 (Document the broken C ABI of `wasm32-unknown-unknown`) - rust-lang#129863 (update comment regarding TargetOptions.features) - rust-lang#129896 (do not attempt to prove unknowable goals) - rust-lang#129926 (Move `SanityCheck` and `MirPass`) - rust-lang#129928 (rustc_driver_impl: remove some old dead logic) - rust-lang#129930 (include 1.80.1 release notes on master) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#128934 - Nadrieril:fix-empty-non-exhaustive, r=compiler-errors Non-exhaustive structs may be empty This is a follow-up to a discrepancy noticed in rust-lang#122792: today, the following struct is considered inhabited (non-empty) outside its defining crate: ```rust #[non_exhaustive] pub struct UninhabitedStruct { pub never: !, // other fields } ``` `#[non_exhaustive]` on a struct should mean that adding fields to it isn't a breaking change. There is no way that adding fields to this struct could make it non-empty since the `never` field must stay and is inconstructible. I suspect this was implemented this way due to confusion with `#[non_exhaustive]` enums, which indeed should be considered non-empty outside their defining crate. I propose that we consider such a struct uninhabited (empty), just like it would be without the `#[non_exhaustive]` annotation. Code that doesn't pass today and will pass after this: ```rust // In a different crate fn empty_match_on_empty_struct<T>(x: UninhabitedStruct) -> T { match x {} } ``` This is not a breaking change. r? ``@compiler-errors``
This is a follow-up to a discrepancy noticed in #122792: today, the following struct is considered inhabited (non-empty) outside its defining crate:
#[non_exhaustive]
on a struct should mean that adding fields to it isn't a breaking change. There is no way that adding fields to this struct could make it non-empty since thenever
field must stay and is inconstructible. I suspect this was implemented this way due to confusion with#[non_exhaustive]
enums, which indeed should be considered non-empty outside their defining crate.I propose that we consider such a struct uninhabited (empty), just like it would be without the
#[non_exhaustive]
annotation.Code that doesn't pass today and will pass after this:
This is not a breaking change.
r? @compiler-errors