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 #89143.
  • Loading branch information
estebank committed Aug 19, 2024
1 parent 6de928d commit 7872d9e
Show file tree
Hide file tree
Showing 2 changed files with 67 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,48 +1704,54 @@ 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::normal("` the your dependency graph".to_string()),
StringPart::highlighted(format!("{crate_name}")),
StringPart::normal("` in the dependency graph".to_string()),
],
);
let candidates = if impl_candidates.is_empty() {
alternative_candidates(trait_def_id)
} 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
11 changes: 7 additions & 4 deletions tests/run-make/crate-loading/rmake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ fn main() {
| |
| required by a bound introduced by this call
|
help: there are multiple different versions of crate `dependency` the your dependency graph
help: there are multiple different versions of crate `dependency` in the dependency graph
--> multiple-dep-versions.rs:1:1
|
1 | extern crate dep_2_reexport;
Expand All @@ -38,14 +38,17 @@ help: there are multiple different versions of crate `dependency` the your depen
.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

0 comments on commit 7872d9e

Please sign in to comment.