rustc: remove unnecessary extern_prelude logic from ty::item_path.

[eddyb]

The checks added in 02357e4 effectively turned crate::std into std, but they were too general (affecting any crate::foo where foo was in the extern prelude, not just extern crates), and unnecessary, as only the extern crates created by "std injection" need any special-casing.

I've replaced the check for an extern prelude name with not printing the full path to an extern crate if the span inside the middle::cstore::ExternCrate is a dummy one.

Since this only affects the user-facing "relative" mode, it shouldn't have interactions with linking, and the only observable effect should be sometimes-shorter paths in diagnostics.

r? @nikomatsakis cc @davidtwco

[nikomatsakis]

Do we want some sort of test?

r=me with the comments + (ideally) a test

[nikomatsakis]

I think a comment would be good here -- it's pretty non-obvious what's going on.

[nikomatsakis]

Similarly, comment here -- or maybe we can pull this into some kind of common helper function with the code above?

[eddyb]

So in a different branch I was able to deduplicate this by moving the local mode logic from push_krate_path to try_push_visible_item_path and relying on the fact that try_push_visible_item_path can override the behavior of push_item_path. But maybe we can avoid the dummy span check here by some other means.

[nikomatsakis]

@eddyb

I guess that this means that if you have an extern crate and something in the prelude, you will get .. what?

[eddyb]

@nikomatsakis You will get whatever is preferred by the rest of the system, likely the extern crate (with a path like crate::foo) - maybe we can fix all of this separately by making extern crate items unpreferred when they're in the extern prelude?

cc @petrochenkov

[petrochenkov]

@eddyb
Values in the GlobalCtxt::extern_prelude map are bool flags telling whether the crate was introduced by an extern crate item or not.
(Not accidentally introduced, like passed with both --extern and extern crate foo;, but really introduced only with extern crate foo;.)

I'm not sure what exactly ty::item_path needs, but hope that helps.

[eddyb]

@petrochenkov That's perhaps useful, but what's needed here is something tied to the CrateNum, not (just) the name of the crate, i.e. what this method records:

rust/src/librustc_metadata/creader.rs

Line 382 in 4c0116e

fn update_extern_crate(&mut self,

And its calls in that module, e.g.:

rust/src/librustc_metadata/creader.rs

Lines 1054 to 1064 in 4c0116e

	self.update_extern_crate(
	cnum,
	ExternCrate {
	src: ExternCrateSource::Path,
	span,
	// to have the least priority in `update_extern_crate`
	path_len: usize::max_value(),
	direct: true,
	},
	&mut FxHashSet::default(),
	);

Right now we track what caused a crate to be loaded, which used to always be an extern crate item, but I think with Rust 2018, that's the wrong perspective.

We should instead record whether the crate can be referred to by name (and specifically by which name), or that we have to spell out the path to an extern crate that loaded the crate.

ExternCrateSource appears to be used only in ty::item_path, so we don't have to consider other usecases if we change how this "simplest way to refer to a crate" tracking system works.

(random aside, ExternCrateSource::Use is dead, we should remove it, I might do it later)

[petrochenkov]

something tied to the CrateNum, not (just) the name of the crate

Then ExternCrate/ExternCrateSource tied to the CrateNum probably need to keep a flag similar to introduced_by_item in ExternPreludeEntry tied to the name.

[nikomatsakis]

@eddyb @petrochenkov -- I can't quite tell what's the status of this PR. I wrote r=me above, modulo testing. Do we think we want to alter the behavior in this case, though?

I guess that this means that if you have an extern crate and something in the prelude, you will get .. what?

[petrochenkov]

Relevant issue - #55779 (infinite recursion in fn push_item_path).

While printing the trait helper::Trait the inner extern crate helper item is (incorrectly) considered "the introducer" of helper crate (ExternCrateSource::Extern), and to print the introducer we need to print its parent - the trait impl including helper::Trait, and so on.

[nikomatsakis]

I'm just going to change this to r? @petrochenkov =)

[stokhos]

Ping from triage @petrochenkov : This PR requires your review.

[petrochenkov]

This PR requires eddyb's return.

[TimNN]

Ping from triage @eddyb: What is the status of this PR?

[petrochenkov]

This seems to be included into #57967

[eddyb]

This seems to be included into #57967

Right, I made this PR while starting work on the refactoring #57967 is based on.

IIRC, @nikomatsakis' comments are addressed in that, by moving the "relative extern crate path" logic into the "visible path" part of path printing (since they're somewhat related).

Do you still want to do #56655 (comment), before landing #57967, or can it wait?

[petrochenkov]

@eddyb

Do you still want to do #56655 (comment), before landing #57967, or can it wait?

Whatever is more convenient.

[bors]

☔ The latest upstream changes (presumably #58151) made this pull request unmergeable. Please resolve the merge conflicts.

[Dylan-DPC-zz]

ping from triage @eddyb any updates on this?

[Dylan-DPC-zz]

ping from triage @eddyb
Unfortunately we haven't heard from you on this in a while, so I'm closing the PR to keep things tidy. Don't worry though, if you'll have time again in the future please reopen this PR, we'll be happy to review it again!

[crlf0710]

Needs a rebase here.

[Dylan-DPC-zz]

Closing this due to inactivity after a discussion with the author.