Stream: t-compiler/wg-rls-2.0

Topic: ItemTree


Jonas Schievink (Jun 16 2020 at 12:00, on Zulip):

I've resurrected the stubs/item tree work and moved the def collector over to it (removing raw items completely). Now I'm looking into replacing queries such as generic_params and impl_data, but they're keyed off of GenericDefId and ImplId, which are just interned IDs that return an AST node, and I can't query the item tree with that.

Not sure about the best way forward here, but I'm feeling like the additional queries should be removed entirely and all callers be moved over to use the item tree directly. Does that make sense?

matklad (Jun 16 2020 at 12:19, on Zulip):

Yup!

matklad (Jun 16 2020 at 12:20, on Zulip):

That is, the ID I think should contain (id of parent, index in the item tree). Well, super long-term I imagine we might want (id of parent, name of child, diambiguator), but we can get away without it for the time being

Jonas Schievink (Jun 16 2020 at 12:23, on Zulip):

Do you mean the parent item or file? FileId would be easier for now

matklad (Jun 16 2020 at 12:26, on Zulip):

Ideally, it should be parent item I think

matklad (Jun 16 2020 at 12:26, on Zulip):

We already track this actually

matklad (Jun 16 2020 at 12:26, on Zulip):
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct ItemLoc<N: AstNode> {
    pub container: ContainerId, // <- this is the parent
    pub ast_id: AstId<N>,
}
matklad (Jun 16 2020 at 12:27, on Zulip):

So, we should replace AstId with ItemTreeId. Note that that'll require some significant changes to get source of the thing infrastructure

matklad (Jun 16 2020 at 12:37, on Zulip):

@Jonas Schievink it might make sense to take a look at the source_to_def.rs file

matklad (Jun 16 2020 at 12:37, on Zulip):

It contains a bit of IDE specific magic which affects how we setup ids and sources

matklad (Jun 16 2020 at 12:37, on Zulip):

Curiously, this is exactly the same pattern that is used in Roslyn and Kotlin (and I really should blog about it)...

matklad (Jun 16 2020 at 12:39, on Zulip):

The core idea is that, if you have an ast::Thing, and want a hir::Thing, you look at the ast parent, recursively get hir::ParentThing, and then look through hir parent's children to find the child whose source is ast::Thing. That is, in a sense a lookup is reversed.

Jonas Schievink (Jun 17 2020 at 12:52, on Zulip):

I did not realize this would be such a huge undertaking :sweat_smile:

matklad (Jun 17 2020 at 12:53, on Zulip):

Well, you are about half-way in, so at this point it's easier just to finish the thing :)

matklad (Jun 17 2020 at 12:53, on Zulip):

But yeah, I feel like we've lost a significant amount of experemination freedom we had initially, when such refactorings were relatively straightforward...

Jonas Schievink (Jun 17 2020 at 12:53, on Zulip):

Any good idea for how to handle/represent inner items in the item tree? Since they might be nested in inner blocks, just having a Vec<ModItem> in Function might not cut it.

Jonas Schievink (Jun 17 2020 at 12:54, on Zulip):

(Body lowering and the item data queries create item IDs from AST nodes, which now doesn't work anymore, so I have to fix that)

Jonas Schievink (Jun 17 2020 at 12:56, on Zulip):

Hmm, since the item data queries return fully expanded data, how do we map that to the item tree? Maybe we need to keep the queries and have them still perform the expansion?

matklad (Jun 17 2020 at 12:56, on Zulip):

Those are good questions....

matklad (Jun 17 2020 at 12:58, on Zulip):

For inner items, I see two approaches:

matklad (Jun 17 2020 at 12:59, on Zulip):

Since they might be nested in inner blocks, just having a Vec<ModItem> in Function might not cut it.

We might still attach all items to the parent item, or we can have an ID of the block. Not sure which one is better

Florian Diebold (Jun 17 2020 at 13:03, on Zulip):

we certainly should still do expansion of item bodies lazily, right?

matklad (Jun 17 2020 at 13:05, on Zulip):

yup, expansion of bodies (and headers, as in const C: ty_macro!() = 92;) should be done lazily I would thing

Jonas Schievink (Jun 17 2020 at 13:06, on Zulip):

Won't we end up doing expansion and parsing of bodies eagerly anyways to collect all impls that could apply to a type?

matklad (Jun 17 2020 at 13:07, on Zulip):

@Jonas Schievink today, we indeed have to do eager expansion, because impls leak beyond function bodies

matklad (Jun 17 2020 at 13:08, on Zulip):

But we plan to deprecate this (to make IDE go faster), so we should try to lazy-ify this. Specifically, the tentative long-term plan is that item body is sort-of like a downstream crate with respect to visibility of things

Jonas Schievink (Jun 17 2020 at 13:08, on Zulip):

Yeah, makes sense to prepare for that I suppose

matklad (Jun 17 2020 at 13:13, on Zulip):

Not as much prepare, as to prove that the optimization makes sense (but it seems like it does)

Florian Diebold (Jun 17 2020 at 13:16, on Zulip):

TBH I think we should act like impls inside function bodies can't affect types outside that body, with the deprecation just making it official that this won't be supported (admittedly it's theoretical currently anyway, since we don't support any kind of impl in function bodies...)

Florian Diebold (Jun 17 2020 at 13:16, on Zulip):

or in other words, it seems a waste of time to plan to implement support for it if we want to deprecate it anyway

Jonas Schievink (Jun 17 2020 at 13:19, on Zulip):

matklad said:

yup, expansion of bodies (and headers, as in const C: ty_macro!() = 92;) should be done lazily I would thing

How this is currently handled? ast::TypeRef does not have a macro variant

matklad (Jun 17 2020 at 13:20, on Zulip):

@Jonas Schievink by not doing macro expansion in this postions at all :D

Florian Diebold (Jun 17 2020 at 13:20, on Zulip):

... I think it just isn't? (... too late)

Jonas Schievink (Jun 17 2020 at 13:20, on Zulip):

Works for me :D

Florian Diebold (Jun 17 2020 at 13:23, on Zulip):

I am kind of working on refactoring type lowering (introducing another intermediate representation), which kind of touches on that -- I guess macro expansion would happen during lowering to that new IR

Jonas Schievink (Jun 17 2020 at 14:16, on Zulip):

What happens when expanding a macro call inside an impl or trait? Does r-a support that currently? How does the resulting AST look? The ItemTree assumes that its given a module, but that wouldn't be true here, right?

matklad (Jun 17 2020 at 14:17, on Zulip):

@Jonas Schievink see https://github.com/rust-analyzer/rust-analyzer/blob/master/crates/ra_hir_def/src/data.rs#L168-L181

matklad (Jun 17 2020 at 14:18, on Zulip):

We collect inner items from sources, bypassing raw items

Jonas Schievink (Jun 17 2020 at 14:19, on Zulip):

So we get MacroItems, which is a ModuleItemsOwner, even though we're only looking for associated items, hmm

Jonas Schievink (Jun 17 2020 at 16:04, on Zulip):

Okay, figured out the item data queries. Now on to inner items.

Jonas Schievink (Jun 17 2020 at 16:04, on Zulip):

Wow, literally anything can have inner items.

Jonas Schievink (Jun 17 2020 at 16:05, on Zulip):
enum En {
    Variant = { fn f() {} 0 },
}
matklad (Jun 17 2020 at 16:06, on Zulip):
const _: [(); {fn lol() {} 0}] = [];
matklad (Jun 17 2020 at 16:06, on Zulip):

but yes, your proc_macro_hack version is better :D

matklad (Jun 17 2020 at 16:07, on Zulip):

In general, for this reason I am somewhat unhappy that we have a very strict split between expression and item worlds...

Jonas Schievink (Jun 17 2020 at 16:45, on Zulip):

Hmm, seems somewhat problematic to make ItemTree the sole source of items if it also wants to process item bodies lazily. That means that inner items won't have an ID in the tree, but we need one when lowering the body.

Jonas Schievink (Jun 17 2020 at 16:54, on Zulip):

Ah, well, the item tree can eagerly process them as long as it doesn't expand macros

Florian Diebold (Jun 17 2020 at 16:58, on Zulip):

the way I would imagine it to work is that item bodies get their own item trees, and item IDs are basically concatenated paths through this 'tree of trees' (i.e. a root item ID is simply the index in the root tree, but a nested item ID would be the outer item's ID + the index in that tree)

Florian Diebold (Jun 17 2020 at 16:58, on Zulip):

but I'm not really up to date on the whole thing :shrug:

matklad (Jun 17 2020 at 19:59, on Zulip):

Yup, I imagine this setup as well

Jonas Schievink (Jun 17 2020 at 20:00, on Zulip):

Okay, that's quite different from what I have

matklad (Jun 17 2020 at 20:02, on Zulip):

Ideally, we should be able to swtich impl strategies relatively painless as well...

matklad (Jun 17 2020 at 20:02, on Zulip):

But I don't know how to achieve that

matklad (Jun 17 2020 at 20:03, on Zulip):

Well, at least, with the hir layer, one can be relatively sure that no changes to IDE will be required

Jonas Schievink (Jun 23 2020 at 11:25, on Zulip):

175mb ItemTreeQuery

Pretty heavy

Last update: Sep 27 2020 at 14:00UTC