rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.79k stars 12.77k forks source link

`#[deriving(Clone)]` et al. should ask for `Clone` bounds on fields, not on generic parameters #19839

Closed japaric closed 9 years ago

japaric commented 9 years ago

For example:

#[deriving(Clone)]
struct Filter<A, I, P> where I: Iterator<A>, P: FnMut(&A) -> bool {
    it: I,
    p: P,
}

expands to:

impl<A: Clone, I: Clone, P: Clone> Clone for Filter<A, I, P> where /* .. */ { /* .. */ }

The A: Clone bound is incorrect. Instead deriving should expand to:

impl<A, I, P> Clone for Filter<A, I, P> where
    I: Clone,
    P: Clone,
    /* .. */ { /* .. */ }

This requires generalized where clauses to handle more complex fields like &'a T: Clone.

@nikomatsakis Are where clauses like &'a int: Clone (note: no generic parameter) going to be allowed? If not, writing the syntax extension is going to be harder.

jroesch commented 9 years ago

@japaric currently we accept both forms but as Niko pointed out the RFC technically disallows the bounding of arbitrary types, so the second bound (&'a int: Clone) is currently valid, but technically illegal. I don't see any reason why it is not possible to have the second form, @nikomatsakis would have to fill you in on his thinking. There is a corresponding issue for fixing this behavior: #20019

eddyb commented 9 years ago

@jroesch the good news is that fixing deriving doesn't need to care about whether or not types that don't contain type params can be bounded. Because it can simply not bound those.

goffrie commented 9 years ago

FWIW, this bug is basically the same as #7671.

alexcrichton commented 9 years ago

I believe @goffrie is right in that this is a dupe of #7671

apasel422 commented 9 years ago

This isn't really a dupe of #7671, because this is specifically about fields. For example, the following does not work:

foo.rs:

#[derive(Clone)]
pub struct Foo<'a, T: 'a>(&'a T);

pub struct Bar;

fn main() {
    let foo = Foo(&Bar);
    foo.clone();
}

Output:

> rustc --version
rustc 1.0.0-nightly (199bdcfef 2015-03-26) (built 2015-03-27)
> rustc foo.rs
foo.rs:8:9: 8:16 error: type `Foo<'_, Bar>` does not implement any method in scope named `clone`
foo.rs:8     foo.clone();
                 ^~~~~~~
foo.rs:8:16: 8:16 help: methods from traits can only be called if the trait is implemented and in scope; the following trait defines a method `clone`, perhaps you need to implement it:
foo.rs:8:16: 8:16 help: candidate #1: `core::clone::Clone`

Supporting this would reduce a lot of boilerplate, especially for by-reference iterators.

ftxqxd commented 9 years ago

I think this should be reopened—even though #7671 is fixed, @apasel422’s example still doesn’t work, although it should.

Eh2406 commented 7 years ago

The FIXME refers to this issue: https://github.com/rust-lang/rust/blob/master/src/liballoc/binary_heap.rs#L929

If this is fixed, we can remove the FIXME. If we can't remove the FIXME, than we should reopen the issue.

eddyb commented 7 years ago

@Eh2406 I believe the current issue is #26925.