-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Add #[defines]
attribute and require it for all type-alias-impl-trait sites that register a hidden type
#128440
base: master
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
☔ The latest upstream changes (presumably #128481) made this pull request unmergeable. Please resolve the merge conflicts. |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
…ler-errors Avoid `opaque type not constrained` errors in the presence of other errors pulled out of rust-lang#128440 These errors carry no new information if the opaque type was actually used in a constraining (but erroneous) way somewhere.
Rollup merge of rust-lang#133850 - oli-obk:push-xryukktpyooq, r=compiler-errors Avoid `opaque type not constrained` errors in the presence of other errors pulled out of rust-lang#128440 These errors carry no new information if the opaque type was actually used in a constraining (but erroneous) way somewhere.
This comment has been minimized.
This comment has been minimized.
This comment was marked as resolved.
This comment was marked as resolved.
This comment has been minimized.
This comment has been minimized.
0b8b995
to
573d6ce
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
if !rustc_feature::encode_cross_crate(attr.name_or_empty()) { | ||
if let hir::Attribute::Parsed(p) = attr { | ||
should_encode = p.encode_cross_crate(); | ||
} else if !rustc_feature::encode_cross_crate(attr.name_or_empty()) { |
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.
@jdonszelmann what do you think about this? There's probably more opportunity for simplifying things, since name_or_empty
is always empty for parsed attrs, but that should be its own PR
Some changes occurred in compiler/rustc_attr_data_structures These commits modify the If this was unintentional then you should revert the changes before this PR is merged. Some changes occurred in src/tools/rustfmt cc @rust-lang/rustfmt Some changes occurred in src/tools/clippy cc @rust-lang/clippy |
r? @lcnr maybe? |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
tests/crashes/131298.rs
Outdated
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 test now lives in tests/ui/type-alias-impl-trait/path_resolution_taint.rs
The job Click to see the possible cause of the failure (guessed by this bot)
|
Instead of relying on the signature of items to decide whether they are constraining an opaque type, the opaque types that the item constrains must be explicitly listed.
A previous version of this PR used an actual attribute, but had to keep the resolved
DefId
s in a side table.Now we just lower to fields in the AST that have no surface syntax, instead a builtin attribute macro fills in those fields where applicable.
Note that for convenience referencing opaque types in associated types from associated methods on the same impl will not require an attribute. If that causes problems
#[defines()]
can be used to overwrite the default of searching for opaques in the signature.One wart of this design is that closures and static items do not have generics. So since I stored the opaques in the generics of functions, consts and methods, I needed to add a custom field to closures and statics to track this information. It's not really worse than the work that had to be done for the attribute/side-table based design, just something to be aware of.