Preserve order of generic args#11140
Conversation
| } | ||
|
|
||
| impl ConstParam { | ||
| pub fn merge(self) -> TypeOrConstParam { |
There was a problem hiding this comment.
Feels like this should be just a plain From impl?
There was a problem hiding this comment.
Though I guess the benefit here is in not having a generic output type involved, the name merge feels wrong though since we don't merge anything
There was a problem hiding this comment.
merge is reverse of split on the TypeOrConstParam. Suggestion for better name? generalize?
There was a problem hiding this comment.
I don't have better names either, so keeping it as merge or generalize is fine
There was a problem hiding this comment.
I would also expect this to just be a From impl, is that not possible?
hkalbasi
force-pushed
the
const_generic
branch
from
db905bb to
2ba3cad
Compare
Veykril
left a comment
Veykril
left a comment
There was a problem hiding this comment.
This looks good in my eyes now, but since there is a lot of type stuff involved @flodiebold should take a look at this as well once they have time since I don't feel comfortable about that part of the codebase
|
@hkalbasi can you please rebase? |
hkalbasi
force-pushed
the
const_generic
branch
from
2ba3cad to
c51caa5
Compare
hkalbasi
force-pushed
the
const_generic
branch
2 times, most recently
from
d3c35f0 to
f4918a4
Compare
|
@flodiebold Can you please review this PR? It touches too many files so it conflicts with master frequently. |
flodiebold
left a comment
flodiebold
left a comment
There was a problem hiding this comment.
Seems ok to me, though I wonder whether if we still have TypeParamId and ConstParamId we should still be using them in some places that explicitly expect a type/const param.
| pub enum TypeNs { | ||
| SelfType(ImplId), | ||
| GenericParam(TypeParamId), | ||
| GenericParam(TypeOrConstParamId), |
There was a problem hiding this comment.
in type ns, this has to be a type, doesn't it? so why can't this stay a TypeParamId?
There was a problem hiding this comment.
Test hover_const_param fails if I change it. Seems I have broken something.
hkalbasi
force-pushed
the
const_generic
branch
4 times, most recently
from
c3f3605 to
c334e92
Compare
|
bors d+ |
|
✌️ HKalbasi can now approve this pull request. To approve and merge a pull request, simply reply with |
|
bors r+ |
4 participants
rust-lang/rust#90207 removed order restriction of generic args, i.e. const generics can now become before of type generics. We need to preserve this order to analyze correctly, and this PR does that.
It also simplifies implementation of const generics a bit IMO.
Implementing default generics the same problem of #7434, we need lower them to body and then evaluate them.