Stream: t-compiler/rust-analyzer

Topic: Devitrualizing diagnostics


matklad (Apr 26 2021 at 17:00, on Zulip):

Hey, anyone got strong feelings about https://github.com/rust-analyzer/rust-analyzer/issues/8672 ?

matklad (Apr 26 2021 at 17:01, on Zulip):

It's going to be a major refactor, I am 80% sure that it's the right call, but I want to double check

Jonas Schievink [he/him] (Apr 26 2021 at 17:02, on Zulip):

no strong feelings one way or the other. if we do change it, what do we gain?

matklad (Apr 26 2021 at 17:07, on Zulip):

I bunch of simplicity in one of the more complicated parth of the code base.

matklad (Apr 26 2021 at 17:08, on Zulip):

Like, I dread working on the diagnostics code, because the level of indirecion is just way over my head :)

Jonas Schievink [he/him] (Apr 26 2021 at 17:09, on Zulip):

oh, okay, then it's probably worth it

matklad (Apr 26 2021 at 17:10, on Zulip):

I think about the case of removing generics from protocol conversion code

https://github.com/rust-analyzer/rust-analyzer/pull/4418/commits/1586bab0b97bef411e6187dfc389557edbc5a16e

matklad (Apr 26 2021 at 17:10, on Zulip):

That definitely was very helpful

Florian Diebold (Apr 26 2021 at 17:29, on Zulip):

won't this lead to a bunch of enums like hir_expand::Diagnostic and 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 hir::Diagnostics?

Florian Diebold (Apr 26 2021 at 17:30, on Zulip):

I _guess_ hir_ty's methods could take a type parameter D: From<hir_ty::Diagnostic>

Florian Diebold (Apr 26 2021 at 17:30, on Zulip):

not sure that still looks less complicated than the current setup though

matklad (Apr 26 2021 at 17:47, on Zulip):

@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 hir/src/lib.rs),

matklad (Apr 26 2021 at 17:47, on Zulip):

This is already happening in some sense:

image.png

matklad (Apr 26 2021 at 17:47, on Zulip):

the impl block sort-of duplicates the struct

matklad (Apr 26 2021 at 17:48, on Zulip):

For accumulator style, we'll be just pushing onto a Vec

matklad (Apr 26 2021 at 17:51, on Zulip):

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

matklad (Apr 26 2021 at 17:58, on Zulip):

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

matklad (Apr 26 2021 at 17:58, on Zulip):

(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)

Jeremy Kolb (Apr 27 2021 at 12:01, on Zulip):

I think this would be a good cleanup. The diagnostics code is really kind of scary.

Last update: Jul 26 2021 at 12:30UTC