Stream: t-compiler/rust-analyzer

Topic: name::name::name::name::name


Jonas Schievink [he/him] (Mar 18 2021 at 14:10, on Zulip):

wut screenshot-2021-03-18-150932.png

Florian Diebold (Mar 18 2021 at 14:20, on Zulip):

why do we get different results for Import and Qualify?

Jonas Schievink [he/him] (Mar 18 2021 at 14:23, on Zulip):

good question

Florian Diebold (Mar 18 2021 at 14:25, on Zulip):

btw, imo "Qualify as cast" is misworded? Yeah it's using as, but <T as Trait> is not really a cast

Jonas Schievink [he/him] (Mar 18 2021 at 14:25, on Zulip):

the long path has 15 segments, which is exactly MAX_PATH_LEN

Jonas Schievink [he/him] (Mar 18 2021 at 14:25, on Zulip):

Oh it doesn't actually use as

Jonas Schievink [he/him] (Mar 18 2021 at 14:25, on Zulip):

It produces name::AsName::as_name(...)

Florian Diebold (Mar 18 2021 at 14:26, on Zulip):

hm yeah right it only uses as if you were calling T::as_name before

Florian Diebold (Mar 18 2021 at 14:26, on Zulip):

anyway, that's just a side point

Kirill Bulatov (Mar 18 2021 at 14:27, on Zulip):

How do you even manage to break it? :sweat_smile:

image.png

matklad (Mar 18 2021 at 14:27, on Zulip):

I feel like rust-analyzer would've been much less successful hadn't we switched to 16x9 displays :rofl:

Florian Diebold (Mar 18 2021 at 14:28, on Zulip):

hm, that one _also_ has different results for Import and Qualify

Florian Diebold (Mar 18 2021 at 14:29, on Zulip):

of course import and qualify can potentially be different, if there are local imports

Kirill Bulatov (Mar 18 2021 at 14:29, on Zulip):

But yeah, the label mismatch is very awkward: we do use the same field to fill the label but even on the "working" example above I see differences.

And since it's the same path to look for imports, it's very odd, as if the index had started to give some non-determined behaviour.
Can be that FxHashMap I've used recently?

Kirill Bulatov (Mar 18 2021 at 14:30, on Zulip):

I have to really take a vacation and make some normal proper work in this field, sorry.

Florian Diebold (Mar 18 2021 at 14:30, on Zulip):

so maybe it's related to local def maps? but the qualify ones are actually more correct...

Lukas Wirth (Mar 18 2021 at 14:32, on Zulip):

Those two assist don't use exactly the same stuff for lookups

Lukas Wirth (Mar 18 2021 at 14:32, on Zulip):

qualify_path does
let proposed_imports = import_assets.search_for_relative_paths(&ctx.sema);
auto_import does
let proposed_imports = import_assets.search_for_imports(&ctx.sema, ctx.config.insert_use.prefix_kind);

Lukas Wirth (Mar 18 2021 at 14:34, on Zulip):

difference is basically just the prefix so it makes sense that the two return different paths there

Lukas Wirth (Mar 18 2021 at 14:37, on Zulip):

If I remember correctly my reasoning for making qualify use relative paths was that the user usually doesn't want fully qualified paths in the middle of their expressions

Lukas Wirth (Mar 18 2021 at 14:37, on Zulip):

I also agree the label could be improved

Jonas Schievink [he/him] (Mar 18 2021 at 14:40, on Zulip):

Florian Diebold said:

so maybe it's related to local def maps? but the qualify ones are actually more correct...

oh dang it you're right

Jonas Schievink [he/him] (Mar 18 2021 at 14:40, on Zulip):

You can reproduce this by going to builtin_derive.rs and removing the AsName import in the tests module, then trigger autoimport on the as_name() calls below

Jonas Schievink [he/him] (Mar 18 2021 at 14:45, on Zulip):

Ah, the name module has a reexport of itself: pub use crate::name;

That must be confusing it

Kirill Bulatov (Mar 18 2021 at 14:46, on Zulip):

In the end, the mod path lookup boils down to https://github.com/rust-analyzer/rust-analyzer/blob/master/crates/hir_def/src/find_path.rs#L94

Kirill Bulatov (Mar 18 2021 at 14:49, on Zulip):

On the ide level, we find all items matching by our search criteria and then for all of them call a higher-level ide method, where specify a few parameters such as prefixes and the module we want this path to be resolved in.
The ide method tosses this around and eventually passes that to the method above, with no significant changes afaik.

Daniel Mcnab (Mar 18 2021 at 16:57, on Zulip):

Ah, the name module has a reexport of itself: pub use crate::name;

The pumping lemma for Rust paths

Jonas Schievink [he/him] (Mar 18 2021 at 17:43, on Zulip):

Hmm, looks like it happens outside of block expressions too: screenshot-2021-03-18-184314.png

Kirill Bulatov (Mar 18 2021 at 22:29, on Zulip):

What does block mean in ModuleId?

I've looked at what find_local_import_locations function returns and for this case, it returns two ModuleIds, one having a blockId, and the other not.
Module id is defined so

#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub struct ModuleId {
    krate: CrateId,
    block: Option<BlockId>,
    pub local_id: LocalModuleId,
}

that due to the blockId difference, these two modules are unequal and there are no existing means to compare ModuleIds differently.

Another weird issue is that I cannot reproduce the bug in all our codebase, including the tests in auto_import.rs, that's all odd.

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

block is Some if the ModuleId refers to a block expression

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

I'll add some docs

Last update: Jul 28 2021 at 04:15UTC