Simplify resolve_lifetimes #57408
Simplify resolve_lifetimes #57408
Conversation
rust-highfive
added
the
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
ljedrz
force-pushed
the
simplify_resolve_lifetimes
branch
from
36cadcb to
0aafea8
Compare
ljedrz
force-pushed
the
simplify_resolve_lifetimes
branch
from
0aafea8 to
a82a741
Compare
|
|
||
| #[derive(Clone, RustcEncodable, RustcDecodable, Debug)] | ||
| pub struct GenericParam { | ||
| pub id: NodeId, |
There was a problem hiding this comment.
Can we avoid having NodeId at all? Then this would be a clear win. Have you looked at the remaining uses of id for GenericParam and Lifetime?
There was a problem hiding this comment.
I have, and there were quite a few. I can try giving it a shot 👍.
There was a problem hiding this comment.
@varkor Actually this is super complex; NodeId is still used in lots of places and just removing it causes lots of breakage, making it hard to debug. I can see that most Hir objects still point to NodeId, probably for the same reason. I have a suggestion: I will try to migrate methods working on NodeId to HirId in following PRs; at one point the compiler should just tell us that the NodeId field is not used anymore - that approach should be a lot cleaner.
|
Do you think the extra memory required to hold |
|
FWIW, in the meantime I'm trying to go HARD (the only way possible tbh) on |
|
Closing in favor of #57578. |
3 participants
Obtaining
hir_idis free during lowering, so I thought it might be a good idea to save it while creatinghir::Lifetimeandhir::GenericParamso thatresolve_lifetimescan do less work.I'm pretty sure
resolve_lifetimescan be simplified further (I'd like to squashNamedRegionMapwithResolveLifetimes), but I need to sleep on it; this seems like a step in the right direction.As an added bonus this should be a step forward for #50928.
r? @varkor