Stream: t-compiler/wg-rls-2.0

Topic: Auto importing traits


Kirill Bulatov (Feb 10 2020 at 23:09, on Zulip):

Is there any sane way to get a collection of ModuleDef::Trait that correspond to a given hir Adt?
Or maybe a way to get a collection of ModuleDef::Trait that have a hir Function defined in them?

In other words, I want to either find all traits that are implemented for a given Adt or find all traits that have the particular function definition.

One strong limitation that makes it fun: the trait is not in the current scope (i.e. no use statement for it from the place we perform a search hence no SourceAnalyzer::resolve_path can be used).

So far I've found an eerie way to get that as

ImplBlock::all_in_crate
-> filter out the ones that target the Adt only
-> get the ra_hir_def::TypeRef via ImplBlock::target_trait on the remaining
-> scrape the trait name out of it (by considering the TypeRef::Path variants only)
-> do ImportsLocator::find_imports on the name

Can there be a better way?
It would be great to get the Trait instead of TypeRef somehow at least.

Florian Diebold (Feb 11 2020 at 08:08, on Zulip):

Yeah, I wouldn't recommend that approach :sweat_smile: This needs to use the method resolution code and tell it to consider all traits, not just the ones in scope (that option doesn't exist yet)

Kirill Bulatov (Feb 11 2020 at 08:27, on Zulip):

I see that the resolution method boils down to this: https://github.com/rust-analyzer/rust-analyzer/blob/master/crates/ra_hir_ty/src/method_resolution.rs#L385

And if I get it right, the method just adds the traits in scope to the iteration, that's it.
So, in order to make it to consider all traits, I have to get all traits first, haven't I?

How do I do that?
I wonder why do I need to change this method, if I'm able to get all traits? :)

Florian Diebold (Feb 11 2020 at 08:31, on Zulip):

You're right, you don't need to change it since it takes the set of traits as a parameter nowadays

Florian Diebold (Feb 11 2020 at 09:25, on Zulip):

I think to get the set of candidate traits the best way would be to go through the CrateDefMap of the current crate and its direct dependencies, and collect the set of all traits in all modules in there. (We might want to make an index of that, but I guess it's better to actually first try the brute-force way)

Kirill Bulatov (Feb 11 2020 at 09:43, on Zulip):

Thanks, I'll try that.

matklad (Feb 11 2020 at 10:28, on Zulip):

I think the right approach here is to filter by function first (using the symbol index)

matklad (Feb 11 2020 at 10:28, on Zulip):

I hope we can live without "find all traits for a type", as that doesn't look like it can scale.

Florian Diebold (Feb 11 2020 at 10:34, on Zulip):

method resolution filters by the function first anyway

Florian Diebold (Feb 11 2020 at 10:35, on Zulip):

but getting all traits with a function with a certain name via an index would of course be nicer

Kirill Bulatov (Feb 11 2020 at 13:03, on Zulip):

If there's a way to get to get impls for a given Trait or TraitDef, I can avoid using the method_resolution method at all.
I think I should, because otherwise I need to soemhow make it available in ra_assists crate which does not make much sense to me.

Kirill Bulatov (Feb 11 2020 at 13:06, on Zulip):

And I think there is a way via ImplBlock::for_trait, so all good.

Florian Diebold (Feb 11 2020 at 13:14, on Zulip):

you have to use method resolution, otherwise you will get wrong and incomplete results

Florian Diebold (Feb 11 2020 at 13:17, on Zulip):

actually, we already expose it: https://github.com/rust-analyzer/rust-analyzer/blob/3c98681d4efeabe91dc91ff3ef0e1ce11c35d067/crates/ra_hir/src/code_model.rs#L1025

Kirill Bulatov (Feb 11 2020 at 13:29, on Zulip):

Well, this method does not find me any candidates for my tiny test.
I'll try to find out why, hope it does not depend on the scope somehow.

Kirill Bulatov (Feb 11 2020 at 13:48, on Zulip):

I think that happens because of this: https://github.com/rust-analyzer/rust-analyzer/blob/3c98681d4efeabe91dc91ff3ef0e1ce11c35d067/crates/ra_hir/src/code_model.rs#L1048

Currently I'm trying to import traits for the associated trait functions:

mod test_mod {
    pub trait TestTrait {
        fn test_function();
    }
    pub struct TestStruct {}
    impl TestTrait for TestStruct {
        fn test_function() {}
    }
}

fn main() {
    test_mod::TestStruct::test_function<|>
}

and all my traits get rejected here: https://github.com/rust-analyzer/rust-analyzer/blob/3c98681d4efeabe91dc91ff3ef0e1ce11c35d067/crates/ra_hir_ty/src/method_resolution.rs#L454

Kirill Bulatov (Feb 11 2020 at 13:49, on Zulip):

If I replace the LookupMode with the other variant, I get something passed into the closure.

So, should we expose another method? Or rather allow to specify the LookupMode as a parameter?
Or am I doing something wrong?

Florian Diebold (Feb 11 2020 at 13:51, on Zulip):

for a path like that, you need LookupMode::Path, yeah

Florian Diebold (Feb 11 2020 at 13:52, on Zulip):

we should maybe expose another variant that uses LookupMode::Path and also supports returning associated constants

Florian Diebold (Feb 11 2020 at 13:53, on Zulip):

maybe call it iterate_associated_candidates or something like that

Florian Diebold (Feb 11 2020 at 13:53, on Zulip):

(I was assuming you're doing method calls like foo.bar())

Kirill Bulatov (Feb 11 2020 at 13:54, on Zulip):

Sounds reasonable, not sure how to pass the AssocItemId::ConstId into that callback though.
I'll try something.

Florian Diebold (Feb 11 2020 at 13:54, on Zulip):

we have an enum AssocItem in code_model

Kirill Bulatov (Feb 11 2020 at 13:59, on Zulip):

Oh wait, there's already a method that does it :)
Thanks for the guidance anyway, that helps a lot.

Kirill Bulatov (Feb 11 2020 at 14:08, on Zulip):

Ok, one last set of questions: how is the https://github.com/rust-analyzer/rust-analyzer/blob/3c98681d4efeabe91dc91ff3ef0e1ce11c35d067/crates/ra_hir/src/code_model.rs#L1056 method supposed to help me to detect the traits I want to import?

The callback signature is mut callback: impl FnMut(&Ty, AssocItem) -> Option<T>, so it does not return a trait.
Or is the plan to call the method for each trait I've located before, one by one and see if something gets passed into the callback for that trait?

Kirill Bulatov (Feb 11 2020 at 14:10, on Zulip):

And do I get it right, that ImplBlock::for_trait approach is worse than the way you've proposed because it does not consider derefs?

Florian Diebold (Feb 11 2020 at 14:53, on Zulip):

once you have the AssocItem, you can get its container, which is the trait

Kirill Bulatov (Feb 11 2020 at 14:53, on Zulip):

It is not, that's the trick, it's the Function.

Florian Diebold (Feb 11 2020 at 14:54, on Zulip):

for a Function as well you can get the container. you might need to add methods for it in code_model, but it's straightforward

Kirill Bulatov (Feb 11 2020 at 14:54, on Zulip):

Ah, silly me, got it. Thanks.

Florian Diebold (Feb 11 2020 at 14:56, on Zulip):

And do I get it right, that ImplBlock::for_trait approach is worse than the way you've proposed because it does not consider derefs?

derefs aren't relevant for path-style method calls, but to decide whether a type implements a trait you have to use chalk, you can't just look at the impls. consider impls like impl<T> Trait for T where T: SomeOtherTrait

Last update: Sep 22 2020 at 03:00UTC