From 7f4072fb8ba770fcf342b48c203399f11f26c527 Mon Sep 17 00:00:00 2001 From: jyn Date: Sun, 5 Nov 2023 05:56:16 -0500 Subject: [PATCH] Distinguish crates with the same name in type errors Previously, errors for crates with the same name would only distinguish them by the span of the source: ``` note: `HashMap<_, _, _, _>` is defined in crate `hashbrown` --> /Users/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hashbrown-0.12.3/src/map.rs:188:1 note: `HashMap` is defined in crate `hashbrown` --> /Users/jyn/.local/lib/cargo/registry/src/index.crates.io-6f17d22bba15001f/hashbrown-0.12.3/src/map.rs:188:1 ``` When the same version of the crate is loaded twice, this isn't particularly helpful. Additionally show where the .rlib was loaded from (in this case one was loaded from the sysroot and the other from a cargo target dir). --- This also remaps cargo paths more eagerly; particularly, it unconditionally remaps cargo paths for libstd unconditionally, even if `remap-debuginfo = false`. This is the first time that cargo paths have shown up in UI tests; if the paths aren't remapped, they will differ depending on the value of `remap-debuginfo`, which means tests will fail either locally or in CI. This can't be worked around with adhoc normalization in the test makefile because it impacts the column width used for line numbers (when paths are remapped, we don't show the cargo sources). I could be convinced this is not the best solution. It's possible we could take some other approach, like how `CFG_VIRTUAL_RUST_SOURCE_BASE_DIR` opportunistically reverses the mapping for `compiler/`, which doesn't impact the error messages standard library developers see. That would be a bit more involved to get working, though. --- .../src/infer/error_reporting/mod.rs | 18 +++++++++++- src/bootstrap/src/core/builder.rs | 6 +++- tests/incremental/circular-dependencies.rs | 2 ++ .../type-mismatch-sysroot-crate/Makefile | 20 +++++++++++++ .../type-mismatch-sysroot-crate/expected.txt | 28 +++++++++++++++++++ .../type-mismatch-sysroot-crate/hashbrown.rs | 1 + .../type-mismatch-sysroot-crate/mismatch.rs | 6 ++++ .../uses-hashbrown.rs | 1 + 8 files changed, 80 insertions(+), 2 deletions(-) create mode 100644 tests/run-make/type-mismatch-sysroot-crate/Makefile create mode 100644 tests/run-make/type-mismatch-sysroot-crate/expected.txt create mode 100644 tests/run-make/type-mismatch-sysroot-crate/hashbrown.rs create mode 100644 tests/run-make/type-mismatch-sysroot-crate/mismatch.rs create mode 100644 tests/run-make/type-mismatch-sysroot-crate/uses-hashbrown.rs diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs index 7882e761a0c31..33ac7b33c2003 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs @@ -1843,6 +1843,8 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { let expected_defid = expected_adt.did(); diagnostic.note(format!("{found_name} and {expected_name} have similar names, but are actually distinct types")); + let have_same_crate_name = self.tcx.crate_name(found_defid.krate) + == self.tcx.crate_name(expected_defid.krate); for (defid, name) in [(found_defid, found_name), (expected_defid, expected_name)] { @@ -1862,7 +1864,21 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { format!("{name} is defined in the current crate") } else { let crate_name = self.tcx.crate_name(defid.krate); - format!("{name} is defined in crate `{crate_name}`") + diagnostic.span_note( + def_span, + format!("{name} is defined in crate `{crate_name}`"), + ); + + // If these are named the same, give a hint about why the compiler thinks they're different. + if have_same_crate_name { + let crate_paths = self.tcx.crate_extern_paths(defid.krate); + diagnostic.note(format!( + "`{crate_name}` was loaded from {}", + crate_paths[0].display() + )); + } + + continue; }; diagnostic.span_note(def_span, msg); } diff --git a/src/bootstrap/src/core/builder.rs b/src/bootstrap/src/core/builder.rs index 3770d0687b242..17beb487898aa 100644 --- a/src/bootstrap/src/core/builder.rs +++ b/src/bootstrap/src/core/builder.rs @@ -1798,7 +1798,11 @@ impl<'a> Builder<'a> { cargo.env("CFG_VIRTUAL_RUST_SOURCE_BASE_DIR", map_to); } - if self.config.rust_remap_debuginfo { + // Users won't have the original cargo registry from the CI builder available, even if they have `rust-src` installed. + // Don't show their sources in UI tests even if they're available. + // As a happy side-effect, this fixes a few UI tests when download-rustc is enabled. + // NOTE: only set this when building std so the error messages are better for rustc itself. + if self.config.rust_remap_debuginfo || mode == Mode::Std { let mut env_var = OsString::new(); if self.config.vendor { let vendor = self.build.src.join("vendor"); diff --git a/tests/incremental/circular-dependencies.rs b/tests/incremental/circular-dependencies.rs index 10673066a9df0..5cbc99750b1c3 100644 --- a/tests/incremental/circular-dependencies.rs +++ b/tests/incremental/circular-dependencies.rs @@ -26,6 +26,7 @@ fn test() { //[cfail2]~| NOTE arguments to this function are incorrect //[cfail2]~| NOTE `Foo` and `circular_dependencies::Foo` have similar names, but are actually distinct types //[cfail2]~| NOTE the crate `circular_dependencies` is compiled multiple times, possibly with different configurations + //[cfail2]~| NOTE loaded from //[cfail2]~| NOTE function defined here consume_foo(aux::produce_foo()); @@ -33,5 +34,6 @@ fn test() { //[cfail2]~| NOTE expected `Foo`, found `circular_dependencies::Foo` //[cfail2]~| NOTE arguments to this function are incorrect //[cfail2]~| NOTE `circular_dependencies::Foo` and `Foo` have similar names, but are actually distinct types + //[cfail2]~| NOTE loaded from //[cfail2]~| NOTE the crate `circular_dependencies` is compiled multiple times, possibly with different configurations } diff --git a/tests/run-make/type-mismatch-sysroot-crate/Makefile b/tests/run-make/type-mismatch-sysroot-crate/Makefile new file mode 100644 index 0000000000000..9438dcb5c976e --- /dev/null +++ b/tests/run-make/type-mismatch-sysroot-crate/Makefile @@ -0,0 +1,20 @@ +# Test that we emit useful diagnostics when the same crate is loaded from the sysroot and --extern. +# This can't be a run-make test because it needs the aux-file to itself have a dependency passed with --extern +# (and just bare `--extern hashbrown` errors too early, while compiling `uses_hashbrown.rs`). + +include ../tools.mk +define SED_REGEX +s#[^ ]*registry/src/index.crates.io[^ ]*/src#CARGO_REGISTRY/hashbrown-VERSION/src#; +s#[^ ]*/build/[^ ]*/lib/rustlib/[^ ]*#BUILD_DIR/HOST/stageN/lib/rustlib/TARGET/lib/libhashbrown.rlib#; +s#[^ ]*/build/[^ ]*/test/run-make/#BUILD_DIR/HOST/test/run-make/# +s#[^ ]*tests/run-make/#TEST_DIR/# +endef +export SED_REGEX + +all: + $(RUSTC) hashbrown.rs --crate-type lib + $(RUSTC) uses-hashbrown.rs --extern hashbrown=$(TMPDIR)/libhashbrown.rlib --crate-type lib + $(RUSTC) mismatch.rs --extern uses_hashbrown=$(TMPDIR)/libuses_hashbrown.rlib 2>$(TMPDIR)/stderr.txt || true + sed -e "$$SED_REGEX" < $(TMPDIR)/stderr.txt > $(TMPDIR)/normalized.txt + $(RUSTC_TEST_OP) $(TMPDIR)/normalized.txt expected.txt + $(CGREP) "loaded from" < $(TMPDIR)/stderr.txt diff --git a/tests/run-make/type-mismatch-sysroot-crate/expected.txt b/tests/run-make/type-mismatch-sysroot-crate/expected.txt new file mode 100644 index 0000000000000..6c4c210198e4c --- /dev/null +++ b/tests/run-make/type-mismatch-sysroot-crate/expected.txt @@ -0,0 +1,28 @@ +error[E0308]: mismatched types + --> mismatch.rs:5:25 + | +5 | uses_hashbrown::foo(hashbrown::HashMap::default()) + | ------------------- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `HashMap`, found `HashMap<_, _, _, _>` + | | + | arguments to this function are incorrect + | + = note: `hashbrown::HashMap<_, _, _, _>` and `hashbrown::HashMap` have similar names, but are actually distinct types +note: `hashbrown::HashMap<_, _, _, _>` is defined in crate `hashbrown` + --> /rust/deps/hashbrown-0.14.3/src/map.rs:190:1 + = note: `hashbrown` was loaded from BUILD_DIR/HOST/stageN/lib/rustlib/TARGET/lib/libhashbrown.rlib +note: `hashbrown::HashMap` is defined in crate `hashbrown` + --> TEST_DIR/type-mismatch-sysroot-crate/hashbrown.rs:1:1 + | +1 | pub struct HashMap; + | ^^^^^^^^^^^^^^^^^^ + = note: `hashbrown` was loaded from BUILD_DIR/HOST/test/run-make/type-mismatch-sysroot-crate/type-mismatch-sysroot-crate/libhashbrown.rlib + = note: perhaps two different versions of crate `hashbrown` are being used? +note: function defined here + --> TEST_DIR/type-mismatch-sysroot-crate/uses-hashbrown.rs:1:8 + | +1 | pub fn foo(_: hashbrown::HashMap) {} + | ^^^ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0308`. diff --git a/tests/run-make/type-mismatch-sysroot-crate/hashbrown.rs b/tests/run-make/type-mismatch-sysroot-crate/hashbrown.rs new file mode 100644 index 0000000000000..7a504c514742d --- /dev/null +++ b/tests/run-make/type-mismatch-sysroot-crate/hashbrown.rs @@ -0,0 +1 @@ +pub struct HashMap; diff --git a/tests/run-make/type-mismatch-sysroot-crate/mismatch.rs b/tests/run-make/type-mismatch-sysroot-crate/mismatch.rs new file mode 100644 index 0000000000000..5a6200aa59f60 --- /dev/null +++ b/tests/run-make/type-mismatch-sysroot-crate/mismatch.rs @@ -0,0 +1,6 @@ +#![feature(rustc_private)] +extern crate hashbrown; + +fn main() { + uses_hashbrown::foo(hashbrown::HashMap::default()) +} diff --git a/tests/run-make/type-mismatch-sysroot-crate/uses-hashbrown.rs b/tests/run-make/type-mismatch-sysroot-crate/uses-hashbrown.rs new file mode 100644 index 0000000000000..fcf6c27b997a6 --- /dev/null +++ b/tests/run-make/type-mismatch-sysroot-crate/uses-hashbrown.rs @@ -0,0 +1 @@ +pub fn foo(_: hashbrown::HashMap) {}