Stream: t-compiler/rust-analyzer

Topic: get a module from a `self` param


Yoshua Wuyts (Apr 17 2021 at 20:10, on Zulip):

Hey, I'm working on https://github.com/rust-analyzer/rust-analyzer/issues/8555 - and I'm not sure how to a reference to the current Module the file refers to. Is there a convenient way to get this?

Lukas Wirth (Apr 17 2021 at 20:12, on Zulip):

I think the simplest way is to go with sema.scope(syntax_node).module()

Yoshua Wuyts (Apr 17 2021 at 20:12, on Zulip):

Or I guess a way to go from a hir::Type to a Module. I'm trying to do a non-local way to get from a given variable to an impl block

Yoshua Wuyts (Apr 17 2021 at 20:12, on Zulip):

@Lukas Wirth ohh, I'll give that a shot!

Yoshua Wuyts (Apr 17 2021 at 20:23, on Zulip):

I guess part of what I'm trying to do is deprecate all uses of https://github.com/rust-analyzer/rust-analyzer/blob/master/crates/ide_assists/src/utils.rs#L310-L318, in favor of going through the hir db

Yoshua Wuyts (Apr 17 2021 at 20:24, on Zulip):

"list me all impl blocks associated with this type, so I can pick the first one" is something which should be answerable

Yoshua Wuyts (Apr 17 2021 at 20:27, on Zulip):

It appears to be somewhat difficult right now to go from "here's a value" to "here are the impl blocks associated with that value's type"

Yoshua Wuyts (Apr 17 2021 at 20:28, on Zulip):

(Mostly just typing stuff out as I go, if anyone knows of better approaches / prior art on this, please do chime in!)

Lukas Wirth (Apr 17 2021 at 20:29, on Zulip):

There is infra to query all impls for a type, you could pick one of those then and map that back to the source node

Lukas Wirth (Apr 17 2021 at 20:29, on Zulip):

thats what you wanna do right?

Lukas Wirth (Apr 17 2021 at 20:30, on Zulip):

https://github.com/Veykril/rust-analyzer/blob/58a6ec549d638455f5b5447bbbc174993e51e288\crates\hir\src\lib.rs#L1571

Lukas Wirth (Apr 17 2021 at 20:31, on Zulip):

You'd have to filter out trait impls though since you dont want those i imagine

Yoshua Wuyts (Apr 17 2021 at 20:32, on Zulip):

oh -- yeah that's a good idea!

Lukas Wirth (Apr 17 2021 at 20:32, on Zulip):

Then there is the HasSource tree that maps defs back to their source node if they have one

Lukas Wirth (Apr 17 2021 at 20:32, on Zulip):

which is implemented for Impl so that should work

Yoshua Wuyts (Apr 17 2021 at 20:37, on Zulip):

oh, yay! -- I successfully got an impl block!

Yoshua Wuyts (Apr 17 2021 at 20:37, on Zulip):

okayyyyyyy, let's gooooo

Yoshua Wuyts (Apr 17 2021 at 20:58, on Zulip):

@Lukas Wirth is there a canonical way to go from a hir type to an ast type?

Lukas Wirth (Apr 17 2021 at 20:58, on Zulip):

Uuh

Yoshua Wuyts (Apr 17 2021 at 20:58, on Zulip):

I've got the hir::Impl but I want its position in the AST (ast::Impl)

Yoshua Wuyts (Apr 17 2021 at 20:58, on Zulip):

not sure if this question makes sense?

Lukas Wirth (Apr 17 2021 at 20:58, on Zulip):

going from hir::Impl to ast::Impl is simple

Lukas Wirth (Apr 17 2021 at 20:58, on Zulip):

There is a trait for that HasSource

Lukas Wirth (Apr 17 2021 at 20:59, on Zulip):
pub trait HasSource {
    type Ast;
    fn source(self, db: &dyn HirDatabase) -> Option<InFile<Self::Ast>>;
}
Lukas Wirth (Apr 17 2021 at 21:00, on Zulip):

so you should be able to just go with impl_.source(db) and get your ast::Impl out of that

Yoshua Wuyts (Apr 17 2021 at 21:01, on Zulip):

oh yay, thank you!

Yoshua Wuyts (Apr 17 2021 at 22:16, on Zulip):

gah, I managed to get hir::Impl::all_for_type to work, but now it's returning empty lists again -- not sure what I changed

Yoshua Wuyts (Apr 17 2021 at 22:16, on Zulip):

think it's time to call it a day, heh

Yoshua Wuyts (Apr 17 2021 at 22:17, on Zulip):

pushed the branch https://github.com/rust-analyzer/rust-analyzer/compare/master...yoshuawuyts:whats-a-method-to-a-method-call in case I don't get around to finishing this up

Lukas Wirth (Apr 17 2021 at 22:43, on Zulip):

I think it might be returning nothing when your type is a reference, you would have to strip the type of references before asking for impls first.

Lukas Wirth (Apr 17 2021 at 22:44, on Zulip):

If I recall correctly impl search doesn't like references(for some valid reasons I don't remember right now)

Yoshua Wuyts (Apr 17 2021 at 22:52, on Zulip):

@Lukas Wirth what do you mean by "stripping it of references" -- cast it somehow?

Lukas Wirth (Apr 17 2021 at 22:52, on Zulip):

ah no, I mean the type the hir::Type represents

Lukas Wirth (Apr 17 2021 at 22:52, on Zulip):

if that is a reference type that it represetns it probably doesnt work

Lukas Wirth (Apr 17 2021 at 22:52, on Zulip):

let me check the api quickly

Yoshua Wuyts (Apr 17 2021 at 22:53, on Zulip):

oh whattttt

Yoshua Wuyts (Apr 17 2021 at 22:53, on Zulip):

that worked?

Yoshua Wuyts (Apr 17 2021 at 22:53, on Zulip):

I changed the signature from &self to self and swoosh it's working again

Lukas Wirth (Apr 17 2021 at 22:54, on Zulip):

Ye okay then my thought was correct

Lukas Wirth (Apr 17 2021 at 22:54, on Zulip):

I think we index impls by non-reference types for simplicity

Yoshua Wuyts (Apr 17 2021 at 22:54, on Zulip):

oh wow, would not have guessed that this was the issue -- been at this for a good while now haha

Lukas Wirth (Apr 17 2021 at 22:54, on Zulip):

Ye its a bit weird :sweat_smile:

Lukas Wirth (Apr 17 2021 at 22:55, on Zulip):

so you wanna strip the references of the hir type

Yoshua Wuyts (Apr 17 2021 at 22:55, on Zulip):

yes please!

Lukas Wirth (Apr 17 2021 at 22:55, on Zulip):
std::iter::successors(Some(ty.clone()), |ty| ty.remove_ref())
        .last()

something like this

Lukas Wirth (Apr 17 2021 at 22:55, on Zulip):

should do the trick, we should add a method for this to hir::Type at some point I think since this is done in multiple locations by now

Lukas Wirth (Apr 17 2021 at 22:56, on Zulip):

Feel free to ask and ping if you get stuck on things like these :big_smile:

Lukas Wirth (Apr 17 2021 at 22:56, on Zulip):

Dont mind helping if I'm online that is

Yoshua Wuyts (Apr 17 2021 at 22:56, on Zulip):

much appreciated!

Yoshua Wuyts (Apr 17 2021 at 22:57, on Zulip):

I guess I got angry-stuck on this one -- I had it working once, so I could get it working again :P

Lukas Wirth (Apr 17 2021 at 22:57, on Zulip):

I guess this is the problem with undocumented functions that have weird special case behaviours :sweat_smile:

Lukas Wirth (Apr 17 2021 at 22:57, on Zulip):

Been there

Yoshua Wuyts (Apr 17 2021 at 22:58, on Zulip):

it's rarely fruitful though -- appreciate the offer to help, and will def opt to do that next time :sweat_smile:

Lukas Wirth (Apr 17 2021 at 23:03, on Zulip):

Getting into RA's codebase takes a bit of time, but once you got a grasp of things its a pleasure to work with in my opinion :big_smile:

Yoshua Wuyts (Apr 17 2021 at 23:20, on Zulip):

yeah, RA is so well structured! -- I've worked on a few compiler projects before, and RA certainly is among the easiest to work on

Yoshua Wuyts (Apr 17 2021 at 23:21, on Zulip):

just got my first test passing for this assist btw -- think it's time to call it a day

Yoshua Wuyts (Apr 17 2021 at 23:21, on Zulip):

goodnight, and thanks for the help!

Lukas Wirth (Apr 17 2021 at 23:23, on Zulip):

No problem and good night :smile:

Florian Diebold (Apr 18 2021 at 08:46, on Zulip):

Lukas Wirth said:

I guess this is the problem with undocumented functions that have weird special case behaviours :sweat_smile:

it's... not a weird special case behavior though? if you have impl S, it's an impl for S, not &S. Removing the references automatically would be the weird special case

Florian Diebold (Apr 18 2021 at 08:53, on Zulip):

Impl::all_for_type actually does have some weird special-case behavior in that it strips references from the self type in trait impls (so the other way around, it'll return impl T for &S if you give it justS), because it's currently mainly built for the impl search (the "N implementations" code lens on structs). It's not a good general abstraction :/

Florian Diebold (Apr 18 2021 at 08:54, on Zulip):

it might actually make sense to add a separate function to return only all inherent impls for a type

Yoshua Wuyts (Apr 18 2021 at 09:35, on Zulip):

@Florian Diebold I'll do that as part of this patch ^__^

Yoshua Wuyts (Apr 18 2021 at 09:40, on Zulip):

@Florian Diebold I have a question: I'm working with generated AST nodes created using make::<node_kind>. I have a method I'd like to attach to an Impl -- is there a good way of doing that?

Yoshua Wuyts (Apr 18 2021 at 09:41, on Zulip):

I'm looking at AssocItemList, but not sure how to attach one node within another

Yoshua Wuyts (Apr 18 2021 at 09:44, on Zulip):

ohh, now seeing Impl::with_assoc_item_list which returns another Impl

Lukas Wirth (Apr 18 2021 at 09:53, on Zulip):

Florian Diebold said:

it's... not a weird special case behavior though? if you have impl S, it's an impl for S, not &S. Removing the references automatically would be the weird special case

Right, I remembered this wrong, was thinking of the type fingerprints ignoring references

Florian Diebold (Apr 18 2021 at 10:08, on Zulip):

@Yoshua Wuyts yeah, with_assoc_item_list would be the current way; there's also @matklad 's new in-place editing infrastructure, with which you could have something like impl.add(your_function), but you'd have to add that (there's not much there yet, you can see it in edit_in_place.rs)

Yoshua Wuyts (Apr 18 2021 at 10:12, on Zulip):

oh cool, thanks!

Yoshua Wuyts (Apr 18 2021 at 10:12, on Zulip):

hehe yeah I'm a bit hesitant to use the edit_in_place stuff right now -- so think I'll stick with manually constructing the list, and maybe refactor later

Yoshua Wuyts (Apr 18 2021 at 10:13, on Zulip):

(side note: getting real close to having all common paths covered on this assist!)

Yoshua Wuyts (Apr 18 2021 at 11:21, on Zulip):

Is there a good way to go from a hir::Type to an ast::Path -- e.g. I'm trying to find out where a type has been defined so I can insert an impl block after it.

Yoshua Wuyts (Apr 18 2021 at 11:21, on Zulip):

cc/ @Lukas Wirth

Yoshua Wuyts (Apr 18 2021 at 11:23, on Zulip):

I guess Type::iterate_path_candidates ?

Lukas Wirth (Apr 18 2021 at 11:23, on Zulip):

That's a good question

Lukas Wirth (Apr 18 2021 at 11:24, on Zulip):

I think iterate_path_candidates iterates the associated things of a type, so probably not what you want

Lukas Wirth (Apr 18 2021 at 11:24, on Zulip):

Ye that iterates the associated items

Lukas Wirth (Apr 18 2021 at 11:24, on Zulip):

ah right you can go with ty.as_adt() and i think then you can use HasSource::source on it like you did for Impl already to get the file and position of the item definition

Lukas Wirth (Apr 18 2021 at 11:25, on Zulip):

Basically that gives you the Adt of the type, if it represents ones. So you have to use that on the reference stripped type again

Yoshua Wuyts (Apr 18 2021 at 11:26, on Zulip):

cool, I'll give that a shot!

Lukas Wirth (Apr 18 2021 at 11:28, on Zulip):

Ah you have to match on the adt, HasSource is implemented for Union, Enum and Struct separately since the ast types differ.

Yoshua Wuyts (Apr 18 2021 at 11:56, on Zulip):

Realizing this is taking up a fair bit of time; going to call it quits for now. GitHub issue is here: https://github.com/rust-analyzer/rust-analyzer/issues/8555#issuecomment-821979973

Yoshua Wuyts (Apr 18 2021 at 11:56, on Zulip):

Progress on this is here: https://github.com/rust-analyzer/rust-analyzer/compare/master...yoshuawuyts:whats-a-method-to-a-method-call

Yoshua Wuyts (Apr 18 2021 at 11:57, on Zulip):

If someone would like to finish this up, please do!

Yoshua Wuyts (Apr 18 2021 at 21:39, on Zulip):

heh, guess I came back to this

Yoshua Wuyts (Apr 18 2021 at 21:41, on Zulip):

@Lukas Wirth what if we had an "edit_in_place" helper which could be: edit_or_create_impl_block for any Type.

Lukas Wirth (Apr 18 2021 at 21:43, on Zulip):

Would be reasonable to add I believe

Lukas Wirth (Apr 18 2021 at 21:45, on Zulip):

The question is how to handle receiving an edited impl vs receiving a fresh impl in the assists

Lukas Wirth (Apr 18 2021 at 21:45, on Zulip):

Since one would updated an old impl and the other would require to add an new impl node

Yoshua Wuyts (Apr 18 2021 at 21:50, on Zulip):

yeah - I don't think I fully understand inserting of nodes in the tree yet

Lukas Wirth (Apr 18 2021 at 21:53, on Zulip):

inserting nodes is in general a bit weird, I don't even remember how we do it right now. Thats probably also the reason why a lot of assists just insert via text range positions. Ideally most of that will become simpler/nicer to look at though now that we have opt-in mutable syntex trees

Yoshua Wuyts (Apr 18 2021 at 21:55, on Zulip):

nods

Yoshua Wuyts (Apr 18 2021 at 21:59, on Zulip):

I guess I'm starting with the basics for this for now -- opened a new branch and adding two methods to Impl: push_front and push_back

Yoshua Wuyts (Apr 18 2021 at 21:59, on Zulip):

just to get the hang of mutable immutable trees

Yoshua Wuyts (Apr 18 2021 at 22:00, on Zulip):

is there like, any reason why an Impl may not have an AssocItemList?

Lukas Wirth (Apr 18 2021 at 22:02, on Zulip):

To accommodate for incomplete syntax trees

Lukas Wirth (Apr 18 2021 at 22:02, on Zulip):

syntax errors and the like can cause that

Jonas Schievink [he/him] (Apr 18 2021 at 22:03, on Zulip):

I don't think there's much point in having no list vs. an empty list though

Lukas Wirth (Apr 18 2021 at 22:03, on Zulip):

if the user was to only type impl Foo we'd get an Impl node that doesnt have an AssocItemList for example

Lukas Wirth (Apr 18 2021 at 22:03, on Zulip):

an empty list has braces though, no list does not

Lukas Wirth (Apr 18 2021 at 22:04, on Zulip):

since the braces belong the AssocItemlist

Yoshua Wuyts (Apr 18 2021 at 22:04, on Zulip):

ah, gotcha!

Yoshua Wuyts (Apr 18 2021 at 22:05, on Zulip):

oh btw, rough outline still -- but if this is right I think this will turn out real nice to work with!

impl ast::Impl {
    /// Push a node to the front of the impl block.
    pub fn push_front(&self, adt: ast::Adt) {
        let l_curly = self.assoc_item_list().unwrap().l_curly_token().unwrap();
        let position = Position::after(l_curly);
        ted::insert(position, adt.syntax());
    }
}
Lukas Wirth (Apr 18 2021 at 22:06, on Zulip):

I imagine you are taking inspiration from whats already implemented for other things in edit_in_place right? Thats gonna be nice to have for sure

Yoshua Wuyts (Apr 18 2021 at 22:06, on Zulip):

heh, yeah a little bit ^^ -- like you said there's really not much

Jonas Schievink [he/him] (Apr 18 2021 at 22:07, on Zulip):

Lukas Wirth said:

since the braces belong the AssocItemlist

ah okay, then it makes sense

Yoshua Wuyts (Apr 18 2021 at 22:08, on Zulip):

hmmm, what if we had like a "heal" method for the various AST types?

Yoshua Wuyts (Apr 18 2021 at 22:08, on Zulip):

e.g. a quick in-place edit that would fill out missing curlies, tokens, etc.

Yoshua Wuyts (Apr 18 2021 at 22:08, on Zulip):

could be one way to get rid of the unwraps we have above

Yoshua Wuyts (Apr 18 2021 at 22:09, on Zulip):

(but maybe that's over-abstracting right now haha)

Lukas Wirth (Apr 18 2021 at 22:10, on Zulip):

I feel like thats too abstract ye

Yoshua Wuyts (Apr 18 2021 at 22:10, on Zulip):

haha yeahh

Lukas Wirth (Apr 18 2021 at 22:10, on Zulip):

for that unwrap up there you could maybe instead have an inplace edit function for get_or_create_assoc_item_list

Lukas Wirth (Apr 18 2021 at 22:11, on Zulip):

similar to GenericParamsOwnerEdit::get_or_create_where_clause

Lukas Wirth (Apr 18 2021 at 22:12, on Zulip):

Though matklad would certainly know better here given he has the grande vision of of this api :big_smile: especially as I havent dug into it yet myself

Lukas Wirth (Apr 18 2021 at 22:12, on Zulip):

aside from porting one small assist over

Yoshua Wuyts (Apr 18 2021 at 22:44, on Zulip):

Okay, breaking this up into a few smaller steps. First PR out: https://github.com/rust-analyzer/rust-analyzer/pull/8568

Lukas Wirth (Apr 18 2021 at 23:01, on Zulip):

I'll take a proper look tomorrow but one thing I saw is that you are pushing Adts as items into the impl. That doesn't seem right.

Lukas Wirth (Apr 18 2021 at 23:02, on Zulip):

You want to push AssocItems in there

Yoshua Wuyts (Apr 18 2021 at 23:04, on Zulip):

gotcha

Yoshua Wuyts (Apr 18 2021 at 23:07, on Zulip):

yeah, think it's bed time for me as well -- let's pick this back up again tomorrow ^^

Yoshua Wuyts (Apr 18 2021 at 23:07, on Zulip):

thanks!

Last update: Jul 29 2021 at 22:30UTC