rustup component rust-src should be used to load cross-crate sources.

@alexcrichton

If I take this arbitrary example (that supports cross-crate spans, via tcx.def_span(...)):

struct Foo;
impl Extend<()> for Foo {
    fn extend(&mut self, _: impl IntoIterator<Item = ()>) {}
}
fn main() {}

and compiling it with a local rustc build, I get this error (note the libcore snippet):
(NB: if that snippet disappears, find other tcx.def_span(...)-using diagnostics and replace the test)

error[E0643]: method `extend` has incompatible signature for trait
   --> xcrate-span.rs:3:29
    |
3   |     fn extend(&mut self, _: impl IntoIterator<Item = ()>) {}
    |                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected generic parameter, found `impl Trait`
    |
   ::: /home/eddy/Projects/rust-2/src/libcore/iter/traits.rs:355:15
    |
355 |     fn extend<T: IntoIterator<Item=A>>(&mut self, iter: T);
    |               - declaration in trait here

but if try with rustup-provided rustc, I only get this shorter error:

error[E0643]: method `extend` has incompatible signature for trait
   --> xcrate-spans.rs:3:29
    |
3   |     fn extend(&mut self, _: impl IntoIterator<Item = ()>) {}
    |                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected generic parameter, found `impl Trait`

But $(rustc --print=sysroot)/lib/rustlib/src/rust/src/libcore/iter/traits.rs does exist, because I have the rust-src component enabled (it's enabled by default, which makes it likely to exist for most users), so it should be possible in theory, to teach rustc to look up certain paths relative to lib/rustlib/src/rust inside the sysroot, if it exists.

Running this:

strings $(rustc --print=sysroot)/lib/rustlib/*/lib/libcore-*.rlib | rg 'iter/traits\.rs'

shows that /checkout/src/libcore/iter/traits.rs and libcore/iter/traits.rs both exist in the rlib, and I assume the former is the one that it tries to load - we can even test this:

# Let's live a little...
sudo ln -s $(rustc --print=sysroot)/lib/rustlib/src/rust /checkout

Trying the test again, we now get:

error[E0643]: method `extend` has incompatible signature for trait
   --> xcrate-spans.rs:3:29
    |
3   |     fn extend(&mut self, _: impl IntoIterator<Item = ()>) {}
    |                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected generic parameter, found `impl Trait`
    |
   ::: /checkout/src/libcore/iter/traits.rs:355:15
    |
355 |     fn extend<T: IntoIterator<Item=A>>(&mut self, iter: T);
    |               - declaration in trait here

So it's definitely compatible, the hash check passes and whatnot, we just need to rename /checkout to something artificial like $rust, I'm guessing.

# ... but also clean up afterwards.
sudo rm /checkout

cc @alexcrichton @rust-lang/dev-tools @rust-lang/compiler

This sounds like a great idea to me! I think we could definitely bake some logic into rustc such that it has a default src directory for the standard library where, if present, it will connect the spans together. We'd then just have rustup naturally fill the location.

We would have to be careful though, as some diagnostics (specially suggestions) use the definition span being available as a proxy for the end user being able to edit those sources. Because of this, if we implement this improvement without accounting for it we would start suggesting people to modify std, core or even third party crates' code, which we should never do.

@estebank

@estebank Right, we'd have to distinguish those more carefully. One thing I should note is that we should be making edit suggestions cross-crate within a workspace, at least IMO.

As soon as we involve other crates, I would reduce the weight of the text from a suggestion to a merely informative explanation of the failure, like we see in your example. We need to add a way to check for this in the diagnostics api. I'm not opposed at all to the idea and think it'd be useful.

@alexcrichton

@alexcrichton How does #53829 impact this? Perhaps, for paths starting with /rustc/<hash> (if the hash matches the one the compiler "remembers" as its own?) look in the sysroot for sources?

We'd have to make sure this works for both rustup and distros.

@eddyb

@eddyb I don't think it does? That just affects debuginfo

Right, but we can use it for recording the paths of files in dependencies as well, right?

Perhaps!

@michaelwoerister

cc @michaelwoerister

@alexcrichton

@alexcrichton I just checked with the latest nightly and it does have an effect:

error[E0643]: method `extend` has incompatible signature for trait
   --> xcrate-spans.rs:3:29
    |
3   |     fn extend(&mut self, _: impl IntoIterator<Item = ()>) {}
    |                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected generic parameter, found `impl Trait`
    |
   ::: /rustc/1f57e4841157d5cbd4c4e22018f93bd1801c98c2/src/libcore/iter/traits.rs:355:15
    |
355 |     fn extend<T: IntoIterator<Item=A>>(&mut self, iter: T);
    |               - declaration in trait here

I had to use (I got that hash with a hex editor from libcore-*.rlib):

sudo mkdir /rustc
sudo ln -s $(rustc --print=sysroot)/lib/rustlib/src/rust /rustc/1f57e4841157d5cbd4c4e22018f93bd1801c98c2

Maybe we can change the lib/rustlib/src/rust path inside the sysroot to either lib/rustlib/src/rustc/<hash> or src/rustc/<hash> (i.e. don't nest it in lib/rustlib)?
Or leave it unchanged and only detect /rustc/<hash>?

Either way, we seem to be set up to handle this gracefully!

I'd be fine with w/e change we need here, should be easy enough to tweak!

I think we can hardcode the same hash via an env var (like we do for other things, such as compiler version - search for CFG_) in rustc so it can check here that the path (name) starts with /rustc/<harcoded hash passed via CFG_...>/, and replace it:

rust/src/librustc_metadata/decoder.rs

Line 1243 in abe19a7

let local_version = local_source_map.new_imported_source_file(name,

Would be nice if #56860 was done at the same time as this work.

@eddyb

@eddyb I would like to take a shot at this. I'm a newbie to rust.

@saleemjaffer

Oops, I missed this notification, sorry @saleemjaffer!
Ping me on Discord, I suppose, if you're still interested.

But $(rustc --print=sysroot)/lib/rustlib/src/rust/src/libcore/iter/traits.rs does exist, because I have the rust-src component enabled (it's enabled by default, which makes it likely to exist for most users)

I don't think that's true, at least not any more.

We would have to be careful though, as some diagnostics (specially suggestions) use the definition span being available as a proxy for the end user being able to edit those sources. Because of this, if we implement this improvement without accounting for it we would start suggesting people to modify std, core or even third party crates' code, which we should never do.

AFAIK we already print span contents for third party crates other than those in the sysroot though, don't we?

@Xanewok

cc @Xanewok Something I noticed lately is that RLS appears to be able to find libstd from rust-src already, I wonder if we could reuse that logic.

Navigation Menu

Repository navigation

rustup component rust-src should be used to load cross-crate sources. #53486

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

rustup component rust-src should be used to load cross-crate sources. #53486

Description

Activity

alexcrichton commented on Aug 20, 2018

estebank commented on Aug 20, 2018

eddyb commented on Aug 20, 2018

estebank commented on Aug 20, 2018

eddyb commented on Sep 26, 2018

alexcrichton commented on Sep 26, 2018

eddyb commented on Sep 28, 2018

alexcrichton commented on Sep 28, 2018

eddyb commented on Sep 28, 2018

eddyb commented on Nov 24, 2018

alexcrichton commented on Nov 25, 2018

eddyb commented on Nov 25, 2018

infinity0 commented on Dec 15, 2018

saleemjaffer commented on Feb 14, 2019

eddyb commented on May 5, 2019

RalfJung commented on Nov 27, 2019

eddyb commented on Nov 27, 2019

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions