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 1/5] 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"#); } From 6fbf4441a33f64d546d501656c31d9c6985f1726 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 20 Aug 2024 18:37:08 +0000 Subject: [PATCH 2/5] Tweak diagnostic output ``` error[E0277]: the trait bound `dep_2_reexport::Type: Trait` is not satisfied because the trait comes from a different crate version --> multiple-dep-versions.rs:7:18 | 7 | do_something(Type); | ^^^^ the trait `Trait` is not implemented for `dep_2_reexport::Type` | note: there are multiple different versions of crate `dependency` in the dependency graph --> /home/gh-estebank/rust/build/x86_64-unknown-linux-gnu/test/run-make/crate-loading/rmake_out/multiple-dep-versions-1.rs:4:1 | 3 | pub struct Type(pub i32); | --------------- this type implements the required trait 4 | pub trait Trait { | ^^^^^^^^^^^^^^^ this is the required trait | ::: multiple-dep-versions.rs:1:1 | 1 | extern crate dep_2_reexport; | ---------------------------- one version of crate `dependency` is used here, as a dependency of crate `foo` 2 | extern crate dependency; | ------------------------ one version of crate `dependency` is used here, as a direct dependency of the current crate | ::: /home/gh-estebank/rust/build/x86_64-unknown-linux-gnu/test/run-make/crate-loading/rmake_out/multiple-dep-versions-2.rs:3:1 | 3 | pub struct Type; | --------------- this type doesn't implement the required trait 4 | pub trait Trait { | --------------- this is the found trait = note: two types coming from two different versions of the same crate are different types even if they look the same = help: you can use `cargo tree` to explore your dependency tree note: required by a bound in `do_something` --> /home/gh-estebank/rust/build/x86_64-unknown-linux-gnu/test/run-make/crate-loading/rmake_out/multiple-dep-versions-1.rs:12:24 | 12 | pub fn do_something(_: X) {} | ^^^^^ required by this bound in `do_something` ``` --- .../traits/fulfillment_errors.rs | 40 +++++------ tests/run-make/crate-loading/rmake.rs | 66 ++++++++----------- 2 files changed, 48 insertions(+), 58 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 b3d622d0c9bf5..088fa668914a7 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 @@ -1723,7 +1723,14 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { } else { None }; - let spans: Vec<_> = [trait_def_id, other_trait_def_id] + let candidates = if impl_candidates.is_empty() { + alternative_candidates(trait_def_id) + } else { + impl_candidates.into_iter().map(|cand| cand.trait_ref).collect() + }; + 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"); + for (sp, label) in [trait_def_id, other_trait_def_id] .iter() .filter_map(|def_id| self.tcx.extern_crate(def_id.krate)) .map(|data| { @@ -1740,25 +1747,9 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { ), ) }) - .collect(); - let mut span: MultiSpan = spans.iter().map(|(sp, _)| *sp).collect::>().into(); - for (sp, label) in spans.into_iter() { + { span.push_span_label(sp, label); } - err.highlighted_span_help(span, vec![ - StringPart::normal("there are ".to_string()), - StringPart::highlighted("multiple different versions".to_string()), - StringPart::normal(" of crate `".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() - }; - 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), @@ -1786,6 +1777,13 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { } 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("there are ".to_string()), + StringPart::highlighted("multiple different versions".to_string()), + StringPart::normal(" of crate `".to_string()), + StringPart::highlighted(format!("{crate_name}")), + StringPart::normal("` in the dependency graph\n".to_string()), + ]); + err.highlighted_note(vec![ StringPart::normal( "two types coming from two different versions of the same crate are \ different types " @@ -1793,7 +1791,11 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { ), StringPart::highlighted("even if they look the same".to_string()), ]); - err.help("you can use `cargo tree` to explore your dependency tree"); + err.highlighted_help(vec![ + StringPart::normal("you can use `".to_string()), + StringPart::highlighted("cargo tree".to_string()), + StringPart::normal("` to explore your dependency tree".to_string()), + ]); return true; } diff --git a/tests/run-make/crate-loading/rmake.rs b/tests/run-make/crate-loading/rmake.rs index ea8802f6e2a20..544bf9ab957a4 100644 --- a/tests/run-make/crate-loading/rmake.rs +++ b/tests/run-make/crate-loading/rmake.rs @@ -18,8 +18,7 @@ fn main() { .extern_("dependency", rust_lib_name("dependency")) .extern_("dep_2_reexport", rust_lib_name("foo")) .run_fail() - .assert_stderr_contains( - r#"error[E0277]: the trait bound `dep_2_reexport::Type: Trait` is not satisfied + .assert_stderr_contains(r#"error[E0277]: the trait bound `dep_2_reexport::Type: Trait` is not satisfied --> multiple-dep-versions.rs:7:18 | 7 | do_something(Type); @@ -27,41 +26,37 @@ fn main() { | | | required by a bound introduced by this call | -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; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ one version of crate `dependency` is used here, as a dependency of crate `foo` -2 | extern crate dependency; - | ^^^^^^^^^^^^^^^^^^^^^^^^ one version of crate `dependency` is used here, as a direct dependency of the current crate"#, - ) - .assert_stderr_contains( - r#" +note: there are multiple different versions of crate `dependency` in the dependency graph"#) + .assert_stderr_contains(r#" 3 | pub struct Type(pub i32); | --------------- this type implements the required trait 4 | pub trait Trait { - | ^^^^^^^^^^^^^^^ this is the required trait"#, - ) - .assert_stderr_contains( - r#" + | ^^^^^^^^^^^^^^^ this is the required trait +"#) + .assert_stderr_contains(r#" +1 | extern crate dep_2_reexport; + | ---------------------------- one version of crate `dependency` is used here, as a dependency of crate `foo` +2 | extern crate dependency; + | ------------------------ one version of crate `dependency` is used here, as a direct dependency of the current crate"#) + .assert_stderr_contains(r#" 3 | pub struct Type; | --------------- 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#" -error[E0599]: no method named `foo` found for struct `dep_2_reexport::Type` in the current scope + = note: two types coming from two different versions of the same crate are different types even if they look the same + = help: you can use `cargo tree` to explore your dependency tree"#) + .assert_stderr_contains(r#"note: required by a bound in `do_something`"#) + .assert_stderr_contains(r#" +12 | pub fn do_something(_: X) {} + | ^^^^^ required by this bound in `do_something`"#) + .assert_stderr_contains(r#"error[E0599]: no method named `foo` found for struct `dep_2_reexport::Type` in the current scope --> multiple-dep-versions.rs:8:10 | 8 | Type.foo(); | ^^^ method not found in `Type` | -note: there are multiple different versions of crate `dependency` in the dependency graph"#, - ) - .assert_stderr_contains( - r#" +note: there are multiple different versions of crate `dependency` in the dependency graph"#) + .assert_stderr_contains(r#" 4 | pub trait Trait { | ^^^^^^^^^^^^^^^ this is the trait that is needed 5 | fn foo(&self); @@ -70,25 +65,19 @@ note: there are multiple different versions of crate `dependency` in the depende ::: multiple-dep-versions.rs:4:18 | 4 | use dependency::{Trait, do_something}; - | ----- `Trait` imported here doesn't correspond to the right version of crate `dependency`"#, - ) - .assert_stderr_contains( - r#" + | ----- `Trait` imported here doesn't correspond to the right version of crate `dependency`"#) + .assert_stderr_contains(r#" 4 | pub trait Trait { - | --------------- this is the trait that was imported"#, - ) - .assert_stderr_contains( - r#" + | --------------- this is the trait that was imported"#) + .assert_stderr_contains(r#" error[E0599]: no function or associated item named `bar` found for struct `dep_2_reexport::Type` in the current scope --> multiple-dep-versions.rs:9:11 | 9 | Type::bar(); | ^^^ function or associated item not found in `Type` | -note: there are multiple different versions of crate `dependency` in the dependency graph"#, - ) - .assert_stderr_contains( - r#" +note: there are multiple different versions of crate `dependency` in the dependency graph"#) + .assert_stderr_contains(r#" 4 | pub trait Trait { | ^^^^^^^^^^^^^^^ this is the trait that is needed 5 | fn foo(&self); @@ -98,8 +87,7 @@ note: there are multiple different versions of crate `dependency` in the depende ::: multiple-dep-versions.rs:4:18 | 4 | use dependency::{Trait, do_something}; - | ----- `Trait` imported here doesn't correspond to the right version of crate `dependency`"#, - ) + | ----- `Trait` imported here doesn't correspond to the right version of crate `dependency`"#) .assert_stderr_contains( r#" 6 | pub struct OtherType; From 8a568d9f15453cbfe5d6f45fa5f5bb32e58b93ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 20 Aug 2024 18:44:29 +0000 Subject: [PATCH 3/5] Remove less relevant info from diagnostic ``` error[E0277]: the trait bound `dep_2_reexport::Type: Trait` is not satisfied because the trait comes from a different crate version --> multiple-dep-versions.rs:7:18 | 7 | do_something(Type); | ^^^^ the trait `Trait` is not implemented for `dep_2_reexport::Type` | note: there are multiple different versions of crate `dependency` in the dependency graph --> /home/gh-estebank/rust/build/x86_64-unknown-linux-gnu/test/run-make/crate-loading/rmake_out/multiple-dep-versions-1.rs:4:1 | 3 | pub struct Type(pub i32); | --------------- this type implements the required trait 4 | pub trait Trait { | ^^^^^^^^^^^^^^^ this is the required trait | ::: multiple-dep-versions.rs:1:1 | 1 | extern crate dep_2_reexport; | ---------------------------- one version of crate `dependency` is used here, as a dependency of crate `foo` 2 | extern crate dependency; | ------------------------ one version of crate `dependency` is used here, as a direct dependency of the current crate | ::: /home/gh-estebank/rust/build/x86_64-unknown-linux-gnu/test/run-make/crate-loading/rmake_out/multiple-dep-versions-2.rs:3:1 | 3 | pub struct Type; | --------------- this type doesn't implement the required trait 4 | pub trait Trait { | --------------- this is the found trait = note: two types coming from two different versions of the same crate are different types even if they look the same = help: you can use `cargo tree` to explore your dependency tree ``` The approach to accomplish this is a HACK, and we'd want a better way to do this. I believe that moving E0277 to be a structured diagnostic would help in that regard. --- .../traits/fulfillment_errors.rs | 18 ++++++++ tests/run-make/crate-loading/rmake.rs | 46 ++++++++----------- 2 files changed, 38 insertions(+), 26 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 088fa668914a7..cb074261cdeed 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 @@ -1796,6 +1796,24 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { StringPart::highlighted("cargo tree".to_string()), StringPart::normal("` to explore your dependency tree".to_string()), ]); + + // FIXME: this is a giant hack for the benefit of this specific diagnostic. Because + // we're so nested in method calls before the error gets emitted, bubbling a single bit + // flag informing the top level caller to stop adding extra detail to the diagnostic, + // would actually be harder to follow. So we do something naughty here: we consume the + // diagnostic, emit it and leave in its place a "delayed bug" that will continue being + // modified but won't actually be printed to end users. This *is not ideal*, but allows + // us to reduce the verbosity of an error that is already quite verbose and increase its + // specificity. Below we modify the main message as well, in a way that *could* break if + // the implementation of Diagnostics change significantly, but that would be caught with + // a make test failure when this diagnostic is tested. + err.primary_message(format!( + "{} because the trait comes from a different crate version", + err.messages[0].0.as_str().unwrap(), + )); + let diag = err.clone(); + err.downgrade_to_delayed_bug(); + self.tcx.dcx().emit_diagnostic(diag); return true; } diff --git a/tests/run-make/crate-loading/rmake.rs b/tests/run-make/crate-loading/rmake.rs index 544bf9ab957a4..2d5913c4bcb09 100644 --- a/tests/run-make/crate-loading/rmake.rs +++ b/tests/run-make/crate-loading/rmake.rs @@ -18,37 +18,31 @@ fn main() { .extern_("dependency", rust_lib_name("dependency")) .extern_("dep_2_reexport", rust_lib_name("foo")) .run_fail() - .assert_stderr_contains(r#"error[E0277]: the trait bound `dep_2_reexport::Type: Trait` is not satisfied - --> multiple-dep-versions.rs:7:18 - | -7 | do_something(Type); - | ------------ ^^^^ the trait `Trait` is not implemented for `dep_2_reexport::Type` - | | - | required by a bound introduced by this call - | + .assert_stderr_contains(r#"error[E0277]: the trait bound `dep_2_reexport::Type: Trait` is not satisfied because the trait comes from a different crate version + --> multiple-dep-versions.rs:7:18 + | +7 | do_something(Type); + | ^^^^ the trait `Trait` is not implemented for `dep_2_reexport::Type` + | note: there are multiple different versions of crate `dependency` in the dependency graph"#) .assert_stderr_contains(r#" -3 | pub struct Type(pub i32); - | --------------- this type implements the required trait -4 | pub trait Trait { - | ^^^^^^^^^^^^^^^ this is the required trait +3 | pub struct Type(pub i32); + | --------------- this type implements the required trait +4 | pub trait Trait { + | ^^^^^^^^^^^^^^^ this is the required trait "#) .assert_stderr_contains(r#" -1 | extern crate dep_2_reexport; - | ---------------------------- one version of crate `dependency` is used here, as a dependency of crate `foo` -2 | extern crate dependency; - | ------------------------ one version of crate `dependency` is used here, as a direct dependency of the current crate"#) +1 | extern crate dep_2_reexport; + | ---------------------------- one version of crate `dependency` is used here, as a dependency of crate `foo` +2 | extern crate dependency; + | ------------------------ one version of crate `dependency` is used here, as a direct dependency of the current crate"#) .assert_stderr_contains(r#" -3 | pub struct Type; - | --------------- this type doesn't implement the required trait -4 | pub trait Trait { - | --------------- this is the found trait - = note: two types coming from two different versions of the same crate are different types even if they look the same - = help: you can use `cargo tree` to explore your dependency tree"#) - .assert_stderr_contains(r#"note: required by a bound in `do_something`"#) - .assert_stderr_contains(r#" -12 | pub fn do_something(_: X) {} - | ^^^^^ required by this bound in `do_something`"#) +3 | pub struct Type; + | --------------- this type doesn't implement the required trait +4 | pub trait Trait { + | --------------- this is the found trait + = note: two types coming from two different versions of the same crate are different types even if they look the same + = help: you can use `cargo tree` to explore your dependency tree"#) .assert_stderr_contains(r#"error[E0599]: no method named `foo` found for struct `dep_2_reexport::Type` in the current scope --> multiple-dep-versions.rs:8:10 | From 81b0de4356a579ac9ca46daac711ce163b8cdf5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 20 Aug 2024 18:59:00 +0000 Subject: [PATCH 4/5] Only show "same type from differnt version" note when relevant --- .../traits/fulfillment_errors.rs | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 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 cb074261cdeed..1109b11d2a71a 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 @@ -1750,6 +1750,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { { span.push_span_label(sp, label); } + let mut points_at_type = false; if let Some(found_type) = found_type { span.push_span_label( self.tcx.def_span(found_type), @@ -1772,6 +1773,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { candidate_span, "this type implements the required trait", ); + points_at_type = true; } } } @@ -1783,14 +1785,20 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { StringPart::highlighted(format!("{crate_name}")), StringPart::normal("` in the dependency graph\n".to_string()), ]); - err.highlighted_note(vec![ - StringPart::normal( - "two types coming from two different versions of the same crate are \ + if points_at_type { + // We only clarify that the same type from different crate versions are not the + // same when we *find* the same type coming from different crate versions, otherwise + // it could be that it was a type provided by a different crate than the one that + // provides the trait, and mentioning this adds verbosity without clarification. + err.highlighted_note(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()), - ]); + .to_string(), + ), + StringPart::highlighted("even if they look the same".to_string()), + ]); + } err.highlighted_help(vec![ StringPart::normal("you can use `".to_string()), StringPart::highlighted("cargo tree".to_string()), From 76c59896e77b5e8c58e6768c24862348baf64874 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 7 Nov 2024 20:56:36 +0000 Subject: [PATCH 5/5] fix test --- tests/ui/typeck/foreign_struct_trait_unimplemented.stderr | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/tests/ui/typeck/foreign_struct_trait_unimplemented.stderr b/tests/ui/typeck/foreign_struct_trait_unimplemented.stderr index b9bb97548f67c..70de107b1ae94 100644 --- a/tests/ui/typeck/foreign_struct_trait_unimplemented.stderr +++ b/tests/ui/typeck/foreign_struct_trait_unimplemented.stderr @@ -6,12 +6,7 @@ LL | needs_test(foreign_struct_trait_unimplemented::B); | | | required by a bound introduced by this call | -help: there are multiple different versions of crate `foreign_struct_trait_unimplemented` in the dependency graph - --> $DIR/foreign_struct_trait_unimplemented.rs:3:1 - | -LL | extern crate foreign_struct_trait_unimplemented; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ one version of crate `foreign_struct_trait_unimplemented` is used here, as a direct dependency of the current crate - = help: you can use `cargo tree` to explore your dependency tree + = help: the trait `Test` is implemented for `A` note: required by a bound in `needs_test` --> $DIR/foreign_struct_trait_unimplemented.rs:10:23 |