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

Add version mismatch help message for unimplemented trait #66561

Merged
merged 3 commits into from
Nov 26, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions src/librustc/traits/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ use std::fmt;
use syntax::ast;
use syntax::symbol::{sym, kw};
use syntax_pos::{DUMMY_SP, Span, ExpnKind, MultiSpan};
use rustc::hir::def_id::LOCAL_CRATE;

use rustc_error_codes::*;

Expand Down Expand Up @@ -799,6 +800,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
self.suggest_fn_call(&obligation, &mut err, &trait_ref, points_at_arg);
self.suggest_remove_reference(&obligation, &mut err, &trait_ref);
self.suggest_semicolon_removal(&obligation, &mut err, span, &trait_ref);
self.note_version_mismatch(&mut err, &trait_ref);

// Try to report a help message
if !trait_ref.has_infer_types() &&
Expand Down Expand Up @@ -1050,6 +1052,43 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
err.emit();
}

/// If the `Self` type of the unsatisfied trait `trait_ref` implements a trait
/// with the same path as `trait_ref`, a help message about
/// a probable version mismatch is added to `err`
fn note_version_mismatch(
&self,
err: &mut DiagnosticBuilder<'_>,
trait_ref: &ty::PolyTraitRef<'tcx>,
) {
let get_trait_impl = |trait_def_id| {
let mut trait_impl = None;
self.tcx.for_each_relevant_impl(trait_def_id, trait_ref.self_ty(), |impl_def_id| {
if trait_impl.is_none() {
trait_impl = Some(impl_def_id);
}
});
trait_impl
};
let required_trait_path = self.tcx.def_path_str(trait_ref.def_id());
let all_traits = self.tcx.all_traits(LOCAL_CRATE);
let traits_with_same_path: std::collections::BTreeSet<_> = all_traits
.iter()
.filter(|trait_def_id| **trait_def_id != trait_ref.def_id())
.filter(|trait_def_id| self.tcx.def_path_str(**trait_def_id) == required_trait_path)
.collect();
for trait_with_same_path in traits_with_same_path {
if let Some(impl_def_id) = get_trait_impl(*trait_with_same_path) {
let impl_span = self.tcx.def_span(impl_def_id);
err.span_help(impl_span, "trait impl with same name found");
let trait_crate = self.tcx.crate_name(trait_with_same_path.krate);
let crate_msg = format!(
"Perhaps two different versions of crate `{}` are being used?",
trait_crate
);
err.note(&crate_msg);
}
}
}
fn suggest_restricting_param_bound(
&self,
err: &mut DiagnosticBuilder<'_>,
Expand Down
12 changes: 5 additions & 7 deletions src/test/ui/traits/auxiliary/crate_a1.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
pub struct Foo;
pub trait Bar {}

pub trait Bar{}
pub fn try_foo(x: impl Bar) {}

pub fn bar() -> Box<Bar> {
TimoFreiberg marked this conversation as resolved.
Show resolved Hide resolved
unimplemented!()
pub struct ImplementsTraitForUsize<T> {
_marker: std::marker::PhantomData<T>,
}


pub fn try_foo(x: Foo){}
pub fn try_bar(x: Box<Bar>){}
impl Bar for ImplementsTraitForUsize<usize> {}
13 changes: 13 additions & 0 deletions src/test/ui/traits/auxiliary/crate_a2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
pub struct Foo;

pub trait Bar {}

impl Bar for Foo {}

pub struct DoesNotImplementTrait;

pub struct ImplementsWrongTraitConditionally<T> {
_marker: std::marker::PhantomData<T>,
}

impl Bar for ImplementsWrongTraitConditionally<isize> {}
55 changes: 55 additions & 0 deletions src/test/ui/traits/trait-bounds-same-crate-name.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// aux-build:crate_a1.rs
// aux-build:crate_a2.rs
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's one new test for the case where the diagnostic is correct. Do we also want testcases that verify that the diagnostic is not emitted?
E.g. there exists a trait with same path but it's not implemented for the struct.

Copy link
Contributor

Choose a reason for hiding this comment

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

Negative tests are always good to have, particularly because incorrect suggestions are sometimes worse than no suggestions. Can you create one for the case you mention?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm adding a few cases:

  • A type that doesn't implement any of the two traits
  • A type that doesn't implement any of the two traits, but a different variant of the type implements the wrong trait
  • A type that doesn't implement any of the two traits, but a different variant of the type implements the correct trait


// Issue 22750
// This tests the extra help message reported when a trait bound
// is not met but the struct implements a trait with the same path.

fn main() {
let foo = {
extern crate crate_a2 as a;
a::Foo
};

let implements_no_traits = {
extern crate crate_a2 as a;
a::DoesNotImplementTrait
};

let other_variant_implements_mismatched_trait = {
extern crate crate_a2 as a;
a::ImplementsWrongTraitConditionally { _marker: std::marker::PhantomData::<isize> }
};

let other_variant_implements_correct_trait = {
extern crate crate_a1 as a;
a::ImplementsTraitForUsize { _marker: std::marker::PhantomData::<isize> }
};

{
extern crate crate_a1 as a;
a::try_foo(foo);
//~^ ERROR E0277
//~| trait impl with same name found
//~| Perhaps two different versions of crate `crate_a2`

// We don't want to see the "version mismatch" help message here
// because `implements_no_traits` has no impl for `Foo`
a::try_foo(implements_no_traits);
//~^ ERROR E0277

// We don't want to see the "version mismatch" help message here
// because `other_variant_implements_mismatched_trait`
// does not have an impl for its `<isize>` variant,
// only for its `<usize>` variant.
a::try_foo(other_variant_implements_mismatched_trait);
//~^ ERROR E0277

// We don't want to see the "version mismatch" help message here
// because `ImplementsTraitForUsize` only has
// impls for the correct trait where the path is not misleading.
a::try_foo(other_variant_implements_correct_trait);
//~^ ERROR E0277
//~| the following implementations were found:
}
}
64 changes: 64 additions & 0 deletions src/test/ui/traits/trait-bounds-same-crate-name.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
error[E0277]: the trait bound `main::a::Foo: main::a::Bar` is not satisfied
--> $DIR/trait-bounds-same-crate-name.rs:31:20
|
LL | a::try_foo(foo);
| ^^^ the trait `main::a::Bar` is not implemented for `main::a::Foo`
|
::: $DIR/auxiliary/crate_a1.rs:3:24
|
LL | pub fn try_foo(x: impl Bar) {}
| --- required by this bound in `main::a::try_foo`
|
help: trait impl with same name found
--> $DIR/auxiliary/crate_a2.rs:5:1
|
LL | impl Bar for Foo {}
| ^^^^^^^^^^^^^^^^^^^
= note: Perhaps two different versions of crate `crate_a2` are being used?

error[E0277]: the trait bound `main::a::DoesNotImplementTrait: main::a::Bar` is not satisfied
--> $DIR/trait-bounds-same-crate-name.rs:38:20
|
LL | a::try_foo(implements_no_traits);
| ^^^^^^^^^^^^^^^^^^^^ the trait `main::a::Bar` is not implemented for `main::a::DoesNotImplementTrait`
|
::: $DIR/auxiliary/crate_a1.rs:3:24
|
LL | pub fn try_foo(x: impl Bar) {}
| --- required by this bound in `main::a::try_foo`

error[E0277]: the trait bound `main::a::ImplementsWrongTraitConditionally<isize>: main::a::Bar` is not satisfied
--> $DIR/trait-bounds-same-crate-name.rs:45:20
|
LL | a::try_foo(other_variant_implements_mismatched_trait);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `main::a::Bar` is not implemented for `main::a::ImplementsWrongTraitConditionally<isize>`
|
::: $DIR/auxiliary/crate_a1.rs:3:24
|
LL | pub fn try_foo(x: impl Bar) {}
| --- required by this bound in `main::a::try_foo`
|
help: trait impl with same name found
--> $DIR/auxiliary/crate_a2.rs:13:1
|
LL | impl Bar for ImplementsWrongTraitConditionally<isize> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= note: Perhaps two different versions of crate `crate_a2` are being used?

error[E0277]: the trait bound `main::a::ImplementsTraitForUsize<isize>: main::a::Bar` is not satisfied
--> $DIR/trait-bounds-same-crate-name.rs:51:20
|
LL | a::try_foo(other_variant_implements_correct_trait);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `main::a::Bar` is not implemented for `main::a::ImplementsTraitForUsize<isize>`
|
::: $DIR/auxiliary/crate_a1.rs:3:24
|
LL | pub fn try_foo(x: impl Bar) {}
| --- required by this bound in `main::a::try_foo`
|
= help: the following implementations were found:
<main::a::ImplementsTraitForUsize<usize> as main::a::Bar>

error: aborting due to 4 previous errors

For more information about this error, try `rustc --explain E0277`.