repr_transparent_external_private_fields: special-case some std types #129487
Conversation
|
r? @nnethercote rustbot has assigned @nnethercote. Use |
rustbot
added
S-waiting-on-review
| tcx.as_lang_item(def.did()), | ||
| Some(LangItem::UnsafeCell | LangItem::ManuallyDrop | LangItem::MaybeUninit) | ||
| ) || matches!( | ||
| tcx.get_diagnostic_name(def.did()), |
There was a problem hiding this comment.
Diagnostic items should not be used in a way that affects whether something is linted. They should only be used to change the quality of a diagnostic (i.e. its labels or its message). These should be turned into lang items.
There was a problem hiding this comment.
Also, is there a reason why we don't apply this to any repr(transparent) ADT?
There was a problem hiding this comment.
Also, is there a reason why we don't apply this to any
repr(transparent)ADT?
As mentioned in the original issue
More generally, quoting the nomicon:
This repr is only considered part of the public ABI of a type if either the single field is pub, or if its layout is documented in prose. Otherwise, the layout should not be relied upon by other crates.
So, in
ManuallyDrop's case, the single field is not pub, but the layout is documented in prose and so should be considered part of the ABI. Is it possible there are other types out there with such a layout guarantee in prose, which might also be incorrectly flagged by this lint?
There was a problem hiding this comment.
These should be turned into lang items.
Is there some sort of FCP required for this?
There was a problem hiding this comment.
No, you just need to do something like this: f4f57bf
There was a problem hiding this comment.
For Pin we also guarantee the layout in prose.
I think it would be better to introduce an internal attribute (e.g. rustc_pub_transparent) to mark those types rather than special case them all individually just for this lint.
There was a problem hiding this comment.
Yep, I think a builtin attr would be nice.
|
@rustbot label +A-lang-item
|
|
Could you please rework this by adding a new attribute which tracks if the repr is "public" via |
compiler-errors
added
S-waiting-on-author
GrigorenkoPV
force-pushed
the
repr_transparent_external_private_fields
branch
from
fc365d5 to
814185a
Compare
|
@rustbot ready |
rustbot
added
S-waiting-on-review
GrigorenkoPV
force-pushed
the
repr_transparent_external_private_fields
branch
from
b3432e2 to
fc022ad
Compare
@rustbot author |
rustbot
added
S-waiting-on-author
| rustc_attr!( | ||
| rustc_pub_transparent, Normal, template!(Word), | ||
| WarnFollowing, EncodeCrossCrate::Yes, | ||
| "rustc_pub_transparent is supposed to be used in libstd only", |
There was a problem hiding this comment.
give this a better description
(rustc_deprecated_safe_2024 also has a bad description, but that's unrelated)
I have force-pushed a new version, where those commits do not even exist. |
|
Please squash this still. We don't need a FIVE commit PR for a simple attribute addition. |
|
Like, this is a pretty basic PR; it does not need to document every single tiny step that you took in implementing something. This PR touches about as many files in the compiler as the number of commits, so I feel like it's a bit excessive. |
|
Sorry, though; I saw 5 commits before and saw 5 commits after so I didn't realize that you actually ended up just writing 4 more commits just to add an internal attribute, lol. |
|
No problem, I will squash them, as they have indeed got a bit too small but numerous |
GrigorenkoPV
force-pushed
the
repr_transparent_external_private_fields
branch
from
fc022ad to
06f2d73
Compare
|
@rustbot ready |
rustbot
added
S-waiting-on-review
|
@bors r+ |
bors
added
S-waiting-on-bors
…rnal_private_fields, r=compiler-errors repr_transparent_external_private_fields: special-case some std types Fixes rust-lang#129470 `@rustbot` label +A-lint +L-repr_transparent_external_private_fields
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#128919 (Add an internal lint that warns when accessing untracked data) - rust-lang#129134 (bootstrap: improve error recovery flags to curl) - rust-lang#129416 (library: Move unstable API of new_uninit to new features) - rust-lang#129459 (handle stage0 `cargo` and `rustc` separately) - rust-lang#129487 (repr_transparent_external_private_fields: special-case some std types) - rust-lang#129511 (Update minifier to 0.3.1) - rust-lang#129523 (Make `rustc_type_ir` build on stable) - rust-lang#129546 (Get rid of `predicates_defined_on`) r? `@ghost` `@rustbot` modify labels: rollup
…rnal_private_fields, r=compiler-errors repr_transparent_external_private_fields: special-case some std types Fixes rust-lang#129470 ``@rustbot`` label +A-lint +L-repr_transparent_external_private_fields
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#129091 (add Box::as_ptr and Box::as_mut_ptr methods) - rust-lang#129134 (bootstrap: improve error recovery flags to curl) - rust-lang#129416 (library: Move unstable API of new_uninit to new features) - rust-lang#129459 (handle stage0 `cargo` and `rustc` separately) - rust-lang#129487 (repr_transparent_external_private_fields: special-case some std types) - rust-lang#129511 (Update minifier to 0.3.1) - rust-lang#129523 (Make `rustc_type_ir` build on stable) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#129091 (add Box::as_ptr and Box::as_mut_ptr methods) - rust-lang#129134 (bootstrap: improve error recovery flags to curl) - rust-lang#129416 (library: Move unstable API of new_uninit to new features) - rust-lang#129459 (handle stage0 `cargo` and `rustc` separately) - rust-lang#129487 (repr_transparent_external_private_fields: special-case some std types) - rust-lang#129511 (Update minifier to 0.3.1) - rust-lang#129523 (Make `rustc_type_ir` build on stable) r? `@ghost` `@rustbot` modify labels: rollup
…, r=tgross35 gitignore: ignore ICE reports regardless of directory Quite often when working on compiler I end up running into ICEs during the standard library compilation. These ICEs generate reports in `/library/` and not at the root of the repo, so they aren't `gitignore`d. I finally ended up committing one today, by accident: rust-lang#129487 (comment)
Rollup merge of rust-lang#129487 - GrigorenkoPV:repr_transparent_external_private_fields, r=compiler-errors repr_transparent_external_private_fields: special-case some std types Fixes rust-lang#129470 ```@rustbot``` label +A-lint +L-repr_transparent_external_private_fields
…, r=tgross35 gitignore: ignore ICE reports regardless of directory Quite often when working on compiler I end up running into ICEs during the standard library compilation. These ICEs generate reports in `/library/` and not at the root of the repo, so they aren't `gitignore`d. I finally ended up committing one today, by accident: rust-lang#129487 (comment)
|
Thank you a lot for the review! |
Rollup merge of rust-lang#129518 - GrigorenkoPV:gitignore-library-ice, r=tgross35 gitignore: ignore ICE reports regardless of directory Quite often when working on compiler I end up running into ICEs during the standard library compilation. These ICEs generate reports in `/library/` and not at the root of the repo, so they aren't `gitignore`d. I finally ended up committing one today, by accident: rust-lang#129487 (comment)
Fixes #129470
@rustbot label +A-lint +L-repr_transparent_external_private_fields