Skip to content

Commit

Permalink
Distinguish crates with the same name in type errors
Browse files Browse the repository at this point in the history
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<u32, u32>` 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.
  • Loading branch information
jyn514 committed Jan 28, 2024
1 parent 6b4f1c5 commit 7f4072f
Show file tree
Hide file tree
Showing 8 changed files with 80 additions and 2 deletions.
18 changes: 17 additions & 1 deletion compiler/rustc_infer/src/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
{
Expand All @@ -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);
}
Expand Down
6 changes: 5 additions & 1 deletion src/bootstrap/src/core/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
2 changes: 2 additions & 0 deletions tests/incremental/circular-dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,14 @@ 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());
//[cfail2]~^ ERROR mismatched types [E0308]
//[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
}
20 changes: 20 additions & 0 deletions tests/run-make/type-mismatch-sysroot-crate/Makefile
Original file line number Diff line number Diff line change
@@ -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
28 changes: 28 additions & 0 deletions tests/run-make/type-mismatch-sysroot-crate/expected.txt
Original file line number Diff line number Diff line change
@@ -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`.
1 change: 1 addition & 0 deletions tests/run-make/type-mismatch-sysroot-crate/hashbrown.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pub struct HashMap;
6 changes: 6 additions & 0 deletions tests/run-make/type-mismatch-sysroot-crate/mismatch.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#![feature(rustc_private)]
extern crate hashbrown;

fn main() {
uses_hashbrown::foo(hashbrown::HashMap::default())
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pub fn foo(_: hashbrown::HashMap) {}

0 comments on commit 7f4072f

Please sign in to comment.