Remap paths from other crates

[jyn514]

Previously, remap-path-prefix only applied to files loaded in the current session. File names
loaded in a previous invocation were used unchanged. This is unfortunate for UI testing when some of
the crates involved are in the standard library (we don't want to recompile the standard library for
arbitrary tests).

Change it to apply to existing files as well. I believe this will eventually allow removing the
special-casing for -Z simulate-remapped-rust-src-base. I'm willing to make that change here if desired.

Fixes #66251. This might allow me to unblock #117609 without first waiting for #113611 to land (need to test).

[rustbot]

r? @petrochenkov

(rustbot has picked a reviewer for you, use r? to override)

[jyn514]

@Nemo157 suggested that remapping should be cumulative, instead of giving precedence to the first flag. In other words, if a file is at path /foo/lib.rs, compiled with remap-path-prefix=/foo=/bar, and then used as a library in an invocation with remap-path-prefix=/bar=/baz, the final path used should be /baz/lib.rs, not /bar/lib.rs which the PR does currently. I'm willing to make that change but I'd like to hear from petrochenkov what he prefers.

[Urgau]

How do this interact with -Zremap-path-scope? Could you also add a test with -Zremap-path-scope=diagnostics.

[jyn514]

@Urgau it should work fine, remap-path-scope works by taking an existing RealFileName::Remapped and choosing whether to use FileNameDisplayPreference::Remapped or FileNameDisplayPreference::Local, it has to handle it by its nature. I'll add a test.

[petrochenkov]

Is there anybody familiar with this topic besides michaelwoerister, who is unavailable?
cc @cbeuw maybe

I only remember that path remapping has seen numerous changes and fixes in rustc and multiple RFCs, and it was never enough.

[petrochenkov]

The changes look reasonable to me, but I'm not sure what are the full implications.

I'm going to ping @rust-lang/compiler, then wait for a week, then r+.
@rustbot team

[cbeuw]

@Nemo157 suggested that remapping should be cumulative

This has been brought up before:

Currently it does not look to me like we need remapping of imported paths (i.e. "double remapping") to fix #73167. The compiler has the stable name of upstream file names available. I would opt for trying to fix the existing bugs without introducing new functionality (which would probably need a major change proposal).
#83813 (comment)

I don't have an opinion either way, but if we were to introduce it, it should be a deliberate change through MCP. It shouldn't be introduced in this PR.

[michaelwoerister]

I think this came up a few times before and I vaguely remember it being more complicated than one would think. I'll add myself as a reviewer and will try to remember what the complications were.

[michaelwoerister]

As far as I can tell, the main reason the remapping is only applied to the current crate is that remapped paths are emitted into compiled artifacts (e.g. panic messages in object files, debuginfo, etc) and subsequent compilation sessions cannot modify those anymore. The effect being that one gets an unpredictable mix of paths in the final output.

For example, if there's a crate A containing an #[inline] function foo() that is called inside A, but also in crates B and C, each of which has a different path remapping, then a panicking call to foo() would point to different file paths, depending on which of the crates foo() was called in.

From my point of view that is a blocker for recursive remapping. Making Cargo do the heavy lifting as described in the trim paths RFC seems like the better approach to me.

[Dylan-DPC]

@jyn514 any updates on this? thanks

[jyn514]

nope