repr_transparent_external_private_fields: special-case some std types by GrigorenkoPV · Pull Request #129487 · rust-lang/rust

GrigorenkoPV · 2024-08-23T21:59:42Z

@rustbot label +A-lint +L-repr_transparent_external_private_fields

rustbot · 2024-08-23T21:59:51Z

rustbot has assigned @nnethercote.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

compiler-errors · 2024-08-23T22:24:35Z

+                        tcx.as_lang_item(def.did()),
+                        Some(LangItem::UnsafeCell | LangItem::ManuallyDrop | LangItem::MaybeUninit)
+                    ) || matches!(
+                        tcx.get_diagnostic_name(def.did()),


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.

Also, is there a reason why we don't apply this to any repr(transparent) ADT?

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?

These should be turned into lang items.

Is there some sort of FCP required for this?

No, you just need to do something like this: f4f57bf

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.

Yep, I think a builtin attr would be nice.

GrigorenkoPV · 2024-08-23T22:55:22Z

@rustbot label +A-lang-item

Cell & SyncUnsafeCell are now lang items. As per #129487 (comment)

compiler-errors · 2024-08-24T15:50:07Z

Could you please rework this by adding a new attribute which tracks if the repr is "public" via rustc_pub_transparent? Then please squash the history since we don't need to track adding a diagnostic item and a lang item if it didn't make it into the final revision.

GrigorenkoPV · 2024-08-24T18:52:26Z

@rustbot ready

compiler-errors · 2024-08-24T19:22:18Z

Then please squash the history since we don't need to track adding a diagnostic item and a lang item if it didn't make it into the final revision.

@rustbot author

compiler-errors · 2024-08-24T19:23:01Z

+    rustc_attr!(
+        rustc_pub_transparent, Normal, template!(Word),
+        WarnFollowing, EncodeCrossCrate::Yes,
+        "rustc_pub_transparent is supposed to be used in libstd only",


give this a better description

(rustc_deprecated_safe_2024 also has a bad description, but that's unrelated)

GrigorenkoPV · 2024-08-24T19:27:19Z

Then please squash the history since we don't need to track adding a diagnostic item and a lang item if it didn't make it into the final revision.

I have force-pushed a new version, where those commits do not even exist.

compiler-errors · 2024-08-24T19:28:37Z

Please squash this still. We don't need a FIVE commit PR for a simple attribute addition.

compiler-errors · 2024-08-24T19:29:45Z

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.

compiler-errors · 2024-08-24T19:31:10Z

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.

compiler-errors

Thanks!

compiler-errors · 2024-08-24T20:14:10Z

@bors r+

bors · 2024-08-24T20:14:12Z

📌 Commit 06f2d73 has been approved by compiler-errors

It is now in the queue for this repository.

…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

…, 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

ghost · 2024-08-26T04:38:14Z

Thank you a lot for the review!

rustbot assigned nnethercote Aug 23, 2024

compiler-errors reviewed Aug 23, 2024

View reviewed changes

rustbot added the A-lang-item Area: Language items label Aug 23, 2024

nnethercote assigned compiler-errors and unassigned nnethercote Aug 24, 2024

compiler-errors removed the A-lang-item Area: Language items label Aug 24, 2024

compiler-errors 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 Aug 24, 2024

GrigorenkoPV force-pushed the repr_transparent_external_private_fields branch from fc365d5 to 814185a Compare August 24, 2024 17:40

rustbot 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 Aug 24, 2024

GrigorenkoPV force-pushed the repr_transparent_external_private_fields branch from b3432e2 to fc022ad Compare August 24, 2024 18:53

GrigorenkoPV mentioned this pull request Aug 24, 2024

gitignore: ignore ICE reports regardless of directory #129518

Merged

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 Aug 24, 2024

compiler-errors suggested changes Aug 24, 2024

View reviewed changes

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 24, 2024

compiler-errors approved these changes Aug 24, 2024

View reviewed changes

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 Aug 24, 2024

matthiaskrgr mentioned this pull request Aug 25, 2024

Rollup of 8 pull requests #129554

Closed

This was referenced Aug 25, 2024

Rollup of 8 pull requests #129562

Closed

Rollup of 7 pull requests #129563

Merged

bors merged commit 9c59e97 into rust-lang:master Aug 25, 2024

rustbot added this to the 1.82.0 milestone Aug 25, 2024

GrigorenkoPV deleted the repr_transparent_external_private_fields branch August 26, 2024 04:37

GrigorenkoPV mentioned this pull request Aug 29, 2024

Lint against getting pointers from immediately dropped temporaries #128985

Merged

1 task

GrigorenkoPV mentioned this pull request Jun 16, 2025

Port #[rustc_pub_transparent] to the new attribute system #142600

Merged

Uh oh!

Conversation

GrigorenkoPV commented Aug 23, 2024

Uh oh!

rustbot commented Aug 23, 2024

Uh oh!

compiler-errors Aug 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

compiler-errors Aug 23, 2024

Choose a reason for hiding this comment

Uh oh!

GrigorenkoPV Aug 23, 2024

Choose a reason for hiding this comment

Uh oh!

GrigorenkoPV Aug 23, 2024

Choose a reason for hiding this comment

Uh oh!

compiler-errors Aug 23, 2024

Choose a reason for hiding this comment

Uh oh!

lukas-code Aug 24, 2024

Choose a reason for hiding this comment

Uh oh!

compiler-errors Aug 24, 2024

Choose a reason for hiding this comment

Uh oh!

GrigorenkoPV commented Aug 23, 2024

Uh oh!

compiler-errors commented Aug 24, 2024

Uh oh!

GrigorenkoPV commented Aug 24, 2024

Uh oh!

compiler-errors commented Aug 24, 2024

Uh oh!

compiler-errors Aug 24, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

GrigorenkoPV commented Aug 24, 2024

Uh oh!

compiler-errors commented Aug 24, 2024

Uh oh!

compiler-errors commented Aug 24, 2024

Uh oh!

compiler-errors commented Aug 24, 2024

Uh oh!

compiler-errors left a comment

Choose a reason for hiding this comment

Uh oh!

compiler-errors commented Aug 24, 2024

Uh oh!

bors commented Aug 24, 2024

Uh oh!

ghost commented Aug 26, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

compiler-errors Aug 23, 2024 •

edited

Loading