Use HashStable_Generic in rustc_type_ir #101549
Conversation
A lot of the types in this crate implemented HashStable directly to avoid circular dependencies. One way around that is to use HashStable_Generic. We adopt that here to avoid a lot of boilerplate. This doesn't update all the types, because some would require `I: Interner + HashStable`.
| @@ -21,6 +21,7 @@ rustc_serialize = { path = "../rustc_serialize" } | |||
| rustc_session = { path = "../rustc_session" } | |||
| rustc_span = { path = "../rustc_span" } | |||
| rustc_target = { path = "../rustc_target" } | |||
| rustc_type_ir = { path = "../rustc_type_ir" } | |||
There was a problem hiding this comment.
Isn't this bad? It adds a new dependency edge that might make the critical path longer (since query_system AFAIK is a very slow crate).
There was a problem hiding this comment.
I wonder if we could move the HashStableContext trait to a higher level crate (like whatever one defines HashStable_Generic) and do a use HashStableContext at the root? It seems like if this is going to be a common pattern where we need these marker traits for anyone who is going to use HashStable_Generic, it'd be nice to use the same one if possible.
There was a problem hiding this comment.
FWIW, I don't think it's so straightforward whether this will regression compile times, since many of the other dependencies of rustc_query_system are not dependencies of rustc_type_ir.
Rollup of 6 pull requests Successful merges: - rust-lang#101153 (Migrate another part of rustc_infer to session diagnostic) - rust-lang#101399 (Shrink span for bindings with subpatterns.) - rust-lang#101422 (Hermit: Add File::set_time stub) - rust-lang#101455 (Avoid UB in the Windows filesystem code in... bootstrap?) - rust-lang#101498 (rustc: Parameterize `ty::Visibility` over used ID) - rust-lang#101549 (Use HashStable_Generic in rustc_type_ir) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
|
@jackh726 took the review, approved it, and I don't mind. Everything is ok. |
A lot of the types in this crate implemented HashStable directly to avoid circular dependencies. One way around that is to use HashStable_Generic. We adopt that here to avoid a lot of boilerplate.
This doesn't update all the types, because some would require
I: Interner + HashStable.r? @cjgillot