-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 { | ||
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) | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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() {} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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() {} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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.)There was a problem hiding this comment.
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 suchmethod in
mitem
Unless you meant something else, I'm really not following.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed... It'd be better to have an error raised from the other
deriving
s too.Oh, this makes sense. Thanks.
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 :)
There was a problem hiding this comment.
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 thatPath
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.
There was a problem hiding this comment.
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: