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.

Sign up for GitHub

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

Point out the known type when field doesn't satisfy bound #38150

Merged
merged 2 commits into from
Dec 21, 2016
Merged

Point out the known type when field doesn't satisfy bound #38150

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ef09db0
Point out the known type when field doesn't satisfy bound
estebank Dec 4, 2016
5c13041
review comments
estebank Dec 12, 2016
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
30 changes: 23 additions & 7 deletions src/librustc/traits/error_reporting.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -487,13 +487,29 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
} else {
let trait_ref = trait_predicate.to_poly_trait_ref();

let mut err = struct_span_err!(self.tcx.sess, span, E0277,
"the trait bound `{}` is not satisfied",
trait_ref.to_predicate());
err.span_label(span, &format!("the trait `{}` is not implemented \
for `{}`",
trait_ref,
trait_ref.self_ty()));
let (post_message, pre_message) =
if let ObligationCauseCode::BuiltinDerivedObligation(ref data)
= obligation.cause.code {
let parent_trait_ref = self.resolve_type_vars_if_possible(
Copy link
Member

Choose a reason for hiding this comment

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

Missing an indent level here.

Copy link
Contributor Author

@estebank estebank Dec 8, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Is it? the if let is part of the previous line, the only reason is below is because I couldn't get to fit nicely in under 100 chars. In this were to fit in one line it would be:

let (post_message, pre_message) = if let ObligationCauseCode::BuiltinDerivedObligation(ref data) = obligation.cause.code {

I don't see why then line 493 would have to have double indentation. :)

I agree that it is slightly confusing as it is now, but haven't come up with a better way of presenting this.

Copy link
Member

Choose a reason for hiding this comment

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

Oh indeed, you're absolutely right, my bad!

Copy link
Contributor

Choose a reason for hiding this comment

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

one better way might be using match :)

let (post_message, pre_message) = match obligation.cause.code {
    ObligationCauseCode::BuiltinDerivedObligation(ref data) => {
        let parent_trait_ref = self.resolve_type_vars_if_possible(
        ...
    }
    _ => {
        (String::new(), String::new())
    }
};

&data.parent_trait_ref);
(format!(" in `{}`", parent_trait_ref.0.self_ty()),
Copy link
Contributor

Choose a reason for hiding this comment

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

One concern I have is that this only goes up one level, but there are similar cases where multiple levels are needed. For example, it'd be nice if this patch addressed this example: https://is.gd/BZjfP6

The current output is

error[E0277]: the trait bound `*const u8: std::marker::Send` is not satisfied
  --> <anon>:16:5
   |
16 |     is_send::<Foo>();
   |     ^^^^^^^^^^^^^^ trait `*const u8: std::marker::Send` not satisfied
   |
   = note: `*const u8` cannot be sent between threads safely
   = note: required because it appears within the type `Baz`
   = note: required because it appears within the type `Bar`
   = note: required because it appears within the type `Foo`
   = note: required by `is_send`

and I think your suggested approach would be a big improvement:

error[E0277]: within `Foo`, the trait bound `*const u8: std::marker::Send` is not satisfied
  --> <anon>:16:5
   |
16 |     is_send::<Foo>();
   |     ^^^^^^^^^^^^^^ trait `*const u8: std::marker::Send` not satisfied
   |
   = note: `*const u8` cannot be sent between threads safely
   = note: required because it appears within the type `Baz`
   = note: required because it appears within the type `Bar`
   = note: required because it appears within the type `Foo`
   = note: required by `is_send`

But I believe your patch as is will print "within Baz" instead.

should be easy enough though -- just have to iterate on the cause chain rather than just going up one level.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added test for new case and a recursive search up the cause chain.

format!("within `{}`, ", parent_trait_ref.0.self_ty()))
} else {
(String::new(), String::new())
};
let mut err = struct_span_err!(
self.tcx.sess,
span,
E0277,
"the trait bound `{}` is not satisfied{}",
trait_ref.to_predicate(),
post_message);
err.span_label(span,
&format!("{}the trait `{}` is not \
implemented for `{}`",
pre_message,
trait_ref,
trait_ref.self_ty()));

// Try to report a help message

Expand Down
9 changes: 9 additions & 0 deletions src/test/compile-fail/E0277.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use std::path::Path;

trait Foo {
fn bar(&self);
}
Expand All @@ -16,6 +18,13 @@ fn some_func<T: Foo>(foo: T) {
foo.bar();
}

fn f(p: Path) { }
//~^ ERROR the trait bound `[u8]: std::marker::Sized` is not satisfied in `std::path::Path`
//~| NOTE within `std::path::Path`, the trait `std::marker::Sized` is not implemented for `[u8]`
//~| NOTE `[u8]` does not have a constant size known at compile-time
//~| NOTE required because it appears within the type `std::path::Path`
//~| NOTE all local variables must have a statically known size

fn main() {
some_func(5i32);
//~^ ERROR the trait bound `i32: Foo` is not satisfied
Expand Down