Add associated types support for #[derive(...)]

[erickt]

This PR adds support for associated types to the #[derive(...)] syntax extension. In order to do this, it switches over to using where predicates to apply the type constraints. So now this:

typeTrait{typeType;}#[derive(Clone)]structFoo<A>whereA:Trait{a:A,b: <AasTrait>::Type,}

Gets expended into this impl:

impl<A:Clone>CloneforFoo<A>whereA:Trait,
<AasTrait>::Type:Clone,{fnclone(&self) -> Foo<T>{Foo{a:self.a.clone(),b:self.b.clone(),}}}

[rust-highfive]

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

[erickt]

Unfortunately this can't land quite yet. It turns out the visibility checker reports that this is exposing a private type:

structFoo<T>(T);impl<T>CloneforFoo<T>whereT:Clone{ ... }pubstructBar<T>(Foo<T>);impl<T>CloneforBar<T>whereFoo<T>:Clone{ ... }

@nikomatsakis said in #21203 (comment) that the privacy rules are being too strict here and might need to be relaxed.

[alexcrichton]

r? @huonw - you're more familiar with #[derive] internals than I

[nikomatsakis]

I'm not so sure about the privacy thing anymore.

[huonw]

I suspect this should check that path.segments has length 1, or maybe the shadowing rules means it doesn't matter? (i.e. is struct Foo<std> { x: std::option::Option<u8> } legal?)

[erickt]

Drat, you're right, this currently compiles:

modT{pubtypeType = i32;}traitTrait{typeType;}structBaz<T:Trait>{a:T::Type,}

I filed #21301 to get that fixed. It'll probably not be that much of an issue in practice though, as you'd have to be crazy to write that :)

[huonw]

@erickt what's the status of this? Is

Unfortunately this can't land quite yet

still true?

[erickt]

@huonw: Ack! I was on a business trip this past week and forgot all about wrapping up this PR. I'll try to get it done this week :)

[erickt]

@huonw: Should work now, and it passes locally. re-r?

[erickt]

@huonw: updated the PR to also work with phantom types, closing #7671.

[huonw]

This approach is definitely an improvement, but it doesn't solve #7671 completely, e.g.

#[derive(Debug)]// T: Debug is not insertedstructFoo<T>;#[derive(Debug)]// T: Debug is inserted, but T is still phantomstructBar<T>{x:Foo<T>}

---

Also, I think this may "break" the standard library marker types like InvariantType (and Phantom if/when it appears) which are possibly meant to represent that some type behaves like it stores a T, so the relevant traits should only be implemented when T has them implemented. The type parameters to these types are phantom types so #[derive] will no longer insert those bounds after this patch.

That said, I'm not sure that it is necessary to have these restricted implementions.

[goffrie]

I suspect it's not worth trying to solve the general case of #7671. The privacy thing is a real kicker - if a public type Bar<T> contains a private type Foo<T>, then Bar essentially needs to expose the full rules for Foo<T> being Clone-able - so the derive mechanism would need to see what impls exist for Foo<T>, which might depend on even more private types... As a middle ground, maybe we could emit the where Foo<T>: Clone clause when Foo is public (or Bar is private), but use the approximation where T: Clone otherwise?

[erickt]

@goffrie: unfortunately in libsyntax at this point in time we can't see if a type is public or private. I believe we would need typeck in order to have that information.

@huon: good point. Maybe phantom types impls should written by hand.

[erickt]

After talking to @huonw, I feel like there isn't a great solution that works for phantom/marker types at the same time as working for an edge case of associated types. Say we extend @huonw's phantom type example from before, this time adding an associated type that's not using the trait type:

#[derive(Debug)]structFoo<T>;#[derive(Debug)]structBar<T>(Foo<T>);traitTrait{typeType;}#[derive(Debug)]structBaz<T:Trait>(T::Type);

We can either generate an impl that constrains all the type parameters, which is our current approach:

structFoo<T>;impl<T>DebugforFoo<T>whereT:Debug{ ... }structBar<T>(Foo<T>);impl<T>DebugforBar<T>whereT:Debug{ ... }traitTrait{typeType;}structBaz<T:Trait>(T::Type);impl<T>DebugforBaz<T>whereT:Debug,T::Type:Debug{ ... }

Notice that the impl for Baz has an unnecessary T: Debug constraint. Or only the type parameters used in the field:

structFoo<T>;impl<T>DebugforFoo<T>{ ... }structBar<T>(Foo<T>);impl<T>DebugforBar<T>whereT:Debug{ ... }traitTrait{typeType;}structBaz<T:Trait>(T::Type);impl<T>DebugforBaz<T>whereT::Type:Debug{ ... }

Notice that for Bar there is an unnecessary T: Debug constraint because Foo<T> doesn't use T.

Does anyone have an opinion on which approach to use, or if there are some good heuristics here to decide what to do here?

[huonw]

Hm, this will pick up non-generic QPath's too, right? e.g. struct Foo(<Bar as Iterator>::Item);.
(I guess these come up rarely enough that it probably doesn't matter?)

[erickt]

I'll add a pass through the QPath that checks if it references any of the type parameters. If it does, I'll add it to the types to be constrained. If not, i'll skip it. That should protect against this case.

[erickt]

Took a while, but I finally updated the patch to compile on head and address @goffrie's comment.

[tamird]

can this be written as

let field_tys = enum_def.variants.flat_map(|variant| {match variant.node.kind{
ast::VariantKind::TupleVariantKind(ref args) => {
args.iter().map(|arg| arg.ty.clone())}
ast::VariantKind::StructVariantKind(ref args) => {
args.fields.iter().map(|field| field.node.ty.clone())}}}).collect();

❓

[erickt]

@tamird: Unfortunately it cannot because those inner iterators are actually two different types.

[erickt]

@bors: r+ edb33f2

[bors]

⌛ Testing commit edb33f2 with merge e989552...

[bors]

💔 Test failed - auto-mac-64-opt

[Manishearth]

---- [compile-fail] compile-fail/derive-assoc-type-not-impl.rs stdout ----
error: compile-fail test compiled successfully!
status: exit code: 0
command: x86_64-apple-darwin/stage2/bin/rustc /Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-opt/build/src/test/compile-fail/derive-assoc-type-not-impl.rs -L x86_64-apple-darwin/test/compile-fail/ --target=x86_64-apple-darwin -L x86_64-apple-darwin/test/compile-fail/derive-assoc-type-not-impl.stage2-x86_64-apple-darwinlibaux -C prefer-dynamic -o x86_64-apple-darwin/test/compile-fail/derive-assoc-type-not-impl.stage2-x86_64-apple-darwin --cfg rtopt --cfg debug -O -L x86_64-apple-darwin/rt
stdout:
------------------------------------------
------------------------------------------
stderr:
------------------------------------------
------------------------------------------
thread '[compile-fail] compile-fail/derive-assoc-type-not-impl.rs' panicked at 'explicit panic', /Users/rustbuild/src/rust-buildbot/slave/auto-mac-64-opt/build/src/compiletest/runtest.rs:1482
failures:
[compile-fail] compile-fail/derive-assoc-type-not-impl.rs

[bors]

⌛ Testing commit 7971f39 with merge 4c3f968...

[bors]

💔 Test failed - auto-linux-64-x-android-t

[erickt]

@bors: retry

[erickt]

@bors: r+ rollup

[bors]

📌 Commit 9cabe27 has been approved by erickt

[bors]

⌛ Testing commit 9cabe27 with merge 72d0695...

[bors]

💔 Test failed - auto-win-32-opt

[erickt]

@bors: retry

[bors]

⌛ Testing commit 9cabe27 with merge 557d434...

[bors]

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-32-nopt-t, auto-win-32-opt, auto-win-64-nopt-t, auto-win-64-opt

[jendrikw]

I think this is badly documented. A few sentences at https://doc.rust-lang.org/std/clone/trait.Clone.html#derivable would be nice.