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?
db905bb to
2ba3cad
Compare
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? |
2ba3cad to
c51caa5
Compare
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
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.
crates/hir_def/src/resolver.rs
Outdated
| 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.
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+ |
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.