Stream: t-compiler/rust-analyzer

Topic: `unresolved import` for some `#[path = ".."]`


Thom Chiovoloni (Mar 03 2021 at 09:06, on Zulip):

This bug (https://github.com/rust-analyzer/rust-analyzer/issues/3898) has been a thorn in my side for a long time. It's marked as actionable, is there any pointers anybody can give me a pointer for the right way to go about it?

Totally unfamiliar with the r-a codebase, but willing to bang my head against the wall a little if I can get some pointers on where I should start.

Laurențiu (Mar 03 2021 at 09:14, on Zulip):

Don't read too much into my "actionable" labels :sweat_smile:

Thom Chiovoloni (Mar 03 2021 at 09:21, on Zulip):

Ah, that was a fear of mine...

Laurențiu (Mar 03 2021 at 09:24, on Zulip):

I think this happens around crates/project_model

Thom Chiovoloni (Mar 03 2021 at 09:24, on Zulip):

(If the "right" fix is hard, is there anything we can do to fix this in just the stdlib's (e.g. core::arch's) case?

It's not that common to use #[path], and usually there are alternatives (see: libc crate moving away from it). unfortunately, for this #[path] use case, it seems pretty tricky to fix in the stdlib, and it makes {core,std}::arch very painful in r-a.)

Thom Chiovoloni (Mar 03 2021 at 09:24, on Zulip):

yeah, thats where i was looking.

Thom Chiovoloni (Mar 03 2021 at 09:36, on Zulip):

Hmm, project model seems more concerned with building a model of a project, and from the description of the issue, it sounds like the model it builds is probably fine — the issue is that the check is done against the package and not the workspace (at least, that's what someone in the issue said, and it makes sense to me).

AFAICT project_model doesn't have much to do with elements in rust code (like #[path]) so much as project configuration, and given the above I don't know that there's a reason to believe this is wrong

Thom Chiovoloni (Mar 03 2021 at 09:36, on Zulip):

That or I'm completely lost — if you're confident it's in there I can keep looking. I don't understand what much of it does in any detail, just what it seems to do based on function names and what bits and pieces I can follow of the internals do

Thom Chiovoloni (Mar 03 2021 at 09:39, on Zulip):

ah, https://github.com/rust-analyzer/rust-analyzer/blob/2b22fc929a5f13d6cddb9458b1b205f9aee60299/crates/vfs/src/anchored_path.rs seems promising...

Thom Chiovoloni (Mar 03 2021 at 09:50, on Zulip):

My current theory is: this code https://github.com/rust-analyzer/rust-analyzer/blob/2b22fc929a5f13d6cddb9458b1b205f9aee60299/crates/base_db/src/lib.rs#L153-L158 (or, maybe something it delegates to) is referring to the source root of the package (is that what SourceRoot is? of the package?)

Hrm. That's a bit tricky if so, since it probably means I was wrong about my thought that project_model seemed out of place... Or, this change might be hard, if it goes against the way a lot of stuff is set up (which seems plausible at the moment)

Thom Chiovoloni (Mar 03 2021 at 09:52, on Zulip):

Actually, hm, is there even a notion of a workspace of a dependency (that isn't part of the current workspace)? (e.g. in this case std)

Florian Diebold (Mar 03 2021 at 09:58, on Zulip):

the actual analysis code is agnostic of cargo, so it doesn't know about workspaces, just about the crate graph, and about the set of source roots. source roots are considered independent, and I don't think imports across them will ever be possible; hence why this probably needs to be solved by changing how source roots are set up by project_model

Florian Diebold (Mar 03 2021 at 09:59, on Zulip):

a source root is just basically a root directory containing source files. every source file has to be in some source root, and we don't know anything about the absolute path of the source root, so relative paths can't reach across source roots. IIRC there aren't any other requirements for source roots

Thom Chiovoloni (Mar 03 2021 at 10:01, on Zulip):

Hm, yeah, I see. That makes sense and is a bit unfortunate (since it makes this change harder). Hrm.

Thom Chiovoloni (Mar 03 2021 at 10:08, on Zulip):

so, i don't know if i see a way of easily solving this in the general case, but solving it in the stdlib case seems doable since we have information about them as part of the sysroot. does that sound plausibly correct?

Thom Chiovoloni (Mar 03 2021 at 10:08, on Zulip):

it'd be kind of hacky though, probably :(

Thom Chiovoloni (Mar 03 2021 at 10:40, on Zulip):

this is kind of odd.

in https://github.com/rust-analyzer/rust-analyzer/blob/2b22fc929a5f13d6cddb9458b1b205f9aee60299/crates/project_model/src/sysroot.rs#L83-L90 STD_DEPS (including stdarch) is added to libstd's set of, well, deps. Thing is, that crate doesn't exist. stdarch is a workspace there, and it doesnt even contain a crate named stdarch. So I'm not sure what adding it as a dep does (if anything?) (EDIT: I see, stdarch will fail the check for {}/src/lib.rs/lib{}/lib.rs`, so it won't get added...)

that said, this must more or less somehow work, since std::is_x86_feature_detected! works and r-a doesn't get upset about it. Additionally, libstd does the same thing to import it: https://github.com/rust-lang/rust/blob/master/library/std/src/lib.rs#L538, e.g. a path outside of its package and into one of the stdarch crates (I actually would have expected #[macro_use] to be required on that mod but, huh. guess not... weird)

Thom Chiovoloni (Mar 03 2021 at 10:41, on Zulip):

... unfortunately it seems like this may just work for macros, and not for other items...

Edwin Cheng (Mar 03 2021 at 13:01, on Zulip):

I am not familiar that part of code, but is it possible to add a new settings : "rust-analyzer.files.includDirs" for explict include those paths ?

Thom Chiovoloni (Mar 03 2021 at 15:26, on Zulip):

hm, I'm not sure. I'm not sure what I'd set it to. These paths are part of the standard library, so the path to include would be dependent on your sysroot. Also, given that it's part of the stdlib that has this issue, it would be nice to handle this for all users, and not just ones that set some config.

Thom Chiovoloni (Mar 03 2021 at 15:27, on Zulip):

i also think the issue isnt so much determining the path of the file in r-a, but ensuring the right part of the code knows about it.

Thom Chiovoloni (Mar 03 2021 at 15:30, on Zulip):

the file in question is something like $sysroot/lib/rustlibs/library/stdarch/crates/core_arch/src/mod.rs, and it's included (via #[path]) from $sysroot/lib/rustlibs/library/core/src/lib.rs, or something.

(on my phone, so don't quote me about either of those paths exactly, its something like that though)

Thom Chiovoloni (Mar 03 2021 at 15:31, on Zulip):

but as you can see its easy to derive the correct path, just hard to get it where it's needed, which is possibly something like https://github.com/rust-analyzer/rust-analyzer/blob/2b22fc929a5f13d6cddb9458b1b205f9aee60299/crates/base_db/src/lib.rs#L153-L158 (not 100% certain, tbh)

Thom Chiovoloni (Mar 03 2021 at 15:33, on Zulip):

(your suggestion would possibly help for the non-libcore case, e.g. where it's my own module that is doing this weird including, but honestly i don't really care about that case that much, and IDK that it would help for the libcore case)

Thom Chiovoloni (Mar 03 2021 at 15:35, on Zulip):

all that said i'm not super familiar with any of this... so i could be totally off base here. maybe i'm missing something.

Florian Diebold (Mar 03 2021 at 15:38, on Zulip):

I'm assuming the problem is that we don't put all of the sysroot into the same source root, so the question would be why we don't

Thom Chiovoloni (Mar 03 2021 at 15:41, on Zulip):

oh. yeah, hm, that sounds like it would do the trick... i couldn't figure out where the sysroot stuff got translated into SourceRoots though, but ill take another look in a bit.

Last update: Jul 29 2021 at 08:45UTC