Hey, anyone got strong feelings about https://github.com/rust-analyzer/rust-analyzer/issues/8672 ?
It's going to be a major refactor, I am 80% sure that it's the right call, but I want to double check
no strong feelings one way or the other. if we do change it, what do we gain?
I bunch of simplicity in one of the more complicated parth of the code base.
Like, I dread working on the diagnostics code, because the level of indirecion is just way over my head :)
oh, okay, then it's probably worth it
I think about the case of removing generics from protocol conversion code
That definitely was very helpful
won't this lead to a bunch of enums like
hir_ty::Diagnostic that are subsets of the big
hir::Diagnostic enum? actually, how would this even look with accumulator style, since e.g.
hir_ty can't produce
I _guess_ hir_ty's methods could take a type parameter
not sure that still looks less complicated than the current setup though
@Florian Diebold I am imagining the setup like in the rest of hir. There's a bunch of diagnostics with raw ids in various crates. Then, there's this giant enum in hir, which mirrors those raw diagnostics, but encapsulates the types, and the code to map between the two (so, a whole tone of boilerplate, a-la
This is already happening in some sense:
the impl block sort-of duplicates the
For accumulator style, we'll be just pushing onto a Vec
We actually already do collect to vec in the most importatn case -- fn bodies: https://github.com/matklad/rust-analyzer/blob/f06e4b8e74bc2cec1fec4075f64b9909356c2bd3/crates/hir_def/src/body.rs#L419-L424
I guess, there's also this issue:
It seems that diagnostics should speak the language of
hir. That's our public API, and seems the right level of abstraction for consumption of diagnostics. But the
trait Diagnostic is defined in
hir_expand, so it can't use hir-types
(there's an obvious problem here is that we don't really have
hir for expressions, but that's where most of the diagnostics are coming from)
I think this would be a good cleanup. The diagnostics code is really kind of scary.