From 35bde071152f422c7234f895ec055d50c80dba79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 8 Aug 2024 21:35:25 +0000 Subject: [PATCH] Tweak detection of multiple crate versions to be more ecompassing 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. --- .../traits/fulfillment_errors.rs | 95 +++++++++++-------- .../crate-loading/multiple-dep-versions-3.rs | 5 + .../crate-loading/multiple-dep-versions.rs | 3 +- tests/run-make/crate-loading/rmake.rs | 15 ++- 4 files changed, 74 insertions(+), 44 deletions(-) diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs index a1c97a385676d..b3d622d0c9bf5 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs @@ -1707,15 +1707,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() - .filter(|def_id| def_id.krate != LOCAL_CRATE) + 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 { @@ -1726,7 +1735,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(); @@ -1738,7 +1749,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()), ]); let candidates = if impl_candidates.is_empty() { @@ -1746,36 +1757,42 @@ 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 = 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", + ); + } } - }) { - 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()), - ]); } + 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; } diff --git a/tests/run-make/crate-loading/multiple-dep-versions-3.rs b/tests/run-make/crate-loading/multiple-dep-versions-3.rs index 07d888e9f1034..f5c4d1baa811a 100644 --- a/tests/run-make/crate-loading/multiple-dep-versions-3.rs +++ b/tests/run-make/crate-loading/multiple-dep-versions-3.rs @@ -3,3 +3,8 @@ extern crate dependency; pub use dependency::Type; +pub struct OtherType; +impl dependency::Trait for OtherType { + fn foo(&self) {} + fn bar() {} +} diff --git a/tests/run-make/crate-loading/multiple-dep-versions.rs b/tests/run-make/crate-loading/multiple-dep-versions.rs index 113af9c30255b..c68a9e6489f54 100644 --- a/tests/run-make/crate-loading/multiple-dep-versions.rs +++ b/tests/run-make/crate-loading/multiple-dep-versions.rs @@ -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::{Trait, do_something}; fn main() { do_something(Type); Type.foo(); Type::bar(); + do_something(OtherType); } diff --git a/tests/run-make/crate-loading/rmake.rs b/tests/run-make/crate-loading/rmake.rs index 5d3302c7b02a0..ea8802f6e2a20 100644 --- a/tests/run-make/crate-loading/rmake.rs +++ b/tests/run-make/crate-loading/rmake.rs @@ -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#" @@ -96,5 +99,9 @@ note: there are multiple different versions of crate `dependency` in the depende | 4 | use dependency::{Trait, do_something}; | ----- `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"#); }