Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

On trait bound mismatch, detect multiple crate versions in dep tree #124944

Merged
merged 8 commits into from
Aug 7, 2024
21 changes: 21 additions & 0 deletions compiler/rustc_errors/src/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -741,6 +741,16 @@ impl<'a, G: EmissionGuarantee> Diag<'a, G> {
self
}

#[rustc_lint_diagnostics]
pub fn highlighted_span_note(
&mut self,
span: impl Into<MultiSpan>,
msg: Vec<StringPart>,
) -> &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<SubdiagMessage>) -> &mut Self {
Expand Down Expand Up @@ -815,6 +825,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<MultiSpan>,
msg: Vec<StringPart>,
) -> &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]
Expand Down
15 changes: 13 additions & 2 deletions compiler/rustc_errors/src/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1347,7 +1347,7 @@ impl HumanEmitter {
label_width += 2;
}
let mut line = 0;
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 text in normalize_whitespace(&text).lines() {
Expand All @@ -1358,10 +1358,21 @@ impl HumanEmitter {
if line == 0 { String::new() } else { " ".repeat(label_width) },
text
),
header_style,
match style {
Style::Highlight => *style,
_ => header_style,
},
);
line += 1;
}
// We add lines above, but if the last line has no explicit newline (which would
// yield an empty line), then we revert one line up to continue with the next
// styled text chunk on the same line as the last one from the prior one. Otherwise
// every `text` would appear on their own line (because even though they didn't end
// in '\n', they advanced `line` by one).
if line > 0 {
line -= 1;
}
}
if self.short_message {
let labels = msp
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@ use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::unord::UnordSet;
use rustc_errors::codes::*;
use rustc_errors::{
pluralize, struct_span_code_err, Applicability, Diag, ErrorGuaranteed, StashKey, StringPart,
pluralize, struct_span_code_err, Applicability, Diag, ErrorGuaranteed, MultiSpan, StashKey,
StringPart,
};
use rustc_hir::def::Namespace;
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_hir::def_id::{DefId, LocalDefId, LOCAL_CRATE};
use rustc_hir::intravisit::Visitor;
use rustc_hir::{self as hir, LangItem, Node};
use rustc_infer::infer::{InferOk, TypeTrace};
Expand Down Expand Up @@ -1624,9 +1625,131 @@ impl<'a, 'tcx> TypeErrCtxt<'a, '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 `do_not_recommend` items
.filter(|def_id| {
!self
.tcx
.has_attrs_with_path(*def_id, &[sym::diagnostic, sym::do_not_recommend])
})
// 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::<Vec<Span>>().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()),
],
);
}
err.help("you can use `cargo tree` to explore your dependency tree");
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);

Expand Down Expand Up @@ -1798,43 +1921,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, '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 `do_not_recommend` items
.filter(|def_id| {
!self
.tcx
.has_attrs_with_path(*def_id, &[sym::diagnostic, sym::do_not_recommend])
})
// 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.
Expand Down
6 changes: 6 additions & 0 deletions tests/run-make/crate-loading/multiple-dep-versions-1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#![crate_name = "dependency"]
#![crate_type = "rlib"]
pub struct Type;
pub trait Trait {}
impl Trait for Type {}
pub fn do_something<X: Trait>(_: X) {}
6 changes: 6 additions & 0 deletions tests/run-make/crate-loading/multiple-dep-versions-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#![crate_name = "dependency"]
#![crate_type = "rlib"]
pub struct Type(pub i32);
pub trait Trait {}
impl Trait for Type {}
pub fn do_something<X: Trait>(_: X) {}
8 changes: 8 additions & 0 deletions tests/run-make/crate-loading/multiple-dep-versions.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
extern crate dep_2_reexport;
extern crate dependency;
use dep_2_reexport::do_something;
use dependency::Type;

fn main() {
do_something(Type);
}
27 changes: 27 additions & 0 deletions tests/run-make/crate-loading/rmake.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
//@ only-linux
//@ ignore-wasm32
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: does this only fail on wasm32, or does it also fail on wasm 64 targets? We could run test-various and x86_64-gnu-llvm-17 try jobs to double check.

//@ ignore-wasm64

use run_make_support::rfs::copy;
use run_make_support::{assert_contains, rust_lib_name, rustc};

fn main() {
rustc().input("multiple-dep-versions-1.rs").run();
rustc().input("multiple-dep-versions-2.rs").extra_filename("2").metadata("2").run();

let out = rustc()
.input("multiple-dep-versions.rs")
.extern_("dependency", rust_lib_name("dependency"))
.extern_("dep_2_reexport", rust_lib_name("dependency2"))
.run_fail()
.assert_stderr_contains(
"you have multiple different versions of crate `dependency` in your dependency graph",
)
.assert_stderr_contains(
"two types coming from two different versions of the same crate are different types \
even if they look the same",
)
.assert_stderr_contains("this type doesn't implement the required trait")
.assert_stderr_contains("this type implements the required trait")
.assert_stderr_contains("this is the required trait");
}
Loading