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

Better error message for transitive req of #[deriving]. (Also, how to use deriving here?) #9607

Closed
pnkfelix opened this issue Sep 29, 2013 · 19 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-proc-macros Area: Procedural macros C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@pnkfelix
Copy link
Member

pnkfelix commented Sep 29, 2013

Updated test (play):

use std::marker::PhantomData;

struct Foo<T> {
  marker: PhantomData<T>
}

trait Bar {}

impl<T: Bar> Clone for Foo<T> {
    fn clone(&self) -> Foo<T> { Foo {marker: PhantomData} }
}

#[derive(Clone)]
struct Baz<T> {
    x: Foo<T>
}

fn main() {}

(I'll try to update the above as necessary.)

The error message is pretty good now, actually; the only thing I'd want beyond this is for the diagnostic to point out that Baz is trying to automatically derive Clone (which is why it is missing the necessary T: Bar bound), and that a manual implementation with that bound would work.

Original bug report follows.


use std::hashmap::HashSet;

#[deriving(Clone)]
struct FollowSet<T> {
  right: HashSet<T>,
  can_end: bool,
}

Error message from rustc:

% rustc --lib /tmp/f.rs
/tmp/f.rs:3:11: 3:16 error: failed to find an implementation of trait std::to_bytes::IterBytes for T
/tmp/f.rs:3 #[deriving(Clone)]
                       ^~~~~

I saw the above, and said "why does it matter about whether I implement IterBytes when I want to implement Clone here. (Then I read source code for hashmap.rs and saw that we actually build up the new table from scratch, rather than copying the existing underlying representation directly; so that explains why one needs it.)

Anyway, it would be good if the error message actually pointed us at the use of HashSet<T> in the definition of struct FollowSet above.


Oh, also: Is there actually a way for one to use deriving here for this? As far as I can tell, I'm going to be forced to manually implement Clone. (Its not hard to do, just a little surprising.)


(not a true dupe of #7621 since that discussing errors when deriving Clone and constituents do not implement Clone. Here, we determine that the constituent does implement Clone, if some it implements some other trait as well. But its possible that a solution for one would also resolve the other.)

@huonw
Copy link
Member

huonw commented Sep 29, 2013

I don't think it's possible to derive here :( #7229 would be an avenue for a solution, but it seems people aren't particularly keen on that.

@reem
Copy link
Contributor

reem commented Sep 15, 2014

Triage: the above snippet compiles cleanly now, should be closed.

@huonw
Copy link
Member

huonw commented Sep 15, 2014

The underlying issue still exists, that snippet compiles due to unrelated library changes.

struct Foo<T>;

trait Bar {}

impl<T: Bar> Clone for Foo<T> {
    fn clone(&self) -> Foo<T> { Foo }
}

#[deriving(Clone)]
struct Baz<T> {
    x: Foo<T>
}

fn main() {}
<anon>:11:5: 11:14 error: failed to find an implementation of trait Bar for T
<anon>:11     x: Foo<T>
              ^~~~~~~~~
note: in expansion of #[deriving]
<anon>:9:1: 9:19 note: expansion site
<anon>:11:5: 11:14 error: failed to find an implementation of trait Bar for T
<anon>:11     x: Foo<T>
              ^~~~~~~~~

@reem
Copy link
Contributor

reem commented Sep 15, 2014

I actually have run into this issue before and have had to manually implement Clone.

I think that this issue is actually unrelated to the stated issue because it has to do with the way that the generated impl for Clone automatically binds all type parameters with Clone, which isn't fixable without more type information at the syntax extension stage.

@huonw
Copy link
Member

huonw commented Sep 15, 2014

It is definitely related.

The contained value Foo needs T: Bar to be Clone while #[deriving] only bounds with Clone, not with T: Bar + Clone (in many circumstances the bounds will be Clone + some other traits, my example above just happened to not need it), but the error message does not make this obvious at all.

@reem
Copy link
Contributor

reem commented Sep 15, 2014

Ah, I see now. Does deriving play nice with struct bounds? i.e. if I did struct Baz<T: Bar> { x: Foo<T> } for instance, would that work?

@huonw
Copy link
Member

huonw commented Sep 15, 2014

Yes it looks like it does, but that doesn't solve the general case, since Cloneing may require more specific behaviour than the default. (Meaning this issue is now lower priority, IMO.)

@flaper87
Copy link
Contributor

Triage bump: Updated test:

use std::marker::{MarkerTrait, PhantomData};

struct Foo<T> {
  marker: PhantomData<T>
}

trait Bar: MarkerTrait {}

impl<T: Bar> Clone for Foo<T> {
    fn clone(&self) -> Foo<T> { Foo {marker: PhantomData} }
}

#[derive(Clone)]
struct Baz<T> {
    x: Foo<T>
}

fn main() {}

Output:

test.rs:15:5: 15:14 error: the trait `Bar` is not implemented for the type `T` [E0277]
test.rs:15     x: Foo<T>
               ^~~~~~~~~
test.rs:13:10: 13:15 note: in expansion of #[derive_Clone]
test.rs:13:10: 13:15 note: expansion site
error: aborting due to previous error

@bltavares
Copy link
Contributor

Triaging:

Updated the latest snippet by removing the MarkerTrait.

use std::marker::PhantomData;

struct Foo<T> {
  marker: PhantomData<T>
}

trait Bar {}

impl<T: Bar> Clone for Foo<T> {
    fn clone(&self) -> Foo<T> { Foo {marker: PhantomData} }
}

#[derive(Clone)]
struct Baz<T> {
    x: Foo<T>
}

fn main() {}

Output:

<anon>:15:5: 15:14 error: the trait `Bar` is not implemented for the type `T` [E0277]
<anon>:15     x: Foo<T>
              ^~~~~~~~~
<anon>:13:10: 13:15 note: in this expansion of #[derive_Clone] (defined in <anon>)
<anon>:15:5: 15:14 help: see the detailed explanation for E0277
<anon>:15:5: 15:14 note: required by `core::clone::Clone::clone`
error: aborting due to previous error
playpen: application terminated with error code 101

@steveklabnik steveklabnik added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 9, 2017
@oli-obk
Copy link
Contributor

oli-obk commented Jul 17, 2017

Update: The error output is now

error[E0277]: the trait bound `T: Bar` is not satisfied
  --> src/main.rs:15:5
   |
15 |     x: Foo<T>
   |     ^^^^^^^^^ the trait `Bar` is not implemented for `T`
   |
   = help: consider adding a `where T: Bar` bound
   = note: required because of the requirements on the impl of `std::clone::Clone` for `Foo<T>`
   = note: required by `std::clone::Clone::clone`

Which is only missing a note about the derive expansion now.

Related to the original question of why we can't use derive here:

Couldn't the code generated by derive have a where Foo<T>: Clone bound? This also applies to Default and probably others. Afaik serde does this for Serialize and Deserialize.

@steveklabnik
Copy link
Member

Triage: no change.

@pnkfelix
Copy link
Member Author

@oli-obk asked:

Related to the original question of why we can't use derive here:

Couldn't the code generated by derive have a where Foo<T>: Clone bound? This also applies to Default and probably others. Afaik serde does this for Serialize and Deserialize.

Would we be able to make that change backward compatibly? Probably not because it would make existing types that did not previously implement Clone start implementing Clone, which is a breaking change, right?

@estebank
Copy link
Contributor

Probably not because it would make existing types that did not previously implement Clone start implementing Clone, which is a breaking change, right?

It would be interesting implementing the behavior and make a crater run to see if there would be significant impact in the ecosystem.

@estebank
Copy link
Contributor

estebank commented Feb 5, 2020

Triage: current output:

error[E0277]: the trait bound `T: std::clone::Clone` is not satisfied
  --> src/main.rs:15:5
   |
15 |     x: Foo<T>
   |     ^^^^^^^^^
   |     |
   |     expected an implementor of trait `std::clone::Clone`
   |     help: consider borrowing here: `&x: Foo<T>`
   |
   = note: required because of the requirements on the impl of `std::clone::Clone` for `Foo<T>`
   = note: required by `std::clone::Clone::clone`

@estebank estebank added D-confusing Diagnostics: Confusing error or lint that should be reworked. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. labels Feb 5, 2020
@rail-rain
Copy link
Contributor

Update: rustc 1.48 does mention that it's from a derive macro:

error[E0277]: the trait bound `T: Clone` is not satisfied
  --> src/main.rs:15:5
   |
15 |     x: Foo<T>
   |     ^^^^^^^^^ expected an implementor of trait `Clone`
   |
   = note: required because of the requirements on the impl of `Clone` for `Foo<T>`
   = note: required by `clone`
   = note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)

@rylev
Copy link
Member

rylev commented Jul 5, 2021

Triage: the current error message is pretty good IMO. (playground)

error[E0277]: the trait bound `T: Clone` is not satisfied
  --> src/main.rs:15:5
   |
15 |     x: Foo<T>
   |     ^^^^^^^^^ expected an implementor of trait `Clone`
   |
   = note: required because of the requirements on the impl of `Clone` for `Foo<T>`
   = note: required by `clone`
   = note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)

Perhaps the only improvement could be to point to the specific derive macro that is causing this?

error[E0277]: the trait bound `T: Clone` is not satisfied
  --> src/main.rs:15:5
   |
15 |     x: Foo<T>
   |     ^^^^^^^^^ expected an implementor of trait `Clone`
   |
   = note: required because of the requirements on the impl of `Clone` for `Foo<T>`
   = note: required by `clone`
   = note: this error originates from the derive macro `#[derive(Clone]`
   = note: in Nightly builds, run with -Z macro-backtrace for more info

Removing D-confusing and D-invalid-suggestion as I don't believe that's true anymore.

@rylev rylev removed D-confusing Diagnostics: Confusing error or lint that should be reworked. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. labels Jul 5, 2021
@jyn514
Copy link
Member

jyn514 commented Jul 14, 2021

Probably not because it would make existing types that did not previously implement Clone start implementing Clone, which is a breaking change, right?

Why would that be a breaking change? If .clone() ended up being ambiguous because of a Deref impl or something?

@jyn514 jyn514 added A-proc-macros Area: Procedural macros and removed A-syntaxext Area: Syntax extensions labels Jul 14, 2021
@estebank
Copy link
Contributor

estebank commented Jul 20, 2021

With #87225, the output is now

error[E0277]: the trait bound `T: Clone` is not satisfied
   --> f26.rs:15:5
    |
13  | #[derive(Clone)]
    |          ----- in this derive macro expansion
14  | struct Baz<T> {
15  |     x: Foo<T>
    |     ^^^^^^^^^ expected an implementor of trait `Clone`
    |
note: required because of the requirements on the impl of `Clone` for `Foo<T>`
   --> f26.rs:9:14
    |
9   | impl<T: Bar> Clone for Foo<T> {
    |              ^^^^^     ^^^^^^
note: required by `clone`
   --> /Users/ekuber/workspace/rust/library/core/src/clone.rs:121:5
    |
121 |     fn clone(&self) -> Self;
    |     ^^^^^^^^^^^^^^^^^^^^^^^^
    = note: this error originates in the derive macro `Clone` (in Nightly builds, run with -Z macro-backtrace for more info)

Screen Shot 2021-07-19 at 5 43 27 PM

There are details in the wording we could improve and some things we could take out, but I feel all the relevant information is there. @pnkfelix what do you think? Should we close this ticket? (It's always a good day when I get to close a pre-1.0 ticket 😄)

@oli-obk oli-obk closed this as completed Apr 27, 2022
@estebank
Copy link
Contributor

estebank commented Apr 27, 2022

For posterity, this is what it looks like now:

error[E0277]: the trait bound `T: Bar` is not satisfied
  --> src/main.rs:15:5
   |
13 | #[derive(Clone)]
   |          ----- in this derive macro expansion
14 | struct Baz<T> {
15 |     x: Foo<T>
   |     ^^^^^^^^^ the trait `Bar` is not implemented for `T`
   |
note: required because of the requirements on the impl of `Clone` for `Foo<T>`
  --> src/main.rs:9:14
   |
9  | impl<T: Bar> Clone for Foo<T> {
   |              ^^^^^     ^^^^^^
   = note: this error originates in the derive macro `Clone` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider restricting type parameter `T`
   |
14 | struct Baz<T: Bar> {
   |             +++++

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-proc-macros Area: Procedural macros C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests