Allow built-in traits implementation and add support for deriving

[flaper87]

This is a first patch towards an opt-in built-in trait world. This patch removes the restriction on built-in traits and allows such traits to be derived.

[RFC#3]

cc #13231

@nikomatsakis r?

[alexcrichton]

(travis appears to have some legitimate errors)

[flaper87]

@alexcrichton r?

[huonw]

This would be better as s => cx.span_bug(span, format!("expected built-in trait name but found {}", s)) to assist with debugging.

[huonw]

Also, it's a little weird to have the words duplicated: why not replace this whole let statement with

let name = mitem.name().get();
// verify we're handling a built-in trait.
match name {
    "Copy" | "Send" | "Share" => {}
    s => cx.span_bug(...)
}

let trait_def = ...;

[huonw]

Shouldn't there be a few more tests? e.g. a test with an explicit implementation (not just deriving), and a failing test for an explicit implementation/deriving on a type that has non-Send/Copy/Share contents?

[nikomatsakis]

@huonw the failing tests, at least, will come later. This is just a first step PR. A test for an explicit impl isn't a bad idea, but I figured that we'll have plenty of those later when the real PRs start coming that actually implement the full semantics.

[nikomatsakis]

(though we should definitely fix the potential ICEs you pointed out)

[huonw]

I do think my suggested code was better than this format; what's the reasoning for this being different to all other #[deriving]s? (Also, this still has the problem of repeating the trait names twice.)

[flaper87]

I'm really not sure what you mean with this being different to the other
derivings. The code you suggested used a mitem.name() and there's no such
method in mitem

Unless you meant something else, I'm really not following.

[huonw]

I'm really not sure what you mean with this being different to the other derivings

No other derivings have an error when used like #[deriving(Eq(int))] etc. I guess it's not a particular problem to have one here, although really all derivings should be updated to not pass through these silent errors.

The code you suggested used a mitem.name() and there's no such method in mitem

Oh, sorry, you have to bring http://static.rust-lang.org/doc/master/syntax/attr/trait.AttrMetaMethods.html into scope.

It would've been nice to tell me that my suggestion didn't work, rather than just ignoring it. In any case, the other part of the suggestion (reducing the duplication of trait names) can be with or without talking the whole thing.

[flaper87]

I guess it's not a particular problem to have one here, although really all derivings should be updated to not pass through these silent errors.

Agreed... It'd be better to have an error raised from the other derivings too.

Oh, sorry, you have to bring http://static.rust-lang.org/doc/master/syntax/attr/trait.AttrMetaMethods.html into scope.

Oh, this makes sense. Thanks.

It would've been nice to tell me that my suggestion didn't work, rather than just ignoring it. In any case, the other part of the suggestion (reducing the duplication of trait names) can be with or without talking the whole thing.

Please, always assume good faith from people's actions. I didn't ignore your suggestion and I couldn't comment back mentioning the issues I had.

I'll address your comment, thanks for the review :)

[flaper87]

The reason why I duplicated the trait names is because with the new treatment of @ pointers and the fact that Path just takes &'a str, it throws a bunch of lifetime errors. I tried to workaround this with no luck. if you've a better recommendation, I'd be very happy to use it.

Either way, I'd really like to move this patch forward, it's blocking the work on opt-in traits now.

[flaper87]

There's probably a bug in the new @-pointers lifetime management. This fails:

let n = mitem.name();
let name = n.get();

/home/flaper87/workspace/personal/rust/opt-in-kinds/src/libsyntax/ext/deriving/bounds.rs:24:16: 24:18 error: `mi` does not live long enough
/home/flaper87/workspace/personal/rust/opt-in-kinds/src/libsyntax/ext/deriving/bounds.rs:24     let name = mi.get();
                                                                                                           ^~
note: reference must be valid for the static lifetime...
/home/flaper87/workspace/personal/rust/opt-in-kinds/src/libsyntax/ext/deriving/bounds.rs:21:45: 55:2 note: ...but borrowed value is only valid for the block at 21:44
/home/flaper87/workspace/personal/rust/opt-in-kinds/src/libsyntax/ext/deriving/bounds.rs:21                              push: |@Item|) {
/home/flaper87/workspace/personal/rust/opt-in-kinds/src/libsyntax/ext/deriving/bounds.rs:22 
/home/flaper87/workspace/personal/rust/opt-in-kinds/src/libsyntax/ext/deriving/bounds.rs:23     let mi = mitem.name();
/home/flaper87/workspace/personal/rust/opt-in-kinds/src/libsyntax/ext/deriving/bounds.rs:24     let name = mi.get();
/home/flaper87/workspace/personal/rust/opt-in-kinds/src/libsyntax/ext/deriving/bounds.rs:25     
/home/flaper87/workspace/personal/rust/opt-in-kinds/src/libsyntax/ext/deriving/bounds.rs:26     match name {

[huonw]

@nikomatsakis oh, ok; I'm looking forward to it.

[alexcrichton]

It sounds like there are some small incremental improvements that can be made, but I have r+'d this because this will be a huge language change and I'd like to get it underway as soon as possible.