rust icon indicating copy to clipboard operation
rust copied to clipboard

Non-exhaustive structs may be empty

Open Nadrieril opened this issue 1 year ago • 11 comments
trafficstars

This is a follow-up to a discrepancy noticed in https://github.com/rust-lang/rust/pull/122792: today, the following struct is considered inhabited (non-empty) outside its defining crate:

#[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:

// 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

Nadrieril avatar Aug 10 '24 17:08 Nadrieril

The job x86_64-gnu-llvm-17 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
------
 > importing cache manifest from ghcr.io/rust-lang/rust-ci-cache:3aacb9c90579defe09351ac5e8ee504359f8054da6326ff19038f1b7c90e3cb2aafe33685c6d9b76ee8d2ccbd187ca80c46ab5380485abdd8c0ce7d69cd8d8fd:
------
##[endgroup]
Setting extra environment values for docker:  --env ENABLE_GCC_CODEGEN=1 --env GCC_EXEC_PREFIX=/usr/lib/gcc/
[CI_JOB_NAME=x86_64-gnu-llvm-17]
---
sccache: Starting the server...
##[group]Configure the build
configure: processing command line
configure: 
configure: build.configure-args := ['--build=x86_64-unknown-linux-gnu', '--llvm-root=/usr/lib/llvm-17', '--enable-llvm-link-shared', '--set', 'rust.thin-lto-import-instr-limit=10', '--set', 'change-id=99999999', '--enable-verbose-configure', '--enable-sccache', '--disable-manage-submodules', '--enable-locked-deps', '--enable-cargo-native-static', '--set', 'rust.codegen-units-std=1', '--set', 'dist.compression-profile=balanced', '--dist-compression-formats=xz', '--set', 'rust.lld=false', '--disable-dist-src', '--release-channel=nightly', '--enable-debug-assertions', '--enable-overflow-checks', '--enable-llvm-assertions', '--set', 'rust.verify-llvm-ir', '--set', 'rust.codegen-backends=llvm,cranelift,gcc', '--set', 'llvm.static-libstdcpp', '--enable-new-symbol-mangling']
configure: target.x86_64-unknown-linux-gnu.llvm-config := /usr/lib/llvm-17/bin/llvm-config
configure: llvm.link-shared     := True
configure: rust.thin-lto-import-instr-limit := 10
configure: change-id            := 99999999
---
diff of stderr:

5    |           ^
6    |
7 note: `IndirectUninhabitedEnum` defined here
+   --> $DIR/auxiliary/uninhabited.rs:27:1
9    |
9    |
10 LL | pub struct IndirectUninhabitedEnum(UninhabitedEnum);

24    |           ^
25    |
26 note: `IndirectUninhabitedStruct` defined here
26 note: `IndirectUninhabitedStruct` defined here
-   --> $DIR/auxiliary/uninhabited.rs:28:1
+   --> $DIR/auxiliary/uninhabited.rs:29:1
28    |
29 LL | pub struct IndirectUninhabitedStruct(UninhabitedStruct);

43    |           ^
44    |
44    |
45 note: `IndirectUninhabitedTupleStruct` defined here
+   --> $DIR/auxiliary/uninhabited.rs:31:1
47    |
47    |
48 LL | pub struct IndirectUninhabitedTupleStruct(UninhabitedTupleStruct);

62    |           ^
63    |
63    |
64 note: `IndirectUninhabitedVariants` defined here
+   --> $DIR/auxiliary/uninhabited.rs:33:1
66    |
66    |
67 LL | pub struct IndirectUninhabitedVariants(UninhabitedVariants);


The actual stderr differed from the expected stderr.
Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/rfcs/rfc-2008-non-exhaustive/uninhabited/indirect_match/indirect_match.stderr
Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/rfcs/rfc-2008-non-exhaustive/uninhabited/indirect_match/indirect_match.stderr
To update references, rerun the tests and pass the `--bless` flag
To only update this specific test, also pass `--test-args rfcs/rfc-2008-non-exhaustive/uninhabited/indirect_match.rs`

error: 1 errors occurred comparing output.
status: exit status: 1
command: env -u RUSTC_LOG_COLOR RUSTC_ICE="0" RUST_BACKTRACE="short" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/tests/ui/rfcs/rfc-2008-non-exhaustive/uninhabited/indirect_match.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=/cargo" "-Z" "ignore-directory-in-diagnostics-source-blocks=/checkout/vendor" "--sysroot" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2" "--target=x86_64-unknown-linux-gnu" "--check-cfg" "cfg(FALSE)" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zwrite-long-types-to-disk=no" "-Cstrip=debuginfo" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/rfcs/rfc-2008-non-exhaustive/uninhabited/indirect_match" "-A" "unused" "-A" "internal_features" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/rfcs/rfc-2008-non-exhaustive/uninhabited/indirect_match/auxiliary"
--- stderr -------------------------------
--- stderr -------------------------------
error[E0004]: non-exhaustive patterns: type `IndirectUninhabitedEnum` is non-empty
   |
   |
LL |     match x {} //~ ERROR non-exhaustive patterns
   |
   |
note: `IndirectUninhabitedEnum` defined here
   |
   |
LL | pub struct IndirectUninhabitedEnum(UninhabitedEnum);
   = note: the matched value is of type `IndirectUninhabitedEnum`
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern as shown
   |
LL ~     match x {
LL ~     match x {
LL +         _ => todo!(),
LL ~     } //~ ERROR non-exhaustive patterns


error[E0004]: non-exhaustive patterns: type `IndirectUninhabitedStruct` is non-empty
   |
   |
LL |     match x {} //~ ERROR non-exhaustive patterns
   |
note: `IndirectUninhabitedStruct` defined here
  --> /checkout/tests/ui/rfcs/rfc-2008-non-exhaustive/uninhabited/auxiliary/uninhabited.rs:29:1
   |
   |
LL | pub struct IndirectUninhabitedStruct(UninhabitedStruct);
   = note: the matched value is of type `IndirectUninhabitedStruct`
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern as shown
   |
LL ~     match x {
LL ~     match x {
LL +         _ => todo!(),
LL ~     } //~ ERROR non-exhaustive patterns


error[E0004]: non-exhaustive patterns: type `IndirectUninhabitedTupleStruct` is non-empty
   |
   |
LL |     match x {} //~ ERROR non-exhaustive patterns
   |
   |
note: `IndirectUninhabitedTupleStruct` defined here
   |
   |
LL | pub struct IndirectUninhabitedTupleStruct(UninhabitedTupleStruct);
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   = note: the matched value is of type `IndirectUninhabitedTupleStruct`
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern as shown
LL ~     match x {
LL +         _ => todo!(),
LL +         _ => todo!(),
LL ~     } //~ ERROR non-exhaustive patterns


error[E0004]: non-exhaustive patterns: type `IndirectUninhabitedVariants` is non-empty
   |
   |
LL |     match x {} //~ ERROR non-exhaustive patterns
   |
   |
note: `IndirectUninhabitedVariants` defined here
   |
   |
LL | pub struct IndirectUninhabitedVariants(UninhabitedVariants);
   = note: the matched value is of type `IndirectUninhabitedVariants`
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern as shown
   |
LL ~     match x {
LL ~     match x {
LL +         _ => todo!(),
LL ~     } //~ ERROR non-exhaustive patterns

error: aborting due to 4 previous errors

For more information about this error, try `rustc --explain E0004`.
---
To only update this specific test, also pass `--test-args rfcs/rfc-2008-non-exhaustive/uninhabited/issue-65157-repeated-match-arm.rs`

error: 1 errors occurred comparing output.
status: exit status: 1
command: env -u RUSTC_LOG_COLOR RUSTC_ICE="0" RUST_BACKTRACE="short" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/tests/ui/rfcs/rfc-2008-non-exhaustive/uninhabited/issue-65157-repeated-match-arm.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=/cargo" "-Z" "ignore-directory-in-diagnostics-source-blocks=/checkout/vendor" "--sysroot" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2" "--target=x86_64-unknown-linux-gnu" "--check-cfg" "cfg(FALSE)" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zwrite-long-types-to-disk=no" "-Cstrip=debuginfo" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/rfcs/rfc-2008-non-exhaustive/uninhabited/issue-65157-repeated-match-arm" "-A" "unused" "-A" "internal_features" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/rfcs/rfc-2008-non-exhaustive/uninhabited/issue-65157-repeated-match-arm/auxiliary"
--- stderr -------------------------------
error: unreachable pattern
##[error]  --> /checkout/tests/ui/rfcs/rfc-2008-non-exhaustive/uninhabited/issue-65157-repeated-match-arm.rs:14:9
   |
---
-    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-    = note: the matched value is of type `UninhabitedStruct`
- help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern as shown
-    |
- LL ~     match x {
- LL +         _ => todo!(),
- LL ~     }
- 
- 
- error[E0004]: non-exhaustive patterns: type `UninhabitedTupleStruct` is non-empty
-    |
- LL |     match x {}
-    |           ^
-    |
-    |
- note: `UninhabitedTupleStruct` defined here
-   --> $DIR/auxiliary/uninhabited.rs:14:1
-    |
- LL | pub struct UninhabitedTupleStruct(!);
-    = note: the matched value is of type `UninhabitedTupleStruct`
- help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern as shown
-    |
-    |
- LL ~     match x {
- LL +         _ => todo!(),
- LL ~     }
- 
- 
- error[E0004]: non-exhaustive patterns: `UninhabitedVariants::Tuple(_)` and `UninhabitedVariants::Struct { .. }` not covered
-    |
- LL |     match x {}
- LL |     match x {}
-    |           ^ patterns `UninhabitedVariants::Tuple(_)` and `UninhabitedVariants::Struct { .. }` not covered
- note: `UninhabitedVariants` defined here
-   --> $DIR/auxiliary/uninhabited.rs:16:1
-    |
- LL | pub enum UninhabitedVariants {
- LL | pub enum UninhabitedVariants {
-    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
- LL |     #[non_exhaustive] Tuple(!),
-    |                       ----- not covered
- LL |     #[non_exhaustive] Struct { x: ! }
-    = note: the matched value is of type `UninhabitedVariants`
-    = note: the matched value is of type `UninhabitedVariants`
- help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern, a match arm with multiple or-patterns as shown, or multiple match arms
-    |
- LL ~     match x {
- LL +         UninhabitedVariants::Tuple(_) | UninhabitedVariants::Struct { .. } => todo!(),
- LL ~     }
- 
- error: aborting due to 4 previous errors
+ error: aborting due to 1 previous error
82 
---
To only update this specific test, also pass `--test-args rfcs/rfc-2008-non-exhaustive/uninhabited/match.rs`

error: 1 errors occurred comparing output.
status: exit status: 1
command: env -u RUSTC_LOG_COLOR RUSTC_ICE="0" RUST_BACKTRACE="short" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/tests/ui/rfcs/rfc-2008-non-exhaustive/uninhabited/match.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=/cargo" "-Z" "ignore-directory-in-diagnostics-source-blocks=/checkout/vendor" "--sysroot" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2" "--target=x86_64-unknown-linux-gnu" "--check-cfg" "cfg(FALSE)" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zwrite-long-types-to-disk=no" "-Cstrip=debuginfo" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/rfcs/rfc-2008-non-exhaustive/uninhabited/match" "-A" "unused" "-A" "internal_features" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/rfcs/rfc-2008-non-exhaustive/uninhabited/match/auxiliary"
--- stderr -------------------------------
error[E0004]: non-exhaustive patterns: type `UninhabitedEnum` is non-empty
##[error]  --> /checkout/tests/ui/rfcs/rfc-2008-non-exhaustive/uninhabited/match.rs:19:11
   |
   |
LL |     match x {} //~ ERROR non-exhaustive patterns
   |
note: `UninhabitedEnum` defined here
  --> /checkout/tests/ui/rfcs/rfc-2008-non-exhaustive/uninhabited/auxiliary/uninhabited.rs:5:1
   |
   |
LL | pub enum UninhabitedEnum {
   | ^^^^^^^^^^^^^^^^^^^^^^^^
   = note: the matched value is of type `UninhabitedEnum`, which is marked as non-exhaustive
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern as shown
   |
LL ~     match x {
LL +         _ => todo!(),
LL ~     } //~ ERROR non-exhaustive patterns

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0004`.

rust-log-analyzer avatar Aug 10 '24 17:08 rust-log-analyzer

@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.

traviscross avatar Aug 14 '24 17:08 traviscross

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 pub.

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...)

pnkfelix avatar Aug 14 '24 17:08 pnkfelix

Team member @traviscross has proposed to merge this. The next step is review by the rest of the tagged team members:

  • [x] @joshtriplett
  • [ ] @nikomatsakis
  • [x] @pnkfelix
  • [x] @scottmcm
  • [ ] @tmandry
  • [x] @traviscross

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. See this document for info about what commands tagged team members can give me.

rfcbot avatar Aug 14 '24 17:08 rfcbot

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 pub.

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...)

There isn't a test for non_exaustive + private empty field anymore. You can see that I changed it because I didn't feel it was testing the interesting case (which is non_exhaustive + public empty field, i.e. what the FCP is about).

Nadrieril avatar Aug 14 '24 17:08 Nadrieril

Surely both cases are interesting, as we want to ensure that the publicness of the uninhabited field is a deciding factor?

RalfJung avatar Aug 14 '24 18:08 RalfJung

I have added such a test case.

Nadrieril avatar Aug 14 '24 18:08 Nadrieril

:bell: This is now entering its final comment period, as per the review above. :bell:

rfcbot avatar Aug 15 '24 20:08 rfcbot

It looks like you discussed this in the lang meeting, so I removed the nomination label.

Noratrieb avatar Aug 17 '24 13:08 Noratrieb

:umbrella: The latest upstream changes (presumably #129359) made this pull request unmergeable. Please resolve the merge conflicts.

bors avatar Aug 21 '24 20:08 bors

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.

rfcbot avatar Aug 25 '24 20:08 rfcbot

@rustbot ready

Nadrieril avatar Sep 02 '24 20:09 Nadrieril

@bors r+

compiler-errors avatar Sep 03 '24 07:09 compiler-errors

:pushpin: Commit 6f6a6bc710e538741aa06d40df6492471e4be8af has been approved by compiler-errors

It is now in the queue for this repository.

bors avatar Sep 03 '24 07:09 bors