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

Don't complain on a single non-exhaustive 1-ZST #115924

Merged
merged 1 commit into from
Sep 19, 2023

Conversation

compiler-errors
Copy link
Member

r? RalfJung, though you mentioned being busy, so feel free to reassign.

This doesn't actually attempt to make the diagnostic better, so when we have two non-exhaustive 1-ZSTs in a struct, we still just point to one. 🤷

Fixes #115922

@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 Sep 18, 2023
@rust-log-analyzer

This comment has been minimized.

Comment on lines +136 to +140
error: zero-sized fields in `repr(transparent)` cannot contain external non-exhaustive types
--> $DIR/repr-transparent-non-exhaustive.rs:102:31
|
LL | pub struct T18(NonExhaustive, NonExhaustive);
| ^^^^^^^^^^^^^
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fine to land this for now to fix the regression, but could you file a diagnostics issue about this? The error says "zero-sized fields [...] cannot contain external non-exhaustive types" but that's not really accurate, some zero-sized fields can in fact contain such non-exhaustive types.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// Okay, since there's only 1 foreign non-exhaustive type.

#[repr(transparent)]
pub struct T22Flipped(NonExhaustive, InternalPrivate);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is identical to T22...?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy paste issue 🤦

Should I just give these structs better names while we're at it? Like, the whole file?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you have good naming ideas, sure, why not.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing comes to mind actually that isn't incredibly verbose. 😓

@RalfJung
Copy link
Member

r=me after fixing the test

@rust-cloud-vms rust-cloud-vms bot force-pushed the non-exhaustive-1-zst branch from 537834d to fd36553 Compare September 19, 2023 06:01
@compiler-errors
Copy link
Member Author

@bors r=RalfJung

@bors
Copy link
Contributor

bors commented Sep 19, 2023

📌 Commit fd36553 has been approved by RalfJung

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 19, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 19, 2023
…llaumeGomez

Rollup of 6 pull requests

Successful merges:

 - rust-lang#112725 (rustdoc-search: add support for type parameters)
 - rust-lang#114941 (Don't resolve generic impls that may be shadowed by dyn built-in impls)
 - rust-lang#115625 (Explain HRTB + infer limitations of old solver)
 - rust-lang#115839 (Bump libc to 0.2.148)
 - rust-lang#115924 (Don't complain on a single non-exhaustive 1-ZST)
 - rust-lang#115946 (panic when encountering an illegal cpumask in thread::available_parallelism)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f1edecf into rust-lang:master Sep 19, 2023
11 checks passed
@rustbot rustbot added this to the 1.74.0 milestone Sep 19, 2023
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Sep 19, 2023
Rollup merge of rust-lang#115924 - compiler-errors:non-exhaustive-1-zst, r=RalfJung

Don't complain on a single non-exhaustive 1-ZST

r? RalfJung, though you mentioned being busy, so feel free to reassign.

This doesn't actually attempt to make the diagnostic better, so when we have two non-exhaustive 1-ZSTs in a struct, we still just point to one. 🤷

Fixes rust-lang#115922
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Regression in lint for non-exhaustive ZST in repr(Transparent)
5 participants