From 87191fa34e741c6a7d00b848d83ca249f8cd8cc8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 9 May 2024 20:55:32 +0000 Subject: [PATCH 1/4] Maintain highlighting in `note` and `help` even when they have a span --- compiler/rustc_errors/src/diagnostic.rs | 21 +++++++++++++++++++++ compiler/rustc_errors/src/emitter.rs | 7 +++++-- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index 1610135a0efad..372600017504d 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -737,6 +737,16 @@ impl<'a, G: EmissionGuarantee> Diag<'a, G> { self } + #[rustc_lint_diagnostics] + pub fn highlighted_span_note( + &mut self, + span: impl Into, + msg: Vec, + ) -> &mut Self { + self.sub_with_highlights(Level::Note, msg, span.into()); + self + } + /// This is like [`Diag::note()`], but it's only printed once. #[rustc_lint_diagnostics] pub fn note_once(&mut self, msg: impl Into) -> &mut Self { @@ -811,6 +821,17 @@ impl<'a, G: EmissionGuarantee> Diag<'a, G> { self } + /// Add a help message attached to this diagnostic with a customizable highlighted message. + #[rustc_lint_diagnostics] + pub fn highlighted_span_help( + &mut self, + span: impl Into, + msg: Vec, + ) -> &mut Self { + self.sub_with_highlights(Level::Help, msg, span.into()); + self + } + /// Prints the span with some help above it. /// This is like [`Diag::help()`], but it gets its own span. #[rustc_lint_diagnostics] diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index 5d4d2555100ed..4d78aacb88c88 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -1328,7 +1328,7 @@ impl HumanEmitter { buffer.append(0, ": ", header_style); label_width += 2; } - for (text, _) in msgs.iter() { + for (text, style) in msgs.iter() { let text = self.translate_message(text, args).map_err(Report::new).unwrap(); // Account for newlines to align output to its label. for (line, text) in normalize_whitespace(&text).lines().enumerate() { @@ -1339,7 +1339,10 @@ impl HumanEmitter { if line == 0 { String::new() } else { " ".repeat(label_width) }, text ), - header_style, + match style { + Style::Highlight => *style, + _ => header_style, + }, ); } } From 0dba6c127a21f132e7f1fd1068025a333d8bb08d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 9 May 2024 21:05:05 +0000 Subject: [PATCH 2/4] On trait bound mismatch, detect multiple crate versions in dep tree When encountering an E0277, if the type and the trait both come from a crate with the same name but different crate number, we explain that there are multiple crate versions in the dependency tree. If there's a type that fulfills the bound, and it has the same name as the passed in type and has the same crate name, we explain that the same type in two different versions of the same crate *are different*. ``` error[E0277]: the trait bound `Type: dependency::Trait` is not satisfied --> src/main.rs:4:18 | 4 | do_something(Type); | ------------ ^^^^ the trait `dependency::Trait` is not implemented for `Type` | | | required by a bound introduced by this call | help: you have multiple different versions of crate `dependency` in your dependency graph --> src/main.rs:1:5 | 1 | use bar::do_something; | ^^^ one version of crate `dependency` is used here, as a dependency of crate `bar` 2 | use dependency::Type; | ^^^^^^^^^^ one version of crate `dependency` 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/crate_versions/baz-2/src/lib.rs:1:1 | 1 | pub struct Type; | ^^^^^^^^^^^^^^^ this type doesn't implement the required trait | ::: /home/gh-estebank/crate_versions/baz/src/lib.rs:1:1 | 1 | pub struct Type; | ^^^^^^^^^^^^^^^ this type implements the required trait 2 | pub trait Trait {} | --------------- this is the required trait note: required by a bound in `bar::do_something` --> /home/gh-estebank/crate_versions/baz/src/lib.rs:4:24 | 4 | pub fn do_something(_: X) {} | ^^^^^ required by this bound in `do_something` ``` Address #22750. --- .../error_reporting/type_err_ctxt_ext.rs | 154 ++++++++++++++---- 1 file changed, 121 insertions(+), 33 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs index e50cb2af4b857..fa8ee91c9cd0a 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs @@ -1942,9 +1942,127 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { other: bool, param_env: ty::ParamEnv<'tcx>, ) -> bool { - // If we have a single implementation, try to unify it with the trait ref - // that failed. This should uncover a better hint for what *is* implemented. + let alternative_candidates = |def_id: DefId| { + let mut impl_candidates: Vec<_> = self + .tcx + .all_impls(def_id) + // Ignore automatically derived impls and `!Trait` impls. + .filter_map(|def_id| self.tcx.impl_trait_header(def_id)) + .filter_map(|header| { + (header.polarity != ty::ImplPolarity::Negative + || self.tcx.is_automatically_derived(def_id)) + .then(|| header.trait_ref.instantiate_identity()) + }) + .filter(|trait_ref| { + let self_ty = trait_ref.self_ty(); + // Avoid mentioning type parameters. + if let ty::Param(_) = self_ty.kind() { + false + } + // Avoid mentioning types that are private to another crate + else if let ty::Adt(def, _) = self_ty.peel_refs().kind() { + // FIXME(compiler-errors): This could be generalized, both to + // be more granular, and probably look past other `#[fundamental]` + // types, too. + self.tcx.visibility(def.did()).is_accessible_from(body_def_id, self.tcx) + } else { + true + } + }) + .collect(); + + impl_candidates.sort_by_key(|tr| tr.to_string()); + impl_candidates.dedup(); + impl_candidates + }; + + // We'll check for the case where the reason for the mismatch is that the trait comes from + // 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_map(|def_id| self.tcx.extern_crate(def_id)) + .map(|data| { + let dependency = if data.dependency_of == LOCAL_CRATE { + "direct dependency of the current crate".to_string() + } else { + let dep = self.tcx.crate_name(data.dependency_of); + format!("dependency of crate `{dep}`") + }; + ( + data.span, + format!("one version of crate `{name}` is used here, as a {dependency}"), + ) + }) + .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("you have ".to_string()), + StringPart::highlighted("multiple different versions".to_string()), + StringPart::normal(" of crate `".to_string()), + StringPart::highlighted(format!("{name}")), + StringPart::normal("` in your 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()), + ], + ); + } + return true; + } + if let [single] = &impl_candidates { + // If we have a single implementation, try to unify it with the trait ref + // that failed. This should uncover a better hint for what *is* implemented. if self.probe(|_| { let ocx = ObligationCtxt::new(self); @@ -2099,37 +2217,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { // Mentioning implementers of `Copy`, `Debug` and friends is not useful. return false; } - let mut impl_candidates: Vec<_> = self - .tcx - .all_impls(def_id) - // Ignore automatically derived impls and `!Trait` impls. - .filter_map(|def_id| self.tcx.impl_trait_header(def_id)) - .filter_map(|header| { - (header.polarity != ty::ImplPolarity::Negative - || self.tcx.is_automatically_derived(def_id)) - .then(|| header.trait_ref.instantiate_identity()) - }) - .filter(|trait_ref| { - let self_ty = trait_ref.self_ty(); - // Avoid mentioning type parameters. - if let ty::Param(_) = self_ty.kind() { - false - } - // Avoid mentioning types that are private to another crate - else if let ty::Adt(def, _) = self_ty.peel_refs().kind() { - // FIXME(compiler-errors): This could be generalized, both to - // be more granular, and probably look past other `#[fundamental]` - // types, too. - self.tcx.visibility(def.did()).is_accessible_from(body_def_id, self.tcx) - } else { - true - } - }) - .collect(); - - impl_candidates.sort_by_key(|tr| tr.to_string()); - impl_candidates.dedup(); - return report(impl_candidates, err); + return report(alternative_candidates(def_id), err); } // Sort impl candidates so that ordering is consistent for UI tests. From ec2ccdc54647e5bacd33cb7d243994541789597e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 13 May 2024 04:19:18 +0000 Subject: [PATCH 3/4] Add test for mixing types from two incompatible crate versions --- .../crate-loading/auxiliary/dep-2-reexport.rs | 4 + .../auxiliary/multiple-dep-versions-1.rs | 7 ++ .../auxiliary/multiple-dep-versions-2.rs | 7 ++ .../ui/crate-loading/multiple-dep-versions.rs | 14 +++ .../crate-loading/multiple-dep-versions.svg | 101 ++++++++++++++++++ 5 files changed, 133 insertions(+) create mode 100644 tests/ui/crate-loading/auxiliary/dep-2-reexport.rs create mode 100644 tests/ui/crate-loading/auxiliary/multiple-dep-versions-1.rs create mode 100644 tests/ui/crate-loading/auxiliary/multiple-dep-versions-2.rs create mode 100644 tests/ui/crate-loading/multiple-dep-versions.rs create mode 100644 tests/ui/crate-loading/multiple-dep-versions.svg diff --git a/tests/ui/crate-loading/auxiliary/dep-2-reexport.rs b/tests/ui/crate-loading/auxiliary/dep-2-reexport.rs new file mode 100644 index 0000000000000..3a793bbbb106e --- /dev/null +++ b/tests/ui/crate-loading/auxiliary/dep-2-reexport.rs @@ -0,0 +1,4 @@ +//@ edition:2021 +//@ aux-build:multiple-dep-versions-2.rs +extern crate dependency; +pub use dependency::do_something; diff --git a/tests/ui/crate-loading/auxiliary/multiple-dep-versions-1.rs b/tests/ui/crate-loading/auxiliary/multiple-dep-versions-1.rs new file mode 100644 index 0000000000000..b96bb6d9e6443 --- /dev/null +++ b/tests/ui/crate-loading/auxiliary/multiple-dep-versions-1.rs @@ -0,0 +1,7 @@ +#![crate_name="dependency"] +//@ edition:2021 +//@ compile-flags: -C metadata=1 -C extra-filename=-1 +pub struct Type; +pub trait Trait {} +impl Trait for Type {} +pub fn do_something(_: X) { } diff --git a/tests/ui/crate-loading/auxiliary/multiple-dep-versions-2.rs b/tests/ui/crate-loading/auxiliary/multiple-dep-versions-2.rs new file mode 100644 index 0000000000000..c79157fa4634f --- /dev/null +++ b/tests/ui/crate-loading/auxiliary/multiple-dep-versions-2.rs @@ -0,0 +1,7 @@ +#![crate_name="dependency"] +//@ edition:2021 +//@ compile-flags: -C metadata=2 -C extra-filename=-2 +pub struct Type(pub i32); +pub trait Trait {} +impl Trait for Type {} +pub fn do_something(_: X) {} diff --git a/tests/ui/crate-loading/multiple-dep-versions.rs b/tests/ui/crate-loading/multiple-dep-versions.rs new file mode 100644 index 0000000000000..9e0a5dc5c5dc9 --- /dev/null +++ b/tests/ui/crate-loading/multiple-dep-versions.rs @@ -0,0 +1,14 @@ +//@ aux-build:dep-2-reexport.rs +//@ aux-build:multiple-dep-versions-1.rs +//@ edition:2021 +//@ compile-flags: --error-format=human --color=always --crate-type bin --extern dependency={{build-base}}/crate-loading/multiple-dep-versions/auxiliary/libdependency-1.so --extern dep_2_reexport={{build-base}}/crate-loading/multiple-dep-versions/auxiliary/libdep_2_reexport.so +//@ ignore-windows + +extern crate dependency; +extern crate dep_2_reexport; +use dependency::Type; +use dep_2_reexport::do_something; + +fn main() { + do_something(Type); +} diff --git a/tests/ui/crate-loading/multiple-dep-versions.svg b/tests/ui/crate-loading/multiple-dep-versions.svg new file mode 100644 index 0000000000000..6b89ca691e0bc --- /dev/null +++ b/tests/ui/crate-loading/multiple-dep-versions.svg @@ -0,0 +1,101 @@ + + + + + + + error[E0277]: the trait bound `Type: dependency::Trait` is not satisfied + + --> $DIR/multiple-dep-versions.rs:13:18 + + | + + LL | do_something(Type); + + | ------------ ^^^^ the trait `dependency::Trait` is not implemented for `Type` + + | | + + | required by a bound introduced by this call + + | + + help: you have multiple different versions of crate `dependency` in your dependency graph + + --> $DIR/multiple-dep-versions.rs:7:1 + + | + + LL | extern crate dependency; + + | ^^^^^^^^^^^^^^^^^^^^^^^^ one version of crate `dependency` is used here, as a direct dependency of the current crate + + LL | extern crate dep_2_reexport; + + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ one version of crate `dependency` is used here, as a dependency of crate `dep_2_reexport` + + note: two types coming from two different versions of the same crate are different types even if they look the same + + --> $DIR/auxiliary/multiple-dep-versions-1.rs:4:1 + + | + + LL | pub struct Type; + + | ^^^^^^^^^^^^^^^ this type doesn't implement the required trait + + | + + ::: $DIR/auxiliary/multiple-dep-versions-2.rs:4:1 + + | + + LL | pub struct Type(pub i32); + + | ^^^^^^^^^^^^^^^ this type implements the required trait + + LL | pub trait Trait {} + + | --------------- this is the required trait + + note: required by a bound in `dep_2_reexport::do_something` + + --> $DIR/auxiliary/multiple-dep-versions-2.rs:7:24 + + | + + LL | pub fn do_something<X: Trait>(_: X) {} + + | ^^^^^ required by this bound in `do_something` + + + + error: aborting due to 1 previous error + + + + For more information about this error, try `rustc --explain E0277`. + + + + + + From 782cba8fe3d83427036734327e5a4b4aa6641277 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 13 May 2024 04:30:14 +0000 Subject: [PATCH 4/4] Add `help` about using `cargo tree` --- .../error_reporting/type_err_ctxt_ext.rs | 6 ++--- .../crate-loading/multiple-dep-versions.svg | 24 ++++++++++--------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs index fa8ee91c9cd0a..1c8eecadfbe67 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs @@ -2041,10 +2041,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { 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", - ); + span.push_span_label(sp_found, "this type doesn't implement the required trait"); err.highlighted_span_note( span, vec![ @@ -2057,6 +2054,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { ], ); } + err.help("you can use `cargo tree` to explore your dependency tree"); return true; } diff --git a/tests/ui/crate-loading/multiple-dep-versions.svg b/tests/ui/crate-loading/multiple-dep-versions.svg index 6b89ca691e0bc..671f38d3eb013 100644 --- a/tests/ui/crate-loading/multiple-dep-versions.svg +++ b/tests/ui/crate-loading/multiple-dep-versions.svg @@ -1,4 +1,4 @@ - +