--remap-path-prefix does not apply to secondary files in diagnostics

[jebrosen]

Since #64151 added some new diagnostics, Rocket's UI tests have had mismatches in stderr that I can't seem to fix. In particular these diagnostics include "secondary file" paths that aren't easily normalizable or remappable.

I haven't tried to make a minimal reproduction yet, but this output should illustrate the problem:

 error[E0277]: the trait bound `usize: rocket::http::uri::Ignorable<rocket::http::uri::Query>` is not satisfied
-  --> $DIR/typed-uri-bad-type.rs:81:34
-   |
-81 |     uri!(other_q: rest = S, id = _);
-   |                                  ^ the trait `rocket::http::uri::Ignorable<rocket::http::uri::Query>` is not implemented for `usize`
-   |
-   = note: required by `rocket::http::uri::assert_ignorable`
+   --> foo/typed-uri-bad-type.rs:41:16
+    |
+41  | fn other_q(id: usize, rest: S) {  }
+    |                ^^^^^ the trait `rocket::http::uri::Ignorable<rocket::http::uri::Query>` is not implemented for `usize`
+...
+81  |     uri!(other_q: rest = S, id = _);
+    |     -------------------------------- in this macro invocation
+    | 
+   ::: /home/jeb/code/Rocket/core/http/src/uri/uri_display.rs:467:40
+    |
+467 | pub fn assert_ignorable<P: UriPart, T: Ignorable<P>>() {  }
+    |                                        ------------ required by this bound in `rocket::http::uri::assert_ignorable`

(...)

status: exit code: 1
command: "rustc" "tests/ui-fail/typed-uri-bad-type.rs" "-L" "/tmp" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-C" "prefer-dynamic" "-o" "/tmp/typed-uri-bad-type.stage-id" "--remap-path-prefix" "/home/jeb/code/Rocket=/Rocket" "--remap-path-prefix" "tests/ui-fail=foo" "-L" "crate=/home/jeb/code/Rocket/target/debug" "-L" "dependency=/home/jeb/code/Rocket/target/debug/deps" "--extern" "rocket_http=/home/jeb/code/Rocket/target/debug/deps/librocket_http-1faeb9d2934513de.rlib" "--extern" "rocket=/home/jeb/code/Rocket/target/debug/deps/librocket-44cf6b9b35acb885.rlib" "-L" "/tmp/typed-uri-bad-type.stage-id.aux" "-A" "unused"

Here I'm remapping tests/ui-fail to foo to demonstrate that --remap-path-prefix works at all. Remapping /home/jeb/code/Rocket to /Rocket is the real goal, because if I can get that to work we should have errors that are identical across build environments. However, it is not actually remapped in the output. Is --remap-path-prefix intended to apply to these?

My first attempt at this was to update compiletest to normalize $SRC_DIR (Manishearth/compiletest-rs#198), but that has some problems that I think the --remap-path-prefix approach would neatly avoid.

Meta

rustc --version --verbose:

rustc 1.40.0-nightly (1423bec 2019-11-05)
binary: rustc
commit-hash: 1423bec
commit-date: 2019-11-05
host: x86_64-unknown-linux-gnu
release: 1.40.0-nightly
LLVM version: 9.0

[jebrosen]

Using https://github.com/jebrosen/rust-lang-66251 (the initial commit here) as a smaller reproduction example:

$ cargo rustc -- --remap-path-prefix="/home/jeb/code/=test"
   Compiling crate2 v0.1.0 (/home/jeb/code/rust-lang-66251/crate2)
   Compiling rust-lang-66251 v0.1.0 (/home/jeb/code/rust-lang-66251)
error[E0277]: `*const ()` doesn't implement `std::fmt::Display`
 --> src/main.rs:2:27
  |
2 |     crate2::wants_display(&() as *const ());
  |                           ^^^^^^^^^^^^^^^^ `*const ()` cannot be formatted with the default formatter
  | 
 ::: /home/jeb/code/rust-lang-66251/crate2/src/lib.rs:1:25
  |
1 | pub fn wants_display<T: std::fmt::Display>(x: T) {
  |                         ----------------- required by this bound in `crate2::wants_display`
  |
  = help: the trait `std::fmt::Display` is not implemented for `*const ()`
  = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead

error: aborting due to previous error

For more information about this error, try `rustc --explain E0277`.
error: could not compile `rust-lang-66251`.

To learn more, run the command again with --verbose.

This only occurs when the secondary file is not in the current crate, and certain operations (e.g. renaming directories) cause the secondary file to not be shown at all until a cargo clean.

[jyn514]

I think this may be a duplicate of #66955.

[jebrosen]

I don't think that is related; I can reproduce the same behavior from my previous comment on the latest nightly, including renaming a directory making the second part of the diagnostic disappear.

Out of curiosity I tried setting it in RUSTFLAGS instead, and that always makes the second part disappear:

.../work/rocket/rust-lang-66251 (master=) ♥ RUSTFLAGS='--remap-path-prefix=/home/jeb/work=oops' cargo build
   Compiling crate2 v0.1.0 (/home/jeb/work/rocket/rust-lang-66251/crate2)
   Compiling rust-lang-66251 v0.1.0 (/home/jeb/work/rocket/rust-lang-66251)
error[E0277]: `*const ()` doesn't implement `std::fmt::Display`
 --> src/main.rs:2:27
  |
2 |     crate2::wants_display(&() as *const ());
  |                           ^^^^^^^^^^^^^^^^ `*const ()` cannot be formatted with the default formatter
  |
  = help: the trait `std::fmt::Display` is not implemented for `*const ()`
  = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead

error: aborting due to previous error

For more information about this error, try `rustc --explain E0277`.
error: could not compile `rust-lang-66251`

To learn more, run the command again with --verbose.

I have no idea how remap-path-prefix is actually implemented, but it seems plausible that prefix remapping would "break" whatever method is used to look up span information and associated source code in crates other than the main target.

[jyn514]

I ran into this today (also while writing a UI test, funnily enough: #117609). The problem is that remap-path-prefix only applies to the current crate, not to paths embedded in metadata.

; /bin/cat lib.rs 
pub fn baz(_: usize) {}
; /bin/cat main.rs 
fn main() {
    lib::baz();
}

; rustc lib.rs --crate-type lib
; rustc main.rs --extern lib=liblib.rlib --remap-path-prefix=$(realpath .)=/foo 2>&1 | grep lib.rs
 --> /home/jyn/src/example/src/lib.rs:1:8

; rustc lib.rs --crate-type lib --remap-path-prefix=$(realpath .)=/foo
; rustc main.rs --extern lib=liblib.rlib --remap-path-prefix=$(realpath .)=/foo 2>&1 | grep lib.rs
 --> /foo/lib.rs:1:8

[jyn514]

rust/compiler/rustc_span/src/source_map.rs

Lines 209 to 211 in f405ce8

	// This is used to apply the file path remapping as specified via
	// `--remap-path-prefix` to all `SourceFile`s allocated within this `SourceMap`.
	path_mapping: FilePathMapping,

specifically says files within this source crate, so it's unclear to me whether this behavior is intentional or not? in particular filename is ignored if there's already a SourceFile registered:

rust/compiler/rustc_span/src/source_map.rs

Lines 323 to 327 in f405ce8

	let (filename, _) = self.path_mapping.map_filename_prefix(&filename);
	let file_id = StableSourceFileId::new_from_name(&filename, LOCAL_CRATE);
	match self.source_file_by_stable_id(file_id) {
	Some(lrc_sf) => Ok(lrc_sf),

@michaelwoerister how do you expect this to behave if --remap-path-prefix was passed to only some of the crates?

[michaelwoerister]

cargo rustc only supplies the given arguments to the final rustc invocation, so it's not a good match for path remapping. RUSTFLAGS is indeed the way to go.

I don't know why it makes part of the warning disappear. I could imagine that's because the file referred does not actually exist on disk (because the path is remapped), and the compiler fails while trying to construct the text of the warning snippet.

However, that issue seems to have been fixed for current compiler versions:

RUSTFLAGS="--remap-path-prefix=/home/xyz/rust-lang-66251=foo" cargo build
   Compiling crate2 v0.1.0 (/home/xyz/rust-lang-66251/crate2)
   Compiling rust-lang-66251 v0.1.0 (/home/xyz/rust-lang-66251)
error[E0277]: `*const ()` doesn't implement `std::fmt::Display`
 --> src/main.rs:2:27
  |
2 |     crate2::wants_display(&() as *const ());
  |     --------------------- ^^^^^^^^^^^^^^^^ `*const ()` cannot be formatted with the default formatter
  |     |
  |     required by a bound introduced by this call
  |
  = help: the trait `std::fmt::Display` is not implemented for `*const ()`
  = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead
note: required by a bound in `wants_display`
 --> foo/crate2/src/lib.rs:1:25
  |
1 | pub fn wants_display<T: std::fmt::Display>(x: T) {
  |                         ^^^^^^^^^^^^^^^^^ required by this bound in `wants_display`

For more information about this error, try `rustc --explain E0277`.
error: could not compile `rust-lang-66251` (bin "rust-lang-66251") due to previous error

[michaelwoerister]

@michaelwoerister how do you expect this to behave if --remap-path-prefix was passed to only some of the crates?

As mentioned in #117836, each crate only gets a single chance of remapping its paths because some of artifacts produced cannot be changed once the crate has been compiled.

[cbeuw]

A special case here is paths in sysroot crates (/rustc/$hash) that got translated to a real local path due to rust-src being present. RFC 3127 said

Hence the default behaviour when rust-src is installed should be to use the local path. These local paths should be then affected by path remappings in the usual way.

That is, despite these paths being imported from another crate, due to the rust-src translation, we should treat them like a real path introduced by the current crate, and apply any --remap-path-prefix to them. This is being implemented in #118149