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.

Sign up for GitHub

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

Jump to bottom

check non_exhaustive attr and private fields for transparent types #99020

Merged
merged 3 commits into from
Jul 13, 2022
Merged

check non_exhaustive attr and private fields for transparent types #99020

merged 3 commits into from
Jul 13, 2022

Conversation

fee1-dead
Copy link
Member

Fixes #78586.

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jul 7, 2022
@rust-highfive
Copy link
Collaborator

r? @oli-obk

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 7, 2022
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@fee1-dead fee1-dead force-pushed the repr_transparent_non_exhaustive branch from b182668 to 6919a57 Compare July 7, 2022 17:30
@oli-obk
Copy link
Contributor

oli-obk commented Jul 8, 2022

I think we also need to address enums: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=7cfa15a0f159c2aff5f070ac52f4d485

Also, I'm not actually sure what the expected behaviour of exporting Baz is:

#[non_exhaustive]
pub struct Foo {}

#[repr(transparent)]
pub struct Baz(pub Foo);

can Baz now be used as a ZST in another transparent type in another crate? That seems just as problematic as using the non-exhaustive type directly.

@fee1-dead
Copy link
Member Author

can Baz now be used as a ZST in another transparent type in another crate?

No, and this checks that as it recurses into the fields and error if any transitive field is non_exhaustive or has private fields.

@oli-obk
Copy link
Contributor

oli-obk commented Jul 8, 2022

No, and this checks that as it recurses into the fields and error if any transitive field is non_exhaustive or has private fields.

cool, please add tests for that!

@fee1-dead
Copy link
Member Author

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 8, 2022
@fee1-dead fee1-dead force-pushed the repr_transparent_non_exhaustive branch from 6919a57 to 5fb6784 Compare July 10, 2022 13:32
@fee1-dead
Copy link
Member Author

There are several tests that already test this (See ExternalIndirection and InternalIndirection) although they are not repr(transparent)

@fee1-dead fee1-dead added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 10, 2022
@oli-obk
Copy link
Contributor

oli-obk commented Jul 11, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Jul 11, 2022

📌 Commit 5fb6784 has been approved by oli-obk

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 Jul 11, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Jul 11, 2022
@Dylan-DPC
Rollup merge of rust-lang#99020 - fee1-dead-contrib:repr_transparent_…
a93152d 
…non_exhaustive, r=oli-obk

check non_exhaustive attr and private fields for transparent types

Fixes rust-lang#78586.
@Dylan-DPC Dylan-DPC mentioned this pull request Jul 11, 2022
Closed
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 11, 2022
@matthiaskrgr
Rollup merge of rust-lang#99020 - fee1-dead-contrib:repr_transparent_…
a19693d 
…non_exhaustive, r=oli-obk

check non_exhaustive attr and private fields for transparent types

Fixes rust-lang#78586.
@matthiaskrgr matthiaskrgr mentioned this pull request Jul 11, 2022
Closed
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 11, 2022
@matthiaskrgr
Rollup merge of rust-lang#99020 - fee1-dead-contrib:repr_transparent_…
29b7427 
…non_exhaustive, r=oli-obk

check non_exhaustive attr and private fields for transparent types

Fixes rust-lang#78586.
@matthiaskrgr matthiaskrgr mentioned this pull request Jul 11, 2022
Closed
@matthiaskrgr
Copy link
Member

@bors r- failed in a rollup
https://github.com/rust-lang-ci/rust/runs/7289757357?check_suite_focus=true#step:25:36455

@bors bors removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jul 11, 2022
@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jul 11, 2022
@fee1-dead fee1-dead force-pushed the repr_transparent_non_exhaustive branch 2 times, most recently from c96a22c to d812850 Compare July 12, 2022 11:03
@oli-obk
Copy link
Contributor

oli-obk commented Jul 12, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Jul 12, 2022

📌 Commit d812850 has been approved by oli-obk

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 12, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Jul 12, 2022
@Dylan-DPC
Rollup merge of rust-lang#99020 - fee1-dead-contrib:repr_transparent_…
70408a0 
…non_exhaustive, r=oli-obk

check non_exhaustive attr and private fields for transparent types

Fixes rust-lang#78586.
@Dylan-DPC Dylan-DPC mentioned this pull request Jul 12, 2022
Closed
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Jul 12, 2022
@Dylan-DPC
Rollup merge of rust-lang#99020 - fee1-dead-contrib:repr_transparent_…
454e597 
…non_exhaustive, r=oli-obk

check non_exhaustive attr and private fields for transparent types

Fixes rust-lang#78586.
@Dylan-DPC Dylan-DPC mentioned this pull request Jul 12, 2022
Closed
@fee1-dead fee1-dead force-pushed the repr_transparent_non_exhaustive branch from d812850 to 1d26006 Compare July 13, 2022 04:49
@fee1-dead
Copy link
Member Author

@bors r=oli-obk

minor doc change, no need to review again

@bors
Copy link
Contributor

bors commented Jul 13, 2022

📌 Commit 1d26006 has been approved by oli-obk

It is now in the queue for this repository.

@GuillaumeGomez GuillaumeGomez mentioned this pull request Jul 13, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 13, 2022
@bors
Auto merge of rust-lang#99203 - GuillaumeGomez:rollup-b2re0dv, r=Guil…
ca4e394 
…laumeGomez

Rollup of 10 pull requests

Successful merges:

 - rust-lang#98789 (rustdoc-json-types: Clean up derives.)
 - rust-lang#98848 (Build the Clippy book as part of x.py doc)
 - rust-lang#99020 (check non_exhaustive attr and private fields for transparent types)
 - rust-lang#99132 (Add some autolabels for A-bootstrap and T-infra)
 - rust-lang#99148 (Clarify that [iu]size bounds were only defined for the target arch)
 - rust-lang#99152 (Use CSS variables to handle theming (part 2))
 - rust-lang#99168 (Add regression test for rust-lang#74713)
 - rust-lang#99176 (:arrow_up: rust-analyzer)
 - rust-lang#99183 (Mention rust-analyzer maintainers when `proc_macro` bridge is changed)
 - rust-lang#99185 (llvm-wrapper: adapt for LLVM API change)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 8a48557 into rust-lang:master Jul 13, 2022
@rustbot rustbot added this to the 1.64.0 milestone Jul 13, 2022
@fee1-dead fee1-dead deleted the repr_transparent_non_exhaustive branch November 21, 2022 14:27
This was referenced Sep 5, 2023
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scottmcm scottmcm scottmcm left review comments

Assignees

@oli-obk oli-obk

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
Milestone
1.64.0
Development

Successfully merging this pull request may close these issues.

repr(transparent) should not consider non_exhaustive types as 1-ZSTs outside their crate
8 participants
@fee1-dead @rust-highfive @rust-log-analyzer @oli-obk @bors @matthiaskrgr @scottmcm @rustbot