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

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

Closed
japaric opened this issue Dec 14, 2014 · 8 comments
Labels
A-syntaxext Area: Syntax extensions

Comments

@japaric
Copy link
Member

japaric commented Dec 14, 2014

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
Copy link
Member

jroesch commented Dec 24, 2014

@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
Copy link
Member

eddyb commented Dec 30, 2014

@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
Copy link
Contributor

goffrie commented Feb 6, 2015

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

@alexcrichton
Copy link
Member

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

@apasel422
Copy link
Contributor

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
Copy link
Contributor

ftxqxd commented Jun 1, 2015

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

@Eh2406
Copy link
Contributor

Eh2406 commented Sep 3, 2017

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
Copy link
Member

eddyb commented Sep 3, 2017

@Eh2406 I believe the current issue is #26925.

nivkner added a commit to nivkner/rust that referenced this issue Sep 30, 2017
remove FIXME(rust-lang#13101) since `assert_receiver_is_total_eq` stays.
remove FIXME(rust-lang#19649) now that stability markers render.
remove FIXME(rust-lang#13642) now the benchmarks were moved.
remove FIXME(rust-lang#6220) now that floating points can be formatted.
remove FIXME(rust-lang#18248) and write tests for `Rc<str>` and `Rc<[u8]>`
remove reference to irelevent issues in FIXME(rust-lang#1697, rust-lang#2178...)
update FIXME(rust-lang#5516) to point to getopts issue 7
update FIXME(rust-lang#7771) to point to RFC 628
update FIXME(rust-lang#19839) to point to issue 26925
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-syntaxext Area: Syntax extensions
Projects
None yet
Development

No branches or pull requests

9 participants