Stream: t-compiler/wg-rls-2.0

Topic: Moving Call Info to HIR


matklad (Jul 16 2020 at 13:34, on Zulip):

Hey, @Florian Diebold , I am trying to move call info over to hir types (to support closures, and to avoid string-based semantic analysis).

I think I am hitting a couple of surprising limitations of our inference infra

matklad (Jul 16 2020 at 13:35, on Zulip):

Here's the first one:

image.png

matklad (Jul 16 2020 at 13:36, on Zulip):

Here, we are showing hints for a function with two generic params: foo(T, U)

matklad (Jul 16 2020 at 13:36, on Zulip):

We know value only for the first param, so the hint shows (i32, {unknown})

matklad (Jul 16 2020 at 13:37, on Zulip):

Now, it would be cool if that {unknown} said U instead. It seems like we might want to extend the defition of Ty::Unknown to not erase this information somehow

Florian Diebold (Jul 16 2020 at 13:38, on Zulip):

hm. It's not U though -- like, U is not a type that even exists at that location. This feels like something that should be tracked on some side channel maybe (i.e., why did a type variable get created)

Laurențiu Nicola (Jul 16 2020 at 13:39, on Zulip):

What if U is an associated type of T? Should it say i32::Foo?

matklad (Jul 16 2020 at 13:39, on Zulip):

The second limitation is simpler. For method calls like x.foo(a, b), we don't seem to record substitutions for foo anywhere. Our table stores only FunctionId.

Note that this problem does not exist for function calls (X::foo(x, a, b)), as we can look up the type of X::foo expr, and that carries subs.

Florian Diebold (Jul 16 2020 at 13:39, on Zulip):

yeah we probably want to just record them

matklad (Jul 16 2020 at 13:40, on Zulip):

like, U is not a type that even exists at that location.

Exactly! The Ty::Unknown is the right type. But it seems like we know that we've got that particular unknown from a type parameter, so we might want to change Unknown to Unknown { type_param: Option<TypeParamId> } or something like that.

Florian Diebold (Jul 16 2020 at 13:44, on Zulip):

so, I'd like to keep additional information out of Ty because in some not-so-far-off future, Ty won't exist anymore and we'll be using chalk_ir::Ty. I think it'd make more sense to 1. have some way of knowing the exact type variable that Unknown came from, and then 2. be able to look up why that variable was created (which I guess would be something like TypeVariableSource::TypeParam(TypeParamId)). I think rustc keeps track of that quite similarly for diagnostics, haven't looked into it yet though

Florian Diebold (Jul 16 2020 at 13:45, on Zulip):

I'm not sure what part would see that and decide to print U instead of {unknown} though :thinking:

Florian Diebold (Jul 16 2020 at 13:46, on Zulip):

it feels wrong to do that in the HIR code, since it's not the correct type, it's more of an IDE decision, but then the IDE code needs to have their own printing :confused:

matklad (Jul 16 2020 at 13:47, on Zulip):

Yeah, that's more general problem: I think something about paths/auto-import already bleeds into HirDisplay?

matklad (Jul 16 2020 at 13:48, on Zulip):

Drawing the line between IDE and compiler feels like separating the toothpaste into three colored strands :)

Florian Diebold (Jul 16 2020 at 13:48, on Zulip):

yeah, it optionally tries to print correct importable paths

Florian Diebold (Jul 16 2020 at 13:52, on Zulip):

I have a plan to fix that, which is basically to use the new type IR I'm slowly working on also in the other direction

matklad (Jul 16 2020 at 13:53, on Zulip):

The separate IR from chalk's?

matklad (Jul 16 2020 at 13:53, on Zulip):

So that we have two type reprs?

matklad (Jul 16 2020 at 13:53, on Zulip):

Adding a new IR always feels like a right solution :D

Florian Diebold (Jul 16 2020 at 13:53, on Zulip):

yeah, basically an IR that represents user-facing types, but resolved

Florian Diebold (Jul 16 2020 at 13:54, on Zulip):

that doesn't help with the type variable problem though

Florian Diebold (Jul 16 2020 at 13:54, on Zulip):

we already have two type reprs, TypeRef and Ty, this would be in between them ;)

Florian Diebold (Jul 16 2020 at 14:09, on Zulip):

@matklad maybe it would be enough of an interim solution to just print _ instead of {unknown}?

matklad (Jul 16 2020 at 14:10, on Zulip):

Nah, for interim solution {unknown} is fine :D

matklad (Jul 16 2020 at 14:11, on Zulip):

Though, Ive been debating if we should render {unknown} as ?

matklad (Jul 16 2020 at 14:11, on Zulip):

(that is, everywhere -- in hover, in inlay hints, etc)

Florian Diebold (Jul 16 2020 at 14:31, on Zulip):

_ makes sense to me because it is actually an inference variable. Maybe it'd be good to distinguish between error types and unresolved inference variables later

matklad (Jul 16 2020 at 14:32, on Zulip):

Yeah, _ also makes sense... I am thinking though if it would be self-explanatory.

If you see let x: Vec<?> = ... it's self-evident that IDE has trouble inferring ?. If you see let x: Vec<_>, you might think that IDE shortents type here, or something.

matklad (Jul 16 2020 at 14:33, on Zulip):

Aren't we actually using _ for type shortening already?

Florian Diebold (Jul 16 2020 at 14:33, on Zulip):

I think we're using ... for type shortening

Last update: Sep 27 2020 at 14:30UTC