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.

Sign up for GitHub

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

derive: Avoid emitting provided PartialEq, PartialOrd methods for c-like enums #31977

Merged
merged 3 commits into from
Mar 19, 2016
Merged

derive: Avoid emitting provided PartialEq, PartialOrd methods for c-like enums #31977

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
190af51
derive: Avoid emitting PartialEq::ne for c-like enums
bluss Feb 29, 2016
57e0a7e
derive: Skip PartialEq::ne for any zero-field enum or struct
bluss Feb 29, 2016
edcc02b
derive: Emit only PartialOrd::partial_cmp for simple enums
bluss Mar 1, 2016
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
33 changes: 28 additions & 5 deletions src/libsyntax_ext/deriving/cmp/partial_eq.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,33 @@
use deriving::generic::*;
use deriving::generic::ty::*;

use syntax::ast::{MetaItem, Expr, BinOpKind};
use syntax::ast::{MetaItem, Expr, BinOpKind, ItemKind, VariantData};
use syntax::codemap::Span;
use syntax::ext::base::{ExtCtxt, Annotatable};
use syntax::ext::build::AstBuilder;
use syntax::parse::token::InternedString;
use syntax::ptr::P;

fn is_clike_enum(item: &Annotatable) -> bool {
match *item {
Annotatable::Item(ref item) => {
match item.node {
ItemKind::Enum(ref enum_def, _) => {
enum_def.variants.iter().all(|v|
if let VariantData::Unit(..) = v.node.data {
Copy link
Contributor

Choose a reason for hiding this comment

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

Unit is not necessary, zero fields would be enough.

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean, enum E { V {} } isn't considered "C-like", but it's still amenable to this derive optimization.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, #[derive(PartialEq)] for struct S;/struct S {} can be optimized too.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good points. I'll try to incorporate that. Do I need to pretend zero field tuple variants can exist (they are disallowed)?

Copy link
Contributor

Choose a reason for hiding this comment

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

derive can just use variant_data.fields().is_empty(), caring about empty tuple structs is not its responsibility.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the pointer. I usually don't poke around in these parts!

true
} else {
false
}
)
}
_ => false,
}
}
_ => false,
}
}

pub fn expand_deriving_partial_eq(cx: &mut ExtCtxt,
span: Span,
mitem: &MetaItem,
Expand Down Expand Up @@ -80,17 +100,20 @@ pub fn expand_deriving_partial_eq(cx: &mut ExtCtxt,
} }
}

// avoid defining `ne` if we can
let mut methods = vec![md!("eq", cs_eq)];
if !is_clike_enum(item) {
methods.push(md!("ne", cs_ne));
}

let trait_def = TraitDef {
span: span,
attributes: Vec::new(),
path: path_std!(cx, core::cmp::PartialEq),
additional_bounds: Vec::new(),
generics: LifetimeBounds::empty(),
is_unsafe: false,
methods: vec!(
md!("eq", cs_eq),
md!("ne", cs_ne)
),
methods: methods,
associated_types: Vec::new(),
};
trait_def.expand(cx, mitem, item, push)
Expand Down