Skip to content

check non_exhaustive attr and private fields for transparent types#99020

Merged
bors merged 3 commits intorust-lang:masterfrom
fee1-dead-contrib:repr_transparent_non_exhaustive
Jul 13, 2022
Merged

check non_exhaustive attr and private fields for transparent types#99020
bors merged 3 commits intorust-lang:masterfrom
fee1-dead-contrib:repr_transparent_non_exhaustive

Conversation

@fee1-dead
Copy link
Member

@fee1-dead fee1-dead commented Jul 7, 2022

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
Contributor

rust-highfive commented Jul 7, 2022

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

fee1-dead commented Jul 8, 2022

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

fee1-dead commented Jul 8, 2022

@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

fee1-dead commented Jul 10, 2022

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
Collaborator

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

matthiaskrgr commented Jul 11, 2022

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

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
scottmcm
scottmcm reviewed Jul 12, 2022
View reviewed changes
@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

fee1-dead commented Jul 13, 2022

@bors r=oli-obk

minor doc change, no need to review again

@bors
Copy link
Collaborator

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) accepts too many types as ZST (repr_transparent_non_zst_fields, formerly repr_transparent_external_private_fields)

8 participants

@fee1-dead @rust-highfive @rust-log-analyzer @oli-obk @bors @matthiaskrgr @scottmcm @rustbot