Skip to content

Regression in lint for non-exhaustive ZST in repr(Transparent) #115922

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
compiler-errors opened this issue Sep 18, 2023 · 2 comments · Fixed by #115924
Closed

Regression in lint for non-exhaustive ZST in repr(Transparent) #115922

compiler-errors opened this issue Sep 18, 2023 · 2 comments · Fixed by #115924
Labels
C-bug Category: This is a bug. P-high High priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@compiler-errors
Copy link
Member

Lifted from #78586


I believe this future compatibility warning was badly affected by #115334. Minimal repro:

// src/lib.rs

#[non_exhaustive]
pub struct ZeroSizedNonExhaustive {}
// src/main.rs

#![deny(warnings)]

#[repr(transparent)]
pub struct Transparent(repro::ZeroSizedNonExhaustive);

fn main() {}

As far as I understand what this issue is about, this code should be 100% fine. This issue is about preventing a previously-accepted repr(transparent) struct from ending up with contents which a transparent struct is not allowed to have, after a permissible size increase of a non-exhaustive formerly-ZST from a different crate.

That's not possible in the code above. Transparent continues to be a valid repr(transparent) struct if ZeroSizedNonExhaustive increases in size.

Regression in nightly-2023-09-18:

error: zero-sized fields in `repr(transparent)` cannot contain external non-exhaustive types
 --> src/main.rs:6:24
  |
6 | pub struct Transparent(repro::ZeroSizedNonExhaustive);
  |                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
  = note: for more information, see issue #78586 <https://github.com/rust-lang/rust/issues/78586>
  = note: this struct contains `ZeroSizedNonExhaustive`, which is marked with `#[non_exhaustive]`, and makes it not a breaking change to become non-zero-sized in the future.

@RalfJung @compiler-errors

Originally posted by @dtolnay in #78586 (comment)

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 18, 2023
@compiler-errors compiler-errors added the regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. label Sep 18, 2023
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Sep 18, 2023
@Noratrieb Noratrieb removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 18, 2023
@RalfJung
Copy link
Member

RalfJung commented Sep 18, 2023

Ah... good catch. When a type is not "stably 1-ZST" we should just consider it the one non-1-ZST field. Though given that this is a lint right now the logic is a bit trickier... we need to remember if there was a non-1-ZST field, and then

  • if there was, all 1-ZST fields need to be stably 1-ZST
  • if there wasn't, all but one 1-ZST field need to be stably 1-ZST. Having a nice diagnostic for this could be tricky because ideally we'd point it at two 1-ZST fields that are not stable, and explain "both of these might become non-1-ZST in the future and then this wouldn't build any more".

Sadly I won't have the time to work on fixing this quickly. Let me know if you want me to revert the PR.

@fmease fmease added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. labels Sep 19, 2023
@apiraino
Copy link
Contributor

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-high

@rustbot rustbot added P-high High priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Sep 19, 2023
@bors bors closed this as completed in f1edecf Sep 19, 2023
rust-timer added a commit to rust-lang-ci/rust that referenced this issue 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
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
C-bug Category: This is a bug. P-high High priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. 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.

6 participants