Skip to content

Commit

Permalink
Tweak detection of multiple crate versions to be more ecompassing
Browse files Browse the repository at this point in the history
Previously, we only emitted the additional context if the type was in the same crate as the trait that appeared multiple times in the dependency tree. Now, we look at all traits looking for two with the same name in different crates with the same crate number, and we are more flexible looking for the types involved. This will work even if the type that implements the wrong trait version is from a different crate entirely.

```
error[E0277]: the trait bound `CustomErrorHandler: ErrorHandler` is not satisfied
 --> src/main.rs:5:17
  |
5 |     cnb_runtime(CustomErrorHandler {});
  |     ----------- ^^^^^^^^^^^^^^^^^^^^^ the trait `ErrorHandler` is not implemented for `CustomErrorHandler`
  |     |
  |     required by a bound introduced by this call
  |
help: you have multiple different versions of crate `c` in your dependency graph
 --> src/main.rs:1:5
  |
1 | use b::CustomErrorHandler;
  |     ^ one version of crate `c` is used here, as a dependency of crate `b`
2 | use c::cnb_runtime;
  |     ^ one version of crate `c` is used here, as a direct dependency of the current crate
note: two types coming from two different versions of the same crate are different types even if they look the same
 --> /home/gh-estebank/testcase-rustc-crate-version-mismatch/c-v0.2/src/lib.rs:1:1
  |
1 | pub trait ErrorHandler {}
  | ^^^^^^^^^^^^^^^^^^^^^^ this is the required trait
  |
 ::: /home/gh-estebank/testcase-rustc-crate-version-mismatch/b/src/lib.rs:1:1
  |
1 | pub struct CustomErrorHandler {}
  | ----------------------------- this type doesn't implement the required trait
  |
 ::: /home/gh-estebank/testcase-rustc-crate-version-mismatch/c-v0.1/src/lib.rs:1:1
  |
1 | pub trait ErrorHandler {}
  | ---------------------- this is the found trait
  = help: you can use `cargo tree` to explore your dependency tree
note: required by a bound in `cnb_runtime`
 --> /home/gh-estebank/testcase-rustc-crate-version-mismatch/c-v0.2/src/lib.rs:3:41
  |
3 | pub fn cnb_runtime(_error_handler: impl ErrorHandler) {}
  |                                         ^^^^^^^^^^^^ required by this bound in `cnb_runtime`
```

Fix rust-lang#89143.
  • Loading branch information
estebank committed Aug 28, 2024
1 parent 100fde5 commit 8bd1191
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 46 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1660,14 +1660,24 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
// one crate version and the type comes from another crate version, even though they both
// are from the same crate.
let trait_def_id = trait_ref.def_id();
if let ty::Adt(def, _) = trait_ref.self_ty().skip_binder().peel_refs().kind()
&& let found_type = def.did()
&& trait_def_id.krate != found_type.krate
&& self.tcx.crate_name(trait_def_id.krate) == self.tcx.crate_name(found_type.krate)
{
let name = self.tcx.crate_name(trait_def_id.krate);
let spans: Vec<_> = [trait_def_id, found_type]
.into_iter()
let trait_name = self.tcx.item_name(trait_def_id);
let crate_name = self.tcx.crate_name(trait_def_id.krate);
if let Some(other_trait_def_id) = self.tcx.all_traits().find(|def_id| {
trait_name == self.tcx.item_name(trait_def_id)
&& trait_def_id.krate != def_id.krate
&& crate_name == self.tcx.crate_name(def_id.krate)
}) {
// We've found two different traits with the same name, same crate name, but
// different crate `DefId`. We highlight the traits.

let found_type =
if let ty::Adt(def, _) = trait_ref.self_ty().skip_binder().peel_refs().kind() {
Some(def.did())
} else {
None
};
let spans: Vec<_> = [trait_def_id, other_trait_def_id]
.iter()
.filter_map(|def_id| self.tcx.extern_crate(def_id.krate))
.map(|data| {
let dependency = if data.dependency_of == LOCAL_CRATE {
Expand All @@ -1678,7 +1688,9 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
};
(
data.span,
format!("one version of crate `{name}` is used here, as a {dependency}"),
format!(
"one version of crate `{crate_name}` is used here, as a {dependency}"
),
)
})
.collect();
Expand All @@ -1692,7 +1704,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
StringPart::normal("there are ".to_string()),
StringPart::highlighted("multiple different versions".to_string()),
StringPart::normal(" of crate `".to_string()),
StringPart::highlighted(format!("{name}")),
StringPart::highlighted(format!("{crate_name}")),
StringPart::normal("` in the dependency graph".to_string()),
],
);
Expand All @@ -1701,39 +1713,45 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
} else {
impl_candidates.into_iter().map(|cand| cand.trait_ref).collect()
};
if let Some((sp_candidate, sp_found)) = candidates.iter().find_map(|trait_ref| {
if let ty::Adt(def, _) = trait_ref.self_ty().peel_refs().kind()
&& let candidate_def_id = def.did()
&& let Some(name) = self.tcx.opt_item_name(candidate_def_id)
&& let Some(found) = self.tcx.opt_item_name(found_type)
&& name == found
&& candidate_def_id.krate != found_type.krate
&& self.tcx.crate_name(candidate_def_id.krate)
== self.tcx.crate_name(found_type.krate)
{
// A candidate was found of an item with the same name, from two separate
// versions of the same crate, let's clarify.
Some((self.tcx.def_span(candidate_def_id), self.tcx.def_span(found_type)))
} else {
None
}
}) {
let mut span: MultiSpan = vec![sp_candidate, sp_found].into();
span.push_span_label(self.tcx.def_span(trait_def_id), "this is the required trait");
span.push_span_label(sp_candidate, "this type implements the required trait");
span.push_span_label(sp_found, "this type doesn't implement the required trait");
err.highlighted_span_note(
span,
vec![
StringPart::normal(
"two types coming from two different versions of the same crate are \
different types "
.to_string(),
),
StringPart::highlighted("even if they look the same".to_string()),
],
let mut span: MultiSpan = self.tcx.def_span(trait_def_id).into();
span.push_span_label(self.tcx.def_span(trait_def_id), "this is the required trait");
if let Some(found_type) = found_type {
span.push_span_label(
self.tcx.def_span(found_type),
"this type doesn't implement the required trait",
);
for trait_ref in candidates {
if let ty::Adt(def, _) = trait_ref.self_ty().peel_refs().kind()
&& let candidate_def_id = def.did()
&& let Some(name) = self.tcx.opt_item_name(candidate_def_id)
&& let Some(found) = self.tcx.opt_item_name(found_type)
&& name == found
&& candidate_def_id.krate != found_type.krate
&& self.tcx.crate_name(candidate_def_id.krate)
== self.tcx.crate_name(found_type.krate)
{
// A candidate was found of an item with the same name, from two separate
// versions of the same crate, let's clarify.
let candidate_span = self.tcx.def_span(candidate_def_id);
span.push_span_label(
candidate_span,
"this type implements the required trait",
);
}
}
}
span.push_span_label(self.tcx.def_span(other_trait_def_id), "this is the found trait");
err.highlighted_span_note(
span,
vec![
StringPart::normal(
"two types coming from two different versions of the same crate are \
different types "
.to_string(),
),
StringPart::highlighted("even if they look the same".to_string()),
],
);
err.help("you can use `cargo tree` to explore your dependency tree");
return true;
}
Expand Down
5 changes: 5 additions & 0 deletions tests/run-make/crate-loading/multiple-dep-versions-3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,8 @@

extern crate dependency;
pub use dependency::Type;
pub struct OtherType;
impl dependency::Trait for OtherType {
fn foo(&self) {}
fn bar() {}
}
3 changes: 2 additions & 1 deletion tests/run-make/crate-loading/multiple-dep-versions.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
extern crate dep_2_reexport;
extern crate dependency;
use dep_2_reexport::Type;
use dep_2_reexport::{OtherType, Type};
use dependency::{do_something, Trait};

fn main() {
do_something(Type);
Type.foo();
Type::bar();
do_something(OtherType);
}
15 changes: 11 additions & 4 deletions tests/run-make/crate-loading/rmake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,17 @@ help: there are multiple different versions of crate `dependency` in the depende
.assert_stderr_contains(
r#"
3 | pub struct Type(pub i32);
| ^^^^^^^^^^^^^^^ this type implements the required trait
| --------------- this type implements the required trait
4 | pub trait Trait {
| --------------- this is the required trait"#,
| ^^^^^^^^^^^^^^^ this is the required trait"#,
)
.assert_stderr_contains(
r#"
3 | pub struct Type;
| ^^^^^^^^^^^^^^^ this type doesn't implement the required trait"#,
| --------------- this type doesn't implement the required trait
4 | pub trait Trait {
| --------------- this is the found trait
= help: you can use `cargo tree` to explore your dependency tree"#,
)
.assert_stderr_contains(
r#"
Expand Down Expand Up @@ -96,5 +99,9 @@ note: there are multiple different versions of crate `dependency` in the depende
|
4 | use dependency::{do_something, Trait};
| ----- `Trait` imported here doesn't correspond to the right version of crate `dependency`"#,
);
)
.assert_stderr_contains(
r#"
6 | pub struct OtherType;
| -------------------- this type doesn't implement the required trait"#);
}

0 comments on commit 8bd1191

Please sign in to comment.