-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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 TyAlias into rustc_type_ir TyKind enum #97974
Add TyAlias into rustc_type_ir TyKind enum #97974
Conversation
Some changes occured to the CTFE / Miri engine cc @rust-lang/miri Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
This comment has been minimized.
This comment has been minimized.
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 just left my first impressions, sorry if anything I said made absolutely no sense. I just wanted to point out areas that made me go "hmm" so a more thorough code-review will not miss them and/or so we can circle back on questions.
@@ -304,6 +304,10 @@ impl<'a, 'tcx> ConstraintContext<'a, 'tcx> { | |||
self.add_constraints_from_invariant_substs(current, substs, variance); | |||
} | |||
|
|||
ty::TyAlias(_, substs) => { | |||
self.add_constraints_from_invariant_substs(current, substs, variance); |
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 wonder if we can skip this (and/or we need to recurse onto the aliased type), since ty aliases shouldn't participate in variance, though I don't really know much about variance calculation.
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'll let it as is for the time being then until we have a better understanding.
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.
It will cause subtle bugs if actually reachable. Change to bug!
instead and only add logic if we actually find code that hits it.
☔ The latest upstream changes (presumably #97905) made this pull request unmergeable. Please resolve the merge conflicts. |
9ac6fa0
to
c82bf7e
Compare
This comment has been minimized.
This comment has been minimized.
c82bf7e
to
4547b52
Compare
Only two UI tests are failing now and it's about mangling. I'll wait for the rest to be approved to update them. |
This comment has been minimized.
This comment has been minimized.
4547b52
to
228f80f
Compare
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.
some more type_of
calls that need to be squashed
This comment has been minimized.
This comment has been minimized.
1e737fd
to
c9f5d4e
Compare
Replaced the |
This comment has been minimized.
This comment has been minimized.
@bors try @rust-timer queue won't get to this before next week |
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
⌛ Trying commit c9f5d4e227127e9c11e6b0a19b534abcbad6be7a with merge a2a309290e7901fd85bbba6cc69b2f368162312d... |
a0f2bbf
to
c383c9d
Compare
The job Click to see the possible cause of the failure (guessed by this bot)
|
So, I know that the main purpose of this change is to aid rustdoc, but after noticing this PR I also mentioned it on rust-lang/rfcs#3296 as something that could also help that RFC as well. Just in case that sways you one way or another in terms of what implementation is best to solve this problem -- I noticed #103985 as well as an alternative, which I would be less in favour of for this reason. |
#103985 is less an alternative than an experiment (that couldn't be finished unfortunately). As for this feature, it would also help for some compiler errors (although it'd be a very small improvement concerning very few errors). But in any case, it's currently blocked on a few things, so we're not in a hurry. :) As for your RFC, good luck! |
It completely was. Completely forgot to close it. |
Properly deal with weak alias types as self types of impls Fixes rust-lang#114216. Fixes rust-lang#116100. Not super happy about the two ad hoc “normalization” implementations for weak alias types: 1. In `inherent_impls`: The “peeling”, normalization to [“WHNF”][whnf]: Semantically that's exactly what we want (neither proper normalization nor shallow normalization would be correct here). Basically a weak alias type is “nominal” (well...^^) if the WHNF is nominal. [rust-lang#97974](rust-lang#97974) followed the same approach. 2. In `constrained_generic_params`: Generic parameters are constrained by a weak alias type if the corresp. “normalized” type constrains them (where we only normalize *weak* alias types not arbitrary ones). Weak alias types are injective if the corresp. “normalized” type is injective. Both have ad hoc overflow detection mechanisms. **Coherence** is handled in rust-lang#117164. r? `@oli-obk` or types [whnf]: https://en.wikipedia.org/wiki/Lambda_calculus_definition#Weak_head_normal_form
Properly deal with weak alias types as self types of impls Fixes rust-lang#114216. Fixes rust-lang#116100. Not super happy about the two ad hoc “normalization” implementations for weak alias types: 1. In `inherent_impls`: The “peeling”, normalization to [“WHNF”][whnf]: Semantically that's exactly what we want (neither proper normalization nor shallow normalization would be correct here). Basically a weak alias type is “nominal” (well...^^) if the WHNF is nominal. [rust-lang#97974](rust-lang#97974) followed the same approach. 2. In `constrained_generic_params`: Generic parameters are constrained by a weak alias type if the corresp. “normalized” type constrains them (where we only normalize *weak* alias types not arbitrary ones). Weak alias types are injective if the corresp. “normalized” type is injective. Both have ad hoc overflow detection mechanisms. **Coherence** is handled in rust-lang#117164. r? `@oli-obk` or types [whnf]: https://en.wikipedia.org/wiki/Lambda_calculus_definition#Weak_head_normal_form
Properly deal with weak alias types as self types of impls Fixes #114216. Fixes #116100. Not super happy about the two ad hoc “normalization” implementations for weak alias types: 1. In `inherent_impls`: The “peeling”, normalization to [“WHNF”][whnf]: Semantically that's exactly what we want (neither proper normalization nor shallow normalization would be correct here). Basically a weak alias type is “nominal” (well...^^) if the WHNF is nominal. [#97974](rust-lang/rust#97974) followed the same approach. 2. In `constrained_generic_params`: Generic parameters are constrained by a weak alias type if the corresp. “normalized” type constrains them (where we only normalize *weak* alias types not arbitrary ones). Weak alias types are injective if the corresp. “normalized” type is injective. Both have ad hoc overflow detection mechanisms. **Coherence** is handled in #117164. r? `@oli-obk` or types [whnf]: https://en.wikipedia.org/wiki/Lambda_calculus_definition#Weak_head_normal_form
Properly deal with weak alias types as self types of impls Fixes #114216. Fixes #116100. Not super happy about the two ad hoc “normalization” implementations for weak alias types: 1. In `inherent_impls`: The “peeling”, normalization to [“WHNF”][whnf]: Semantically that's exactly what we want (neither proper normalization nor shallow normalization would be correct here). Basically a weak alias type is “nominal” (well...^^) if the WHNF is nominal. [#97974](rust-lang/rust#97974) followed the same approach. 2. In `constrained_generic_params`: Generic parameters are constrained by a weak alias type if the corresp. “normalized” type constrains them (where we only normalize *weak* alias types not arbitrary ones). Weak alias types are injective if the corresp. “normalized” type is injective. Both have ad hoc overflow detection mechanisms. **Coherence** is handled in #117164. r? `@oli-obk` or types [whnf]: https://en.wikipedia.org/wiki/Lambda_calculus_definition#Weak_head_normal_form
Properly deal with weak alias types as self types of impls Fixes #114216. Fixes #116100. Not super happy about the two ad hoc “normalization” implementations for weak alias types: 1. In `inherent_impls`: The “peeling”, normalization to [“WHNF”][whnf]: Semantically that's exactly what we want (neither proper normalization nor shallow normalization would be correct here). Basically a weak alias type is “nominal” (well...^^) if the WHNF is nominal. [#97974](rust-lang/rust#97974) followed the same approach. 2. In `constrained_generic_params`: Generic parameters are constrained by a weak alias type if the corresp. “normalized” type constrains them (where we only normalize *weak* alias types not arbitrary ones). Weak alias types are injective if the corresp. “normalized” type is injective. Both have ad hoc overflow detection mechanisms. **Coherence** is handled in #117164. r? `@oli-obk` or types [whnf]: https://en.wikipedia.org/wiki/Lambda_calculus_definition#Weak_head_normal_form
Fixes rust-lang/compiler-team#504.
This is still WIP but I think we're getting close to the end of the implementation. Thanks a lot to both @compiler-errors and @oli-obk for their great guidance and help!
cc @compiler-errors
r? @oli-obk