-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Handle attributes on cross-crate tuple-structs correctly #11932
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
Conversation
@@ -1 +1 @@ | |||
Subproject commit e1dabb48f0f898d1a808b3de3a26f5ee3735c7dd | |||
Subproject commit 535989a92ce1f6f6488c94a2c8f4ed438349f162 |
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.
Rebase fallout?
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 believe so - how do I fix it?
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.
(Nevermind - git reset
to the rescue!)
Nice catch! And while #11741 isn't officially on the 1.0 milestone it probably should be! This is a great bugfix for the real usage of stability attributes in the future. |
pub fn get_item_attrs(cstore: @cstore::CStore, | ||
def_id: ast::DefId, | ||
f: |~[@ast::MetaItem]|) { | ||
let def_id = get_tuple_struct_def_from_ctor(cstore, def_id).unwrap_or(def_id); |
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.
Could you add a comment here explaining why it's calling this function? It seems odd in a get_item
function it's calling something which is expecting a tuple_struct_def
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.
Maybe we could call it get_tuple_struct_def_if_ctor(..)
to (attempt to) indicate that it only gets the def if it's actually a tuple struct constructor.
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'd be down with that. Comments are still welcome too :)
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.
Of course (I didn't mean to imply that there shouldn't be a comment).
Just a minor comment for future readers, otherwise r=me (feel free to comment when updated) |
The test failures aren't your fault, I'm trying to fix mac-64 and I'll retry this when I think I have a fix. |
Fixes #11741
Added tests and removed xfail-fast from run-pass/simd-experimental which is now fixed (see #11738).