End game for find usages

[matklad]

Zulip thread: https://rust-lang.zulipchat.com/#narrow/stream/185405-t-compiler.2Fwg-rls-2.2E0/topic/End.20Game.20for.20Find.20Usages

Consider this code:

fn foo() {}

macro_rules! m {
    () => {
        $crate::foo()
    };
}

mod inner {
    fn f() {
        m!();
    }
}

Does the m!() call constitute a usage of the foo function? Under our current model, the answer is no: a usage requires a literal foo present in the source code. This is because find usages works by pruning a super-set of textual occurrences (https://rust-analyzer.github.io/blog/2019/11/13/find-usages.html), and there's no textual occurrence to prune here!

There are two ways we can deal with it, which lead to dramatically different architectures.

First approach is to stick with our current model, and just implement heuristics that would flag a potential usage in the macro definition. Note that no heuristic would work for procedural macros.

Second approach is to always eagerly expand all macros for find usage. This is still much cheaper than typecheking everything, but is way costlier than our current text-based pruning.

If we go with the second approach, that has implications for the overall architecture. For example, if we are going to expand all the macros anyway, we might want to dump expansions to disk and treat them as inputs.

[matklad]

@steffahn provides an interesting macro-less example of this on irlo:

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=ca6f39d9aac999a492bd07b03781c983

trait Foo {
    type Ty;
}

impl Foo for [(); 42] {
    type Ty = Bar;
}

struct Bar {
    x: u32,
}

fn main() {
    let x = 1;
    type T = <[(); 40+2] as Foo>::Ty;
    let t = T { x }; // <- this is usage of Bar!
    t.x;
}

It makes me think that our hope for a correct fast-path for find usages which doesn't rely on type inference is not really feasible :(

[matklad]

Hm, as pointed out by @vlad20012 this might actually be not that bad -- all the interesting stuff happens on the item level. That is, the example is

type T = <[(); 40+2] as Foo>::Ty;

fn main() {
    let x = 1;
    let t = T { x }; // <- this is usage of Bar!
    t.x;
}

which actually isn't that much different from

type T = Bar;

fn main() {
    let x = 1;
    let t = T { x }; // <- this is usage of Bar!
    t.x;
}

That is, searches already have to close over type aliases.

[bjorn3]

I did say that for the original example the usage of foo would be $crate::foo() inside the macro and not the m!() macro expansion. For that to work I think you would need to look everywhere the identifier foo is used and if it is inside a macro definition, search for all expansion sites of the macro and check in the resolutions of the expansion if foo refers to the function in question. Something like that would be necessary for go to definition inside macro definitions too I think.

[flodiebold]

Well, arguably, the type T = Bar is the usage of Bar, isn't it? E.g. for renaming purposes there's no use in closing over type aliases. Ideally I guess the user would get a dialog where they can choose to follow type aliases as far as they wish.

(is there a usage of Bar here? If not, what's the use of making T { x } a usage? Would any reference to T in be a usage?)

fn foo<T: Foo>(t: T) {
    let o: Option<T::Ty> = None;
}

fn main() {
    foo::<[(); 42]>();
}

[steffahn]

For renaming-ofBar purposes, the relevant usage would only by in the type synonym definition (e.g. the type Ty = Bar; in the trait impl). But other refactorings, for example renaming the fields, you would need to find the T { x } expression.

[flodiebold]

For renaming the fields, we search for textual occurences of the field name (and then check that the record expression has the right type), not the struct. That's why e.g. renaming field in the following example works right now:

struct Struct {
    field: u32,
}

macro_rules! construct {
    ($i:ident) => { Struct { $i: 0 } }
}

fn test2() {
    let x = construct!(field);
}

[steffahn]

@flodiebold I don’t quite understand what you’re trying to argue; I was talking about code in this comment, which doesn’t feature macros. And in that code example if I ask rust-analzer (IDK what version I have, probably current stable or something like that…) to rename the field x (in the struct Bar definition), it does not find the T { x } nor the subsequent t.x.

If you simplify the 40 + 2 to 42, then the t.x is renamed, but the T { x } still isn’t. Only simplifying type T = <[(); 42] as Foo>::Ty; further to type T = Bar; appears to do the trick for me.

[flodiebold]

Neither of those problems are related to or fixed by doing type inference when searching for usages; the first is caused by limitations in our const eval, and the second one a bug in our type inference (it seems we don't resolve projections for record literals? there might be a FIXME for that somewhere). If you change T to a simple type alias instead of a projection, everything is renamed as expected. My example was just an easy way to demonstrate that we don't rely on finding usages of the struct to rename fields. I'm questioning whether the concept of "find usages" needs to include seeing through type aliases.

[steffahn]

My example was just an easy way to demonstrate that we don't rely on finding usages of the struct to rename fields. I'm questioning whether the concept of "find usages" needs to include seeing through type aliases.

Thanks for clarifying, this makes complete sense to me.

[steffahn]

One case that was brougth up in the IRLO thread was refactoring between tuple structs or record structs, and that syntactical searching is faster than type-inference-based search. Arguably, such a refactor will need to touch all kinds of field expressions, too, anyways; so especially for tuple-struct-to-record struct all foo.0 expressions are, syntactically, candidates to needing to be changed.

To relate this to the example, do something like

trait Foo {
    type Ty;
}

impl Foo for [(); 42] {
    type Ty = Bar;
}

struct Bar(u32);

fn main() {
    let x = 1;
    type T = <[(); 40 + 2] as Foo>::Ty;
    let t = T{ 0: x };
    t.0;
}

and then “Convert to named struct” on Bar. This will only properly work if simplified to type T = Bar;; there appears to be the same kind of general behavior that using 42 instead of 40 + 2 makes it at least find the t.0, but still not the T{ 0: x }.

There remains the question of whether or not this refactor logically operates by “finding a usage of Bar”, or it’s actually just a “find the usage of all Bar-fields in order to rename them from 0, 1, 2… to field1, field2, field3…”. I guess the answer to this is just an arbitrary choice of what the convention for terminology should be, and one that ultimately doesn’t matter in any practical way, is it considered a “usage of Bar” or is it not?

There doesn’t appear to be a refactor to turn a record struct back into a tuple struct, so I can’t comment on

[flodiebold]

Yeah, in the end it doesn't matter much for assists like "Convert to named struct", because as you've noted we have to find all instances of x.0 where x is of the struct type, so there was no way we would get around type inference anyway. (From everything we've seen so far, we can still restrict it to all functions that make some textual mention of one of the fields we're looking for though.) So in conclusion,

• different features need different searches;
• renaming almost surely can use text-based search, since if there's no textual mention of the item it can't be renamed anyway;
• some other features need type inference, but we knew that already;
• and for the actual "Find references" feature, this is a UI question -- do we want to include usages through type aliases / macros; can we display them in some nice way / let the user control it?

The only case I can currently come up with where we might be forced to run inference on everything is if an assist needs to find occurrences of Struct { ...some_other_instance }, though I don't know what such an assist might be. Although even then we could still look for occurrences of that pattern, which should be very rare.