From 190af51f303e21e22ea0a4d7dffeb09805d19010 Mon Sep 17 00:00:00 2001 From: Ulrik Sverdrup Date: Mon, 29 Feb 2016 21:27:20 +0100 Subject: [PATCH 1/3] derive: Avoid emitting PartialEq::ne for c-like enums `ne` is completely symmetrical with the method `eq`, and we can save rust code size and compilation time here if we only emit one of them when possible. One case where it's easy to recognize is when it's a C-like enum. Most other cases can not omit ne, because any value field may have a custom PartialEq implementation. --- src/libsyntax_ext/deriving/cmp/partial_eq.rs | 33 +++++++++++++++++--- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/src/libsyntax_ext/deriving/cmp/partial_eq.rs b/src/libsyntax_ext/deriving/cmp/partial_eq.rs index 0150a073b07..dac25112112 100644 --- a/src/libsyntax_ext/deriving/cmp/partial_eq.rs +++ b/src/libsyntax_ext/deriving/cmp/partial_eq.rs @@ -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 { + true + } else { + false + } + ) + } + _ => false, + } + } + _ => false, + } +} + pub fn expand_deriving_partial_eq(cx: &mut ExtCtxt, span: Span, mitem: &MetaItem, @@ -80,6 +100,12 @@ 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(), @@ -87,10 +113,7 @@ pub fn expand_deriving_partial_eq(cx: &mut ExtCtxt, 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) From 57e0a7e5d8872c8fcea47fc20239b8921bda2576 Mon Sep 17 00:00:00 2001 From: Ulrik Sverdrup Date: Mon, 29 Feb 2016 22:15:51 +0100 Subject: [PATCH 2/3] derive: Skip PartialEq::ne for any zero-field enum or struct Also detect unit structs and enums with zero field struct variants. --- src/libsyntax_ext/deriving/cmp/partial_eq.rs | 32 +++++++++----------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/src/libsyntax_ext/deriving/cmp/partial_eq.rs b/src/libsyntax_ext/deriving/cmp/partial_eq.rs index dac25112112..24444c3c39b 100644 --- a/src/libsyntax_ext/deriving/cmp/partial_eq.rs +++ b/src/libsyntax_ext/deriving/cmp/partial_eq.rs @@ -11,30 +11,26 @@ use deriving::generic::*; use deriving::generic::ty::*; -use syntax::ast::{MetaItem, Expr, BinOpKind, ItemKind, VariantData}; +use syntax::ast::{MetaItem, Expr, BinOpKind, ItemKind}; 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 { - true - } else { - false - } - ) - } - _ => false, +fn is_type_without_fields(item: &Annotatable) -> bool { + if let Annotatable::Item(ref item) = *item { + match item.node { + ItemKind::Enum(ref enum_def, _) => { + enum_def.variants.iter().all(|v| v.node.data.fields().is_empty()) } + ItemKind::Struct(ref variant_data, _) => { + variant_data.fields().is_empty() + } + _ => false } - _ => false, + } else { + false } } @@ -101,8 +97,10 @@ pub fn expand_deriving_partial_eq(cx: &mut ExtCtxt, } // avoid defining `ne` if we can + // c-like enums, enums without any fields and structs without fields + // can safely define only `eq`. let mut methods = vec![md!("eq", cs_eq)]; - if !is_clike_enum(item) { + if !is_type_without_fields(item) { methods.push(md!("ne", cs_ne)); } From edcc02bfee262ce8bc3f087d9793ce68d73b1a40 Mon Sep 17 00:00:00 2001 From: Ulrik Sverdrup Date: Tue, 1 Mar 2016 02:27:27 +0100 Subject: [PATCH 3/3] derive: Emit only PartialOrd::partial_cmp for simple enums Using the same logic as for `PartialEq`, when possible define only `partial_cmp` and leave `lt, le, gt, ge` to their default implementations. This works well for c-like enums. --- src/libsyntax_ext/deriving/cmp/partial_eq.rs | 18 +-------------- src/libsyntax_ext/deriving/cmp/partial_ord.rs | 23 +++++++++++++------ src/libsyntax_ext/deriving/generic/mod.rs | 18 +++++++++++++++ 3 files changed, 35 insertions(+), 24 deletions(-) diff --git a/src/libsyntax_ext/deriving/cmp/partial_eq.rs b/src/libsyntax_ext/deriving/cmp/partial_eq.rs index 24444c3c39b..6406ee59a5e 100644 --- a/src/libsyntax_ext/deriving/cmp/partial_eq.rs +++ b/src/libsyntax_ext/deriving/cmp/partial_eq.rs @@ -11,29 +11,13 @@ use deriving::generic::*; use deriving::generic::ty::*; -use syntax::ast::{MetaItem, Expr, BinOpKind, ItemKind}; +use syntax::ast::{MetaItem, Expr, BinOpKind}; 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_type_without_fields(item: &Annotatable) -> bool { - if let Annotatable::Item(ref item) = *item { - match item.node { - ItemKind::Enum(ref enum_def, _) => { - enum_def.variants.iter().all(|v| v.node.data.fields().is_empty()) - } - ItemKind::Struct(ref variant_data, _) => { - variant_data.fields().is_empty() - } - _ => false - } - } else { - false - } -} - pub fn expand_deriving_partial_eq(cx: &mut ExtCtxt, span: Span, mitem: &MetaItem, diff --git a/src/libsyntax_ext/deriving/cmp/partial_ord.rs b/src/libsyntax_ext/deriving/cmp/partial_ord.rs index e857f7d52f9..fafcf0ea960 100644 --- a/src/libsyntax_ext/deriving/cmp/partial_ord.rs +++ b/src/libsyntax_ext/deriving/cmp/partial_ord.rs @@ -67,6 +67,21 @@ pub fn expand_deriving_partial_ord(cx: &mut ExtCtxt, })) }; + // avoid defining extra methods if we can + // c-like enums, enums without any fields and structs without fields + // can safely define only `partial_cmp`. + let methods = if is_type_without_fields(item) { + vec![partial_cmp_def] + } else { + vec![ + partial_cmp_def, + md!("lt", true, false), + md!("le", true, true), + md!("gt", false, false), + md!("ge", false, true) + ] + }; + let trait_def = TraitDef { span: span, attributes: vec![], @@ -74,13 +89,7 @@ pub fn expand_deriving_partial_ord(cx: &mut ExtCtxt, additional_bounds: vec![], generics: LifetimeBounds::empty(), is_unsafe: false, - methods: vec![ - partial_cmp_def, - md!("lt", true, false), - md!("le", true, true), - md!("gt", false, false), - md!("ge", false, true) - ], + methods: methods, associated_types: Vec::new(), }; trait_def.expand(cx, mitem, item, push) diff --git a/src/libsyntax_ext/deriving/generic/mod.rs b/src/libsyntax_ext/deriving/generic/mod.rs index c0237a5d29a..e8954b1a2fc 100644 --- a/src/libsyntax_ext/deriving/generic/mod.rs +++ b/src/libsyntax_ext/deriving/generic/mod.rs @@ -1638,3 +1638,21 @@ pub fn cs_same_method(f: F, } } } + +/// Return true if the type has no value fields +/// (for an enum, no variant has any fields) +pub fn is_type_without_fields(item: &Annotatable) -> bool { + if let Annotatable::Item(ref item) = *item { + match item.node { + ast::ItemKind::Enum(ref enum_def, _) => { + enum_def.variants.iter().all(|v| v.node.data.fields().is_empty()) + } + ast::ItemKind::Struct(ref variant_data, _) => { + variant_data.fields().is_empty() + } + _ => false + } + } else { + false + } +}