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

Allow built-in traits implementation and add support for deriving #13868

Merged
merged 2 commits into from
May 3, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 1 addition & 8 deletions src/librustc/middle/typeck/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -631,14 +631,7 @@ pub fn convert(ccx: &CrateCtxt, it: &ast::Item) {
parent_visibility);

for trait_ref in opt_trait_ref.iter() {
let trait_ref = instantiate_trait_ref(ccx, trait_ref, selfty);

// Prevent the builtin kind traits from being manually implemented.
if tcx.lang_items.to_builtin_kind(trait_ref.def_id).is_some() {
tcx.sess.span_err(it.span,
"cannot provide an explicit implementation \
for a builtin kind");
}
instantiate_trait_ref(ccx, trait_ref, selfty);
}
},
ast::ItemTrait(ref generics, _, _, ref trait_methods) => {
Expand Down
46 changes: 46 additions & 0 deletions src/libsyntax/ext/deriving/bounds.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use ast::{MetaItem, MetaWord, Item};
use codemap::Span;
use ext::base::ExtCtxt;
use ext::deriving::generic::*;

pub fn expand_deriving_bound(cx: &mut ExtCtxt,
span: Span,
mitem: @MetaItem,
item: @Item,
push: |@Item|) {

let name = match mitem.node {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 {

MetaWord(ref tname) => {
match tname.get() {
"Copy" => "Copy",
"Send" => "Send",
"Share" => "Share",
ref tname => cx.span_bug(span,
format!("expected built-in trait name but found {}",
*tname))
}
},
_ => return cx.span_err(span, "unexpected value in deriving, expected a trait")
};

let trait_def = TraitDef {
span: span,
attributes: Vec::new(),
path: Path::new(vec!("std", "kinds", name)),
additional_bounds: Vec::new(),
generics: LifetimeBounds::empty(),
methods: vec!()
};

trait_def.expand(cx, mitem, item, push)
}
5 changes: 5 additions & 0 deletions src/libsyntax/ext/deriving/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use ast::{Item, MetaItem, MetaList, MetaNameValue, MetaWord};
use ext::base::ExtCtxt;
use codemap::Span;

pub mod bounds;
pub mod clone;
pub mod encodable;
pub mod decodable;
Expand Down Expand Up @@ -90,6 +91,10 @@ pub fn expand_meta_deriving(cx: &mut ExtCtxt,

"FromPrimitive" => expand!(primitive::expand_deriving_from_primitive),

"Send" => expand!(bounds::expand_deriving_bound),
"Share" => expand!(bounds::expand_deriving_bound),
"Copy" => expand!(bounds::expand_deriving_bound),

ref tname => {
cx.span_err(titem.span, format!("unknown \
`deriving` trait: `{}`", *tname));
Expand Down
19 changes: 0 additions & 19 deletions src/test/compile-fail/cant-implement-builtin-kinds.rs

This file was deleted.

17 changes: 17 additions & 0 deletions src/test/compile-fail/deriving-bounds.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

//NOTE: Remove in the next snapshot
#[cfg(not(stage0))]
#[deriving(Share(Bad),Send,Copy)]
//~^ ERROR unexpected value in deriving, expected a trait
struct Test;

pub fn main() {}
16 changes: 16 additions & 0 deletions src/test/run-pass/deriving-bounds.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

//NOTE: Remove in the next snapshot
#[cfg(not(stage0))]
#[deriving(Share,Send,Copy)]
struct Test;

pub fn main() {}