Non-exhaustive structs may be empty by Nadrieril · Pull Request #128934 · rust-lang/rust (original) (raw)

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

Nadrieril

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] 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(x: UninhabitedStruct) -> T { match x {} }

This is not a breaking change.

r? @compiler-errors

@rustbot rustbot added S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

T-compiler

Relevant to the compiler team, which will review and decide on the PR/issue.

labels

Aug 10, 2024

@rust-log-analyzer

This comment has been minimized.

@traviscross

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

@pnkfelix

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

@rfcbot

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

@Nadrieril

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

@compiler-errors

@bors

📌 Commit 6f6a6bc has been approved by compiler-errors

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

and removed S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

labels

Sep 3, 2024

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request

Sep 3, 2024

@matthiaskrgr

…, 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:

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

bors added a commit to rust-lang-ci/rust that referenced this pull request

Sep 3, 2024

@bors

…iaskrgr

Rollup of 9 pull requests

Successful merges:

r? @ghost @rustbot modify labels: rollup

bors added a commit to rust-lang-ci/rust that referenced this pull request

Sep 3, 2024

@bors

bors added a commit to rust-lang-ci/rust that referenced this pull request

Sep 3, 2024

@bors

bors added a commit to rust-lang-ci/rust that referenced this pull request

Sep 4, 2024

@bors

bors added a commit to rust-lang-ci/rust that referenced this pull request

Sep 4, 2024

@bors

bors added a commit to rust-lang-ci/rust that referenced this pull request

Sep 4, 2024

@bors

bors added a commit to rust-lang-ci/rust that referenced this pull request

Sep 4, 2024

@bors

bors added a commit to rust-lang-ci/rust that referenced this pull request

Sep 4, 2024

@bors

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request

Sep 5, 2024

@rust-timer

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:

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

@cuviper cuviper added the relnotes

Marks issues that should be documented in the release notes of the next release.

label

Nov 22, 2024

wip-sync pushed a commit to NetBSD/pkgsrc-wip that referenced this pull request

Nov 30, 2024

@he32

Pkgsrc changes compared to rust182:

TODO:

Upstream changes:

Version 1.83.0 (2024-11-28)

Language

Compiler

Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support.

Libraries

Stabilized APIs

These APIs are now stable in const contexts:

Cargo

Rustdoc

Compatibility Notes

tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request

Dec 5, 2024

@tmeijn

This MR contains the following updates:

Package Update Change
rust minor 1.82.0 -> 1.83.0

MR created with the help of el-capitano/tools/renovate-bot.

Proposed changes to behavior should be submitted there as MRs.


Release Notes

rust-lang/rust (rust)

v1.83.0

Compare Source

==========================

Language

Compiler

Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support.

Libraries

Stabilized APIs

These APIs are now stable in const contexts:

Cargo

Rustdoc

Compatibility Notes


Configuration

📅 Schedule: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.

Rebasing: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 Ignore: Close this MR and you won't be reminded about this update again.



This MR has been generated by Renovate Bot.

Veykril added a commit to rust-lang/fls that referenced this pull request

Dec 9, 2024

@Veykril

Veykril added a commit to rust-lang/fls that referenced this pull request

Dec 9, 2024

@Veykril

Veykril added a commit to rust-lang/fls that referenced this pull request

Dec 10, 2024

@Veykril

tshepang pushed a commit to ferrocene/ferrocene that referenced this pull request

Dec 18, 2024

@Veykril @tshepang

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request

Feb 24, 2025

@he32

Pkgsrc changes:

Upstream changes:

Version 1.83.0 (2024-11-28)

Language

Compiler

Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support.

Libraries

Stabilized APIs

These APIs are now stable in const contexts:

Cargo

Rustdoc

Compatibility Notes

Labels

A-exhaustiveness-checking

Relating to exhaustiveness / usefulness checking of patterns

A-patterns

Relating to patterns and pattern matching

disposition-merge

This issue / PR is in PFCP or FCP with a disposition to merge it.

F-non_exhaustive

`#![feature(non_exhaustive)]`

finished-final-comment-period

The final comment period is finished for this PR / Issue.

relnotes

Marks issues that should be documented in the release notes of the next release.

S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

T-lang

Relevant to the language team, which will review and decide on the PR/issue.