Stream: t-compiler/rust-analyzer

Topic: issue rust-analyzer#8100


cynecx (May 30 2021 at 20:56, on Zulip):

@Florian Diebold I am currently looking into https://github.com/rust-analyzer/rust-analyzer/issues/8100.

I think the substitution might be wrong here:

https://github.com/rust-analyzer/rust-analyzer/blob/92b9e5ef3c03d51713ff5fa32cd58bdf97701b5e/crates/hir_ty/src/infer/expr.rs#L897

Logging:

StructId(0) = Box
StructId(1) = Foo
[crates/hir_ty/src/infer/expr.rs:895] canonicalized_receiver.decanonicalize_ty(ty) = AdtId(StructId(StructId(1)))<[?0 := Int(I32)]>
[crates/hir_ty/src/infer/expr.rs:896] generics(self.db.upcast(), func.into()) = Generics {
    def: FunctionId(
        FunctionId(
            4,
        ),
    ),
    params: GenericParams {
        types: Arena {
            len: 0,
            data: [],
        },
        lifetimes: Arena {
            len: 1,
            data: [
                LifetimeParamData {
                    name: Name(
                        Text(
                            "'a",
                        ),
                    ),
                },
            ],
        },
        consts: Arena {
            len: 0,
            data: [],
        },
        where_predicates: [],
    },
    parent_generics: Some(
        Generics {
            def: ImplId(
                ImplId(
                    1,
                ),
            ),
            params: GenericParams {
                types: Arena {
                    len: 1,
                    data: [
                        TypeParamData {
                            name: Some(
                                Name(
                                    Text(
                                        "T",
                                    ),
                                ),
                            ),
                            default: None,
                            provenance: TypeParamList,
                        },
                    ],
                },
                lifetimes: Arena {
                    len: 0,
                    data: [],
                },
                consts: Arena {
                    len: 0,
                    data: [],
                },
                where_predicates: [],
            },
            parent_generics: None,
        },
    ),
}
[crates/hir_ty/src/infer/expr.rs:897] self.substs_for_method_call(generics, generic_args, &ty) = [?0 := ?5]

I would've expected that we would have applied the subst from ty (?0 := Int(I32)) instead of ?0 := ?5 (new inference var).

Am I missing something?

Florian Diebold (May 30 2021 at 21:01, on Zulip):

no, that happens later through

self.unify(&expected_receiver_ty, &actual_receiver_ty);
cynecx (May 30 2021 at 21:01, on Zulip):

Ahh

cynecx (May 30 2021 at 21:02, on Zulip):

lol i should've seen that, i didn't read far enough

Florian Diebold (May 30 2021 at 21:04, on Zulip):

keep in mind you might have something like impl<A, B> SomeType<SomeOtherType<B>, StillSomeOtherType<A>>

Florian Diebold (May 30 2021 at 21:04, on Zulip):

so you can't just take the substs

cynecx (May 30 2021 at 21:11, on Zulip):
[crates/hir_ty/src/infer/expr.rs:928] expected_receiver_ty = (&'static AdtId(StructId(StructId(0)))<[?0 := AdtId(StructId(StructId(1)))<[?0 := ?2]>]>)
[crates/hir_ty/src/infer/expr.rs:928] actual_receiver_ty = (&'static AdtId(StructId(StructId(1)))<[?0 := Int(I32)]>)

should unify "work" on that? that looks wrong? (Or I might have a different understanding what unify should do lol)

cynecx (May 30 2021 at 21:13, on Zulip):

oh wait

cynecx (May 30 2021 at 21:17, on Zulip):

well nvm, idk this might be over my head xD. i might pick this up later.

Florian Diebold (May 30 2021 at 21:59, on Zulip):

those are completely different structs

Florian Diebold (May 30 2021 at 21:59, on Zulip):

ah, the expected receiver ty is wrapped in something additional

Florian Diebold (May 30 2021 at 22:00, on Zulip):

yeah I don't think this is trivial to do, but I don't remember the details

cynecx (May 31 2021 at 16:19, on Zulip):

On second thought:

test case:

#[lang = "deref"]
pub trait Deref {
    type Target;
    fn deref(&self) -> &Self::Target;
}

struct Box<T>(T);

impl<T> Deref for Box<T> {
    type Target = T;
    fn deref(&self) -> &Self::Target;
}

struct Foo<T>(T);

impl<T> Foo<T> {
    fn get_inner<'a>(self: &'a Box<Self>) -> &'a T {}

    // fn get_self<'a>(self: &'a Box<Self>) -> &'a Self {}
}

fn main() {
    let boxed = Box(Foo(0_i32));
    let bad1 = boxed.get_inner();
}

Here https://github.com/rust-analyzer/rust-analyzer/blob/10b15b27f2f4bee6de63f66c344f741482becd21/crates/hir_ty/src/infer/expr.rs#L883, we are resolving the method, which returns:

[crates/hir_ty/src/infer/expr.rs:893] resolved = Some(
    (
        AdtId(StructId(StructId(1)))<[?0 := Int(I32)]>, // Foo<i32>
        FunctionId(
            3, // get_inner
        ),
    ),
)
[crates/hir_ty/src/infer/expr.rs:895] canonicalized_receiver.decanonicalize_ty(dbg!(ty)) = AdtId(StructId(StructId(1)))<[?0 := Int(I32)]> //  derefed_receiver_ty = Foo<i32>

This (derefed_receiver_ty) seems wrong to me because we aren't actually deref'ing anything except while resolving the method because the receiver type accepts the "underefed" type (Box<Foo<i32>>).

Florian Diebold (May 31 2021 at 16:30, on Zulip):

yes, the determination of the receiver type is wrong

cynecx (May 31 2021 at 17:08, on Zulip):

Okay, I may or may not have found the issue

cynecx (May 31 2021 at 17:08, on Zulip):

https://github.com/rust-analyzer/rust-analyzer/blob/10b15b27f2f4bee6de63f66c344f741482becd21/crates/hir_ty/src/method_resolution.rs#L724

cynecx (May 31 2021 at 17:09, on Zulip):

We are using self_ty.value here which might differ from the actual receiver type.

cynecx (May 31 2021 at 17:09, on Zulip):

We should've used receiver_ty: Option<&Canonical<Ty>> instead here

cynecx (May 31 2021 at 17:11, on Zulip):

Hm, it seems that the type also has an extra ref &'static <receiver_ty>

cynecx (May 31 2021 at 17:12, on Zulip):

the logic here is complicated lol

cynecx (May 31 2021 at 17:12, on Zulip):

i am not even sure i totally understand whats going on here

cynecx (May 31 2021 at 17:14, on Zulip):

Oh nice, it seems to actually work that way:

let receiver_ty = if let Some(receiver_ty) = receiver_ty {
    match receiver_ty.value.kind(&Interner) {
        TyKind::Ref(_, _, ty) => ty,
        _ => &receiver_ty.value,
    }
} else {
    &self_ty.value
}
if callback(receiver_ty, item) {
    return true;
}
cynecx (May 31 2021 at 17:18, on Zulip):

but i don't think that is correct (stripping the reference)

Florian Diebold (May 31 2021 at 17:49, on Zulip):

hmm

Florian Diebold (May 31 2021 at 17:50, on Zulip):

something about this smells, admittedly... method resolution is returning the self_ty, but inference then treats it as receiver_ty. those are different things

Florian Diebold (May 31 2021 at 17:51, on Zulip):

aah but with non-arbitrary self types, the only difference is the optional autoref

Florian Diebold (May 31 2021 at 17:53, on Zulip):

I kind of think you're right that method resolution should be returning receiver_ty

Florian Diebold (May 31 2021 at 17:54, on Zulip):

and maybe we don't actually need these lines then

cynecx (May 31 2021 at 17:55, on Zulip):

ah

cynecx (May 31 2021 at 17:56, on Zulip):

let me try that

cynecx (May 31 2021 at 17:58, on Zulip):

nice that works :D

Florian Diebold (May 31 2021 at 17:58, on Zulip):

o_O is it really that simple

cynecx (May 31 2021 at 17:58, on Zulip):

hir_ty test suite also passes

cynecx (May 31 2021 at 17:59, on Zulip):

let me open up a PR

cynecx (May 31 2021 at 18:06, on Zulip):

https://github.com/rust-analyzer/rust-analyzer/pull/9090

Last update: Jul 24 2021 at 19:30UTC