Allow projections to be used as const generic#104443
Allow projections to be used as const generic#104443GuillaumeGomez wants to merge 4 commits intorust-lang:masterfrom
Conversation
This comment has been minimized.
This comment has been minimized.
be960b4 to
1e1f73e
Compare
|
I moved the normalization outside of the query and added an |
1e1f73e to
cbbe7ea
Compare
There was a problem hiding this comment.
Both reveal_all and normalize_erasing_regions are almost always the wrong thing. Unless you know what it means to use it, you can be sure it's wrong to use it.
The fact that an unnormalized type ends up in a node.ty is the problem, find out where it comes from and normalize there.
There was a problem hiding this comment.
Ah I marked this as resolved mistakenly. Should still check where the node gets its type and normalize there
This comment has been minimized.
This comment has been minimized.
cbbe7ea to
05ee2ab
Compare
|
I only cleaned up. I still need to find the root cause of the missing normalization. |
There was a problem hiding this comment.
tbh I am not sure whether i want to insta stabilize projections in const params or keep them as part of the adt_const_params feature 🤔 needs a lang FCP if insta stable
There was a problem hiding this comment.
As you prefer! If a FCP is needed, I'll let someone from your team handle this part.
|
☔ The latest upstream changes (presumably #104170) made this pull request unmergeable. Please resolve the merge conflicts. |
There was a problem hiding this comment.
This normalization is causing a lot of cycle error. If I nest it into a if TypeVisitable::has_projections(&ty) {, it reduces greatly the number of such errors but it's not a good solution imo.
Do you have any idea what's going on here? Why would normalization would create cycle errors? (in code like src/test/ui/variance/variance-associated-consts.rs)
There was a problem hiding this comment.
ah yeah ofc that causes issues 😅
having a const parameter in the where clauses now relies on the param_env query for normalization to get the ty::Const but we need the ty::Const as part of the output of the param_env query.
There was a problem hiding this comment.
Any idea how to fix it?
There was a problem hiding this comment.
not really 😅 even changing the type of type level constants to not inherit their parents generics won't work because normalization depends on impl headers which can also contain ty::Const. Normalizing when creating ty::Const is too early 🤔
If we were to delay normalization of the type of consts to a later point this may mostly work. tbh that makes me think that we shouldn't land this change and should keep forbidding projections in the type of type system constants. Sorry for not noticing this issue earlier 😅 especially as we've had to deal with pretty much the same issue for generic_const_exprs as well.
05ee2ab to
00d9f41
Compare
|
Closing as we can't really do it currently. I added the test in #104717. |
…d-as-const-generic, r=oli-obk Add failing test for projections used as const generic Based on the experiment done in rust-lang#104443, we realized it's currently not possible to support projections in const generics. More information about it in rust-lang#104443 (comment). This PR adds the UI test in any case so we can gather data in order to work towards adding `TyAlias` into the ABI in the future. r? `@oli-obk`
…d-as-const-generic, r=oli-obk Add failing test for projections used as const generic Based on the experiment done in rust-lang#104443, we realized it's currently not possible to support projections in const generics. More information about it in rust-lang#104443 (comment). This PR adds the UI test in any case so we can gather data in order to work towards adding `TyAlias` into the ABI in the future. r? ``@oli-obk``
…d-as-const-generic, r=oli-obk Add failing test for projections used as const generic Based on the experiment done in rust-lang#104443, we realized it's currently not possible to support projections in const generics. More information about it in rust-lang#104443 (comment). This PR adds the UI test in any case so we can gather data in order to work towards adding `TyAlias` into the ABI in the future. r? ```@oli-obk```
Thanks to #103985, we were able to find out about this limitation.
Hopefully, more should follow.
r? @oli-obk