-
Notifications
You must be signed in to change notification settings - Fork 13.9k
add transparent attribute for mod items #147709
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.
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
base: master
Are you sure you want to change the base?
Conversation
|
Some changes occurred in compiler/rustc_hir/src/attrs Some changes occurred in compiler/rustc_attr_parsing Some changes occurred in compiler/rustc_passes/src/check_attr.rs |
| /// * A trait or an enum (it implicitly contains associated types, methods and variant | ||
| /// constructors). | ||
| Def(DefKind, DefId, Option<Symbol>), | ||
| Def(DefKind, DefId, Option<(Symbol, bool)>), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was the most obvious place to put this but it doesn't feel great, don't love all the places where I changed shit to name_and_transparent, open to suggestions for how to better thread this information through
| transmute_opts, | ||
| transmute_trait, | ||
| transmute_unchecked, | ||
| transparent, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like I'm probably doing this part subtly wrong.
The transparent symbol already exists and I'm using it, I tried seeing if I could stay away from it and find a way to have all the compiler internal stuff talk about it with a unique symbol (transparent_modules) but have the actual ident of the attribute be transparent but it seemed like this was controlling that. If I got that wrong lmk.
This comment has been minimized.
This comment has been minimized.
|
With that nit, r=me for just the attributes stuff. I don't know as much about the name resolution part to sign off on that but thats for petrochenkov :) |
08b1160 to
b2651d1
Compare
This comment has been minimized.
This comment has been minimized.
20236fb to
741a558
Compare
741a558 to
74e1b33
Compare
| /// Allows for transmuting between arrays with sizes that contain generic consts. | ||
| (unstable, transmute_generic_consts, "1.70.0", Some(109929)), | ||
| /// Allows #[transparent] on modules to inherit lexical scopes. | ||
| (unstable, transparent_modules, "1.91.0", Some(79260)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not a real tracking issue
|
|
||
| gated!( | ||
| transparent, Normal, | ||
| template!(Word, "https://github.com/rust-lang/rust/issues/79260#issuecomment-731773625"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably also want to update this to point to actual documentation (I don't intend to add that, if its a blocker then leave this open or close it until someone comes along who wants to push this over the finish line)
74e1b33 to
afaa6b5
Compare
| pub(crate) use s; | ||
| } | ||
|
|
||
| trait IWantAMethod { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you'll also want a test in which you use #[transparent] without the feature gate to make sure it's properly rejected without the feature
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: this does work correctly, I just tested it, but you still want to have a test for it
|
@rustbot author |
|
☔ The latest upstream changes (presumably #148356) made this pull request unmergeable. Please resolve the merge conflicts. |
Addresses #79260
r? @petrochenkov