From 43e52e4bf144bbd2f366a4f32ac0128136273226 Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Fri, 7 Jun 2013 17:30:38 +1000 Subject: [PATCH 1/4] syntax: move functions from deriving/mod to deriving/generic. These are now only called in generic and can be private. This includes manually inlining/merging some that are called once. --- src/libsyntax/ext/deriving/generic.rs | 221 +++++++++++++++++++--- src/libsyntax/ext/deriving/mod.rs | 255 +------------------------- 2 files changed, 199 insertions(+), 277 deletions(-) diff --git a/src/libsyntax/ext/deriving/generic.rs b/src/libsyntax/ext/deriving/generic.rs index 2e6cac1876b..e386a2cfb53 100644 --- a/src/libsyntax/ext/deriving/generic.rs +++ b/src/libsyntax/ext/deriving/generic.rs @@ -169,7 +169,6 @@ use ast::{enum_def, expr, ident, Generics, struct_def}; use ext::base::ExtCtxt; use ext::build::AstBuilder; -use ext::deriving::*; use codemap::{span,respan}; use opt_vec; @@ -184,18 +183,28 @@ pub fn expand_deriving_generic(cx: @ExtCtxt, _mitem: @ast::meta_item, in_items: ~[@ast::item], trait_def: &TraitDef) -> ~[@ast::item] { - let expand_enum: ExpandDerivingEnumDefFn = - |cx, span, enum_def, type_ident, generics| { - trait_def.expand_enum_def(cx, span, enum_def, type_ident, generics) - }; - let expand_struct: ExpandDerivingStructDefFn = - |cx, span, struct_def, type_ident, generics| { - trait_def.expand_struct_def(cx, span, struct_def, type_ident, generics) - }; - - expand_deriving(cx, span, in_items, - expand_struct, - expand_enum) + let mut result = ~[]; + for in_items.each |item| { + result.push(*item); + match item.node { + ast::item_struct(struct_def, ref generics) => { + result.push(trait_def.expand_struct_def(cx, + span, + struct_def, + item.ident, + generics)); + } + ast::item_enum(ref enum_definition, ref generics) => { + result.push(trait_def.expand_enum_def(cx, + span, + enum_definition, + item.ident, + generics)); + } + _ => () + } + } + result } pub struct TraitDef<'self> { @@ -301,23 +310,71 @@ pub type EnumNonMatchFunc<'self> = impl<'self> TraitDef<'self> { + /** + * + * Given that we are deriving a trait `Tr` for a type `T<'a, ..., + * 'z, A, ..., Z>`, creates an impl like: + * + * impl<'a, ..., 'z, A:Tr B1 B2, ..., Z: Tr B1 B2> Tr for T { ... } + * + * where B1, B2, ... are the bounds given by `bounds_paths`.' + * + */ fn create_derived_impl(&self, cx: @ExtCtxt, span: span, type_ident: ident, generics: &Generics, methods: ~[@ast::method]) -> @ast::item { let trait_path = self.path.to_path(cx, span, type_ident, generics); - let trait_generics = self.generics.to_generics(cx, span, type_ident, generics); + let mut trait_generics = self.generics.to_generics(cx, span, type_ident, generics); + // Copy the lifetimes + for generics.lifetimes.each |l| { + trait_generics.lifetimes.push(copy *l) + }; + // Create the type parameters. + for generics.ty_params.each |ty_param| { + // I don't think this can be moved out of the loop, since + // a TyParamBound requires an ast id + let mut bounds = opt_vec::from( + // extra restrictions on the generics parameters to the type being derived upon + do self.additional_bounds.map |p| { + cx.typarambound(p.to_path(cx, span, type_ident, generics)) + }); + // require the current trait + bounds.push(cx.typarambound(trait_path)); - let additional_bounds = opt_vec::from( - do self.additional_bounds.map |p| { - p.to_path(cx, span, type_ident, generics) - }); + trait_generics.ty_params.push(cx.typaram(ty_param.ident, @bounds)); + } - create_derived_impl(cx, span, - type_ident, generics, - methods, trait_path, - trait_generics, - additional_bounds) + // Create the reference to the trait. + let trait_ref = cx.trait_ref(trait_path); + + // Create the type parameters on the `self` path. + let self_ty_params = do generics.ty_params.map |ty_param| { + cx.ty_ident(span, ty_param.ident) + }; + + let self_lifetime = if generics.lifetimes.is_empty() { + None + } else { + Some(@*generics.lifetimes.get(0)) + }; + + // Create the type of `self`. + let self_type = cx.ty_path(cx.path_all(span, false, ~[ type_ident ], self_lifetime, + opt_vec::take_vec(self_ty_params))); + + let doc_attr = cx.attribute( + span, + cx.meta_name_value(span, + ~"doc", ast::lit_str(@~"Automatically derived."))); + cx.item( + span, + ::parse::token::special_idents::clownshoes_extensions, + ~[doc_attr], + ast::item_impl(trait_generics, + Some(trait_ref), + self_type, + methods.map(|x| *x))) } fn expand_struct_def(&self, cx: @ExtCtxt, @@ -834,6 +891,124 @@ fn summarise_struct(cx: @ExtCtxt, span: span, } } +pub fn create_subpatterns(cx: @ExtCtxt, + span: span, + field_paths: ~[@ast::Path], + mutbl: ast::mutability) + -> ~[@ast::pat] { + do field_paths.map |&path| { + cx.pat(span, ast::pat_ident(ast::bind_by_ref(mutbl), path, None)) + } +} + +#[deriving(Eq)] // dogfooding! +enum StructType { + Unknown, Record, Tuple +} + +fn create_struct_pattern(cx: @ExtCtxt, + span: span, + struct_ident: ident, + struct_def: &struct_def, + prefix: &str, + mutbl: ast::mutability) + -> (@ast::pat, ~[(Option, @expr)]) { + if struct_def.fields.is_empty() { + return ( + cx.pat_ident_binding_mode( + span, struct_ident, ast::bind_infer), + ~[]); + } + + let matching_path = cx.path(span, ~[ struct_ident ]); + + let mut paths = ~[]; + let mut ident_expr = ~[]; + let mut struct_type = Unknown; + + for struct_def.fields.eachi |i, struct_field| { + let opt_id = match struct_field.node.kind { + ast::named_field(ident, _) if (struct_type == Unknown || + struct_type == Record) => { + struct_type = Record; + Some(ident) + } + ast::unnamed_field if (struct_type == Unknown || + struct_type == Tuple) => { + struct_type = Tuple; + None + } + _ => { + cx.span_bug(span, "A struct with named and unnamed fields in `deriving`"); + } + }; + let path = cx.path_ident(span, + cx.ident_of(fmt!("%s_%u", prefix, i))); + paths.push(path); + ident_expr.push((opt_id, cx.expr_path(path))); + } + + let subpats = create_subpatterns(cx, span, paths, mutbl); + + // struct_type is definitely not Unknown, since struct_def.fields + // must be nonempty to reach here + let pattern = if struct_type == Record { + let field_pats = do vec::build |push| { + for vec::each2(subpats, ident_expr) |&pat, &(id, _)| { + // id is guaranteed to be Some + push(ast::field_pat { ident: id.get(), pat: pat }) + } + }; + cx.pat_struct(span, matching_path, field_pats) + } else { + cx.pat_enum(span, matching_path, subpats) + }; + + (pattern, ident_expr) +} + +fn create_enum_variant_pattern(cx: @ExtCtxt, + span: span, + variant: &ast::variant, + prefix: &str, + mutbl: ast::mutability) + -> (@ast::pat, ~[(Option, @expr)]) { + + let variant_ident = variant.node.name; + match variant.node.kind { + ast::tuple_variant_kind(ref variant_args) => { + if variant_args.is_empty() { + return (cx.pat_ident_binding_mode( + span, variant_ident, ast::bind_infer), ~[]); + } + + let matching_path = cx.path_ident(span, variant_ident); + + let mut paths = ~[]; + let mut ident_expr = ~[]; + for uint::range(0, variant_args.len()) |i| { + let path = cx.path_ident(span, + cx.ident_of(fmt!("%s_%u", prefix, i))); + + paths.push(path); + ident_expr.push((None, cx.expr_path(path))); + } + + let subpats = create_subpatterns(cx, span, paths, mutbl); + + (cx.pat_enum(span, matching_path, subpats), + ident_expr) + } + ast::struct_variant_kind(struct_def) => { + create_struct_pattern(cx, span, + variant_ident, struct_def, + prefix, + mutbl) + } + } +} + + /* helpful premade recipes */ diff --git a/src/libsyntax/ext/deriving/mod.rs b/src/libsyntax/ext/deriving/mod.rs index 13c552388e1..1107f21319c 100644 --- a/src/libsyntax/ext/deriving/mod.rs +++ b/src/libsyntax/ext/deriving/mod.rs @@ -20,16 +20,10 @@ library. use core::prelude::*; -use ast; -use ast::{Ty, enum_def, expr, ident, item, Generics, meta_item, struct_def}; +use ast::{enum_def, ident, item, Generics, meta_item, struct_def}; use ext::base::ExtCtxt; use ext::build::AstBuilder; use codemap::span; -use parse::token::special_idents::clownshoes_extensions; -use opt_vec; - -use core::uint; -use core::vec; pub mod clone; pub mod iter_bytes; @@ -117,250 +111,3 @@ pub fn expand_meta_deriving(cx: @ExtCtxt, } } } - -pub fn expand_deriving(cx: @ExtCtxt, - span: span, - in_items: ~[@item], - expand_deriving_struct_def: ExpandDerivingStructDefFn, - expand_deriving_enum_def: ExpandDerivingEnumDefFn) - -> ~[@item] { - let mut result = ~[]; - for in_items.each |item| { - result.push(copy *item); - match item.node { - ast::item_struct(struct_def, ref generics) => { - result.push(expand_deriving_struct_def(cx, - span, - struct_def, - item.ident, - generics)); - } - ast::item_enum(ref enum_definition, ref generics) => { - result.push(expand_deriving_enum_def(cx, - span, - enum_definition, - item.ident, - generics)); - } - _ => () - } - } - result -} - -pub fn create_self_type_with_params(cx: @ExtCtxt, - span: span, - type_ident: ident, - generics: &Generics) - -> @Ty { - // Create the type parameters on the `self` path. - let mut self_ty_params = ~[]; - for generics.ty_params.each |ty_param| { - let self_ty_param = cx.ty_ident(span, ty_param.ident); - self_ty_params.push(self_ty_param); - } - - let lifetime = if generics.lifetimes.is_empty() { - None - } else { - Some(@*generics.lifetimes.get(0)) - }; - - - // Create the type of `self`. - cx.ty_path(cx.path_all(span, false, ~[ type_ident ], lifetime, self_ty_params)) -} - -pub fn create_derived_impl(cx: @ExtCtxt, - span: span, - type_ident: ident, - generics: &Generics, - methods: &[@ast::method], - trait_path: @ast::Path, - mut impl_generics: Generics, - bounds_paths: opt_vec::OptVec<@ast::Path>) - -> @item { - /*! - * - * Given that we are deriving a trait `Tr` for a type `T<'a, ..., - * 'z, A, ..., Z>`, creates an impl like: - * - * impl<'a, ..., 'z, A:Tr B1 B2, ..., Z: Tr B1 B2> Tr for T { ... } - * - * where B1, B2, ... are the bounds given by `bounds_paths`. - * - */ - - // Copy the lifetimes - for generics.lifetimes.each |l| { - impl_generics.lifetimes.push(copy *l) - }; - - // Create the type parameters. - for generics.ty_params.each |ty_param| { - // extra restrictions on the generics parameters to the type being derived upon - let mut bounds = do bounds_paths.map |&bound_path| { - cx.typarambound(bound_path) - }; - - let this_trait_bound = cx.typarambound(trait_path); - bounds.push(this_trait_bound); - - impl_generics.ty_params.push(cx.typaram(ty_param.ident, @bounds)); - } - - // Create the reference to the trait. - let trait_ref = cx.trait_ref(trait_path); - - // Create the type of `self`. - let self_type = create_self_type_with_params(cx, - span, - type_ident, - generics); - - let doc_attr = cx.attribute( - span, - cx.meta_name_value(span, - ~"doc", ast::lit_str(@~"Automatically derived."))); - cx.item( - span, - clownshoes_extensions, - ~[doc_attr], - ast::item_impl(impl_generics, - Some(trait_ref), - self_type, - methods.map(|x| *x))) -} - -pub fn create_subpatterns(cx: @ExtCtxt, - span: span, - field_paths: ~[@ast::Path], - mutbl: ast::mutability) - -> ~[@ast::pat] { - do field_paths.map |&path| { - cx.pat(span, - ast::pat_ident(ast::bind_by_ref(mutbl), path, None)) - } -} - -#[deriving(Eq)] // dogfooding! -enum StructType { - Unknown, Record, Tuple -} - -pub fn create_struct_pattern(cx: @ExtCtxt, - span: span, - struct_ident: ident, - struct_def: &struct_def, - prefix: &str, - mutbl: ast::mutability) - -> (@ast::pat, ~[(Option, @expr)]) { - if struct_def.fields.is_empty() { - return ( - cx.pat_ident_binding_mode( - span, struct_ident, ast::bind_infer), - ~[]); - } - - let matching_path = cx.path(span, ~[ struct_ident ]); - - let mut paths = ~[]; - let mut ident_expr = ~[]; - let mut struct_type = Unknown; - - for struct_def.fields.eachi |i, struct_field| { - let opt_id = match struct_field.node.kind { - ast::named_field(ident, _) if (struct_type == Unknown || - struct_type == Record) => { - struct_type = Record; - Some(ident) - } - ast::unnamed_field if (struct_type == Unknown || - struct_type == Tuple) => { - struct_type = Tuple; - None - } - _ => { - cx.span_bug(span, "A struct with named and unnamed fields in `deriving`"); - } - }; - let path = cx.path_ident(span, - cx.ident_of(fmt!("%s_%u", prefix, i))); - paths.push(path); - ident_expr.push((opt_id, cx.expr_path(path))); - } - - let subpats = create_subpatterns(cx, span, paths, mutbl); - - // struct_type is definitely not Unknown, since struct_def.fields - // must be nonempty to reach here - let pattern = if struct_type == Record { - let field_pats = do vec::build |push| { - for vec::each2(subpats, ident_expr) |&pat, &(id, _)| { - // id is guaranteed to be Some - push(ast::field_pat { ident: id.get(), pat: pat }) - } - }; - cx.pat_struct(span, matching_path, field_pats) - } else { - cx.pat_enum(span, matching_path, subpats) - }; - - (pattern, ident_expr) -} - -pub fn create_enum_variant_pattern(cx: @ExtCtxt, - span: span, - variant: &ast::variant, - prefix: &str, - mutbl: ast::mutability) - -> (@ast::pat, ~[(Option, @expr)]) { - - let variant_ident = variant.node.name; - match variant.node.kind { - ast::tuple_variant_kind(ref variant_args) => { - if variant_args.is_empty() { - return (cx.pat_ident_binding_mode( - span, variant_ident, ast::bind_infer), ~[]); - } - - let matching_path = cx.path_ident(span, variant_ident); - - let mut paths = ~[]; - let mut ident_expr = ~[]; - for uint::range(0, variant_args.len()) |i| { - let path = cx.path_ident(span, - cx.ident_of(fmt!("%s_%u", prefix, i))); - - paths.push(path); - ident_expr.push((None, cx.expr_path(path))); - } - - let subpats = create_subpatterns(cx, span, paths, mutbl); - - (cx.pat_enum(span, matching_path, subpats), - ident_expr) - } - ast::struct_variant_kind(struct_def) => { - create_struct_pattern(cx, span, - variant_ident, struct_def, - prefix, - mutbl) - } - } -} - -pub fn variant_arg_count(_cx: @ExtCtxt, _span: span, variant: &ast::variant) -> uint { - match variant.node.kind { - ast::tuple_variant_kind(ref args) => args.len(), - ast::struct_variant_kind(ref struct_def) => struct_def.fields.len(), - } -} - -pub fn expand_enum_or_struct_match(cx: @ExtCtxt, - span: span, - arms: ~[ ast::arm ]) - -> @expr { - let self_expr = cx.expr_deref(span, cx.expr_self(span)); - cx.expr_match(span, self_expr, arms) -} From 6d5beda6772230d8ec0e5e4b9e561100acf81a6f Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Fri, 7 Jun 2013 17:46:44 +1000 Subject: [PATCH 2/4] syntax: move expand_generic_deriving to be a method on TraitDef --- src/libsyntax/ext/deriving/clone.rs | 8 +--- src/libsyntax/ext/deriving/cmp/eq.rs | 4 +- src/libsyntax/ext/deriving/cmp/ord.rs | 4 +- src/libsyntax/ext/deriving/cmp/totaleq.rs | 4 +- src/libsyntax/ext/deriving/cmp/totalord.rs | 3 +- src/libsyntax/ext/deriving/decodable.rs | 3 +- src/libsyntax/ext/deriving/encodable.rs | 3 +- src/libsyntax/ext/deriving/generic.rs | 55 ++++++++++------------ src/libsyntax/ext/deriving/iter_bytes.rs | 2 +- src/libsyntax/ext/deriving/rand.rs | 3 +- src/libsyntax/ext/deriving/to_str.rs | 3 +- 11 files changed, 37 insertions(+), 55 deletions(-) diff --git a/src/libsyntax/ext/deriving/clone.rs b/src/libsyntax/ext/deriving/clone.rs index 8d9abb186fd..4fc67f2f52e 100644 --- a/src/libsyntax/ext/deriving/clone.rs +++ b/src/libsyntax/ext/deriving/clone.rs @@ -38,9 +38,7 @@ pub fn expand_deriving_clone(cx: @ExtCtxt, ] }; - expand_deriving_generic(cx, span, - mitem, in_items, - &trait_def) + trait_def.expand(cx, span, mitem, in_items) } pub fn expand_deriving_deep_clone(cx: @ExtCtxt, @@ -67,9 +65,7 @@ pub fn expand_deriving_deep_clone(cx: @ExtCtxt, ] }; - expand_deriving_generic(cx, span, - mitem, in_items, - &trait_def) + trait_def.expand(cx, span, mitem, in_items) } fn cs_clone( diff --git a/src/libsyntax/ext/deriving/cmp/eq.rs b/src/libsyntax/ext/deriving/cmp/eq.rs index 67107b4218a..5fc75511e57 100644 --- a/src/libsyntax/ext/deriving/cmp/eq.rs +++ b/src/libsyntax/ext/deriving/cmp/eq.rs @@ -54,7 +54,5 @@ pub fn expand_deriving_eq(cx: @ExtCtxt, md!("ne", cs_ne) ] }; - - expand_deriving_generic(cx, span, mitem, in_items, - &trait_def) + trait_def.expand(cx, span, mitem, in_items) } diff --git a/src/libsyntax/ext/deriving/cmp/ord.rs b/src/libsyntax/ext/deriving/cmp/ord.rs index 8b8ee37691c..e07215a905a 100644 --- a/src/libsyntax/ext/deriving/cmp/ord.rs +++ b/src/libsyntax/ext/deriving/cmp/ord.rs @@ -49,9 +49,7 @@ pub fn expand_deriving_ord(cx: @ExtCtxt, md!("ge", false, true) ] }; - - expand_deriving_generic(cx, span, mitem, in_items, - &trait_def) + trait_def.expand(cx, span, mitem, in_items) } /// `less`: is this `lt` or `le`? `equal`: is this `le` or `ge`? diff --git a/src/libsyntax/ext/deriving/cmp/totaleq.rs b/src/libsyntax/ext/deriving/cmp/totaleq.rs index f07c8949438..acd2073b273 100644 --- a/src/libsyntax/ext/deriving/cmp/totaleq.rs +++ b/src/libsyntax/ext/deriving/cmp/totaleq.rs @@ -42,7 +42,5 @@ pub fn expand_deriving_totaleq(cx: @ExtCtxt, } ] }; - - expand_deriving_generic(cx, span, mitem, in_items, - &trait_def) + trait_def.expand(cx, span, mitem, in_items) } diff --git a/src/libsyntax/ext/deriving/cmp/totalord.rs b/src/libsyntax/ext/deriving/cmp/totalord.rs index 4c1c940927b..94407cd6e72 100644 --- a/src/libsyntax/ext/deriving/cmp/totalord.rs +++ b/src/libsyntax/ext/deriving/cmp/totalord.rs @@ -38,8 +38,7 @@ pub fn expand_deriving_totalord(cx: @ExtCtxt, ] }; - expand_deriving_generic(cx, span, mitem, in_items, - &trait_def) + trait_def.expand(cx, span, mitem, in_items) } diff --git a/src/libsyntax/ext/deriving/decodable.rs b/src/libsyntax/ext/deriving/decodable.rs index 1991b2456d9..2081f262825 100644 --- a/src/libsyntax/ext/deriving/decodable.rs +++ b/src/libsyntax/ext/deriving/decodable.rs @@ -49,8 +49,7 @@ pub fn expand_deriving_decodable(cx: @ExtCtxt, ] }; - expand_deriving_generic(cx, span, mitem, in_items, - &trait_def) + trait_def.expand(cx, span, mitem, in_items) } fn decodable_substructure(cx: @ExtCtxt, span: span, diff --git a/src/libsyntax/ext/deriving/encodable.rs b/src/libsyntax/ext/deriving/encodable.rs index b9c4bf7bf26..2d24de553d6 100644 --- a/src/libsyntax/ext/deriving/encodable.rs +++ b/src/libsyntax/ext/deriving/encodable.rs @@ -109,8 +109,7 @@ pub fn expand_deriving_encodable(cx: @ExtCtxt, ] }; - expand_deriving_generic(cx, span, mitem, in_items, - &trait_def) + trait_def.expand(cx, span, mitem, in_items) } fn encodable_substructure(cx: @ExtCtxt, span: span, diff --git a/src/libsyntax/ext/deriving/generic.rs b/src/libsyntax/ext/deriving/generic.rs index e386a2cfb53..52a2d9ff9de 100644 --- a/src/libsyntax/ext/deriving/generic.rs +++ b/src/libsyntax/ext/deriving/generic.rs @@ -178,35 +178,6 @@ use core::vec; pub use self::ty::*; mod ty; -pub fn expand_deriving_generic(cx: @ExtCtxt, - span: span, - _mitem: @ast::meta_item, - in_items: ~[@ast::item], - trait_def: &TraitDef) -> ~[@ast::item] { - let mut result = ~[]; - for in_items.each |item| { - result.push(*item); - match item.node { - ast::item_struct(struct_def, ref generics) => { - result.push(trait_def.expand_struct_def(cx, - span, - struct_def, - item.ident, - generics)); - } - ast::item_enum(ref enum_definition, ref generics) => { - result.push(trait_def.expand_enum_def(cx, - span, - enum_definition, - item.ident, - generics)); - } - _ => () - } - } - result -} - pub struct TraitDef<'self> { /// Path of the trait, including any type parameters path: Path<'self>, @@ -310,6 +281,32 @@ pub type EnumNonMatchFunc<'self> = impl<'self> TraitDef<'self> { + pub fn expand(&self, cx: @ExtCtxt, + span: span, + _mitem: @ast::meta_item, + in_items: ~[@ast::item]) -> ~[@ast::item] { + let mut result = ~[]; + for in_items.each |item| { + result.push(*item); + match item.node { + ast::item_struct(struct_def, ref generics) => { + result.push(self.expand_struct_def(cx, span, + struct_def, + item.ident, + generics)); + } + ast::item_enum(ref enum_def, ref generics) => { + result.push(self.expand_enum_def(cx, span, + enum_def, + item.ident, + generics)); + } + _ => () + } + } + result + } + /** * * Given that we are deriving a trait `Tr` for a type `T<'a, ..., diff --git a/src/libsyntax/ext/deriving/iter_bytes.rs b/src/libsyntax/ext/deriving/iter_bytes.rs index 10fb4b8ecd4..13f83b55a40 100644 --- a/src/libsyntax/ext/deriving/iter_bytes.rs +++ b/src/libsyntax/ext/deriving/iter_bytes.rs @@ -42,7 +42,7 @@ pub fn expand_deriving_iter_bytes(cx: @ExtCtxt, ] }; - expand_deriving_generic(cx, span, mitem, in_items, &trait_def) + trait_def.expand(cx, span, mitem, in_items) } fn iter_bytes_substructure(cx: @ExtCtxt, span: span, substr: &Substructure) -> @expr { diff --git a/src/libsyntax/ext/deriving/rand.rs b/src/libsyntax/ext/deriving/rand.rs index 54d31de7c50..ab5ac6d7847 100644 --- a/src/libsyntax/ext/deriving/rand.rs +++ b/src/libsyntax/ext/deriving/rand.rs @@ -47,8 +47,7 @@ pub fn expand_deriving_rand(cx: @ExtCtxt, } ] }; - - expand_deriving_generic(cx, span, mitem, in_items, &trait_def) + trait_def.expand(cx, span, mitem, in_items) } fn rand_substructure(cx: @ExtCtxt, span: span, substr: &Substructure) -> @expr { diff --git a/src/libsyntax/ext/deriving/to_str.rs b/src/libsyntax/ext/deriving/to_str.rs index 52efabd4b9b..41be3a775c1 100644 --- a/src/libsyntax/ext/deriving/to_str.rs +++ b/src/libsyntax/ext/deriving/to_str.rs @@ -37,8 +37,7 @@ pub fn expand_deriving_to_str(cx: @ExtCtxt, } ] }; - - expand_deriving_generic(cx, span, mitem, in_items, &trait_def) + trait_def.expand(cx, span, mitem, in_items) } fn to_str_substructure(cx: @ExtCtxt, span: span, substr: &Substructure) -> @expr { From ebf7281b7b16d5083d1a6d114ae49e6f878c72d3 Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Fri, 7 Jun 2013 18:36:16 +1000 Subject: [PATCH 3/4] syntax: rewrite deriving(Ord) to not require Eq. lt and gt are implement directly in terms of the corresponding method on their elements, and le and ge are the negations of these. --- src/libsyntax/ext/deriving/cmp/ord.rs | 116 +++++++++++++------------- 1 file changed, 59 insertions(+), 57 deletions(-) diff --git a/src/libsyntax/ext/deriving/cmp/ord.rs b/src/libsyntax/ext/deriving/cmp/ord.rs index e07215a905a..759576814d5 100644 --- a/src/libsyntax/ext/deriving/cmp/ord.rs +++ b/src/libsyntax/ext/deriving/cmp/ord.rs @@ -10,6 +10,7 @@ use core::prelude::*; +use ast; use ast::{meta_item, item, expr}; use codemap::span; use ext::base::ExtCtxt; @@ -21,7 +22,7 @@ pub fn expand_deriving_ord(cx: @ExtCtxt, mitem: @meta_item, in_items: ~[@item]) -> ~[@item] { macro_rules! md ( - ($name:expr, $less:expr, $equal:expr) => { + ($name:expr, $func:expr, $op:expr) => { MethodDef { name: $name, generics: LifetimeBounds::empty(), @@ -29,82 +30,67 @@ pub fn expand_deriving_ord(cx: @ExtCtxt, args: ~[borrowed_self()], ret_ty: Literal(Path::new(~["bool"])), const_nonmatching: false, - combine_substructure: |cx, span, substr| - cs_ord($less, $equal, cx, span, substr) + combine_substructure: |cx, span, substr| $func($op, cx, span, substr) } } ); - - let trait_def = TraitDef { path: Path::new(~["std", "cmp", "Ord"]), - // XXX: Ord doesn't imply Eq yet - additional_bounds: ~[Literal(Path::new(~["std", "cmp", "Eq"]))], + additional_bounds: ~[], generics: LifetimeBounds::empty(), methods: ~[ - md!("lt", true, false), - md!("le", true, true), - md!("gt", false, false), - md!("ge", false, true) + md!("lt", cs_strict, true), + md!("le", cs_nonstrict, true), // inverse operation + md!("gt", cs_strict, false), + md!("ge", cs_nonstrict, false) ] }; trait_def.expand(cx, span, mitem, in_items) } -/// `less`: is this `lt` or `le`? `equal`: is this `le` or `ge`? -fn cs_ord(less: bool, equal: bool, - cx: @ExtCtxt, span: span, - substr: &Substructure) -> @expr { - let binop = if less { - cx.ident_of("lt") - } else { - cx.ident_of("gt") - }; - let base = cx.expr_bool(span, equal); - +/// Strict inequality. +fn cs_strict(less: bool, cx: @ExtCtxt, span: span, substr: &Substructure) -> @expr { + let op = if less {ast::lt} else {ast::gt}; cs_fold( false, // need foldr, |cx, span, subexpr, self_f, other_fs| { /* - - build up a series of nested ifs from the inside out to get - lexical ordering (hence foldr), i.e. + build up a series of chain ||'s and &&'s from the inside + out (hence foldr) to get lexical ordering, i.e. for op == + `ast::lt` ``` - if self.f1 `binop` other.f1 { - true - } else if self.f1 == other.f1 { - if self.f2 `binop` other.f2 { - true - } else if self.f2 == other.f2 { - `equal` - } else { - false - } - } else { - false - } + self.f1 < other.f1 || (!(other.f1 < self.f1) && + (self.f2 < other.f2 || (!(other.f2 < self.f2) && + (false) + )) + ) ``` - The inner "`equal`" case is only reached if the two - items have all fields equal. + The optimiser should remove the redundancy. We explicitly + get use the binops to avoid auto-deref derefencing too many + layers of pointers, if the type includes pointers. */ - if other_fs.len() != 1 { - cx.span_bug(span, "Not exactly 2 arguments in `deriving(Ord)`"); - } + let other_f = match other_fs { + [o_f] => o_f, + _ => cx.span_bug(span, "Not exactly 2 arguments in `deriving(Ord)`") + }; - let cmp = cx.expr_method_call(span, - self_f, cx.ident_of("eq"), other_fs.to_owned()); - let elseif = cx.expr_if(span, cmp, - subexpr, Some(cx.expr_bool(span, false))); + let cmp = cx.expr_binary(span, op, + cx.expr_deref(span, self_f), + cx.expr_deref(span, other_f)); - let cmp = cx.expr_method_call(span, - self_f, binop, other_fs.to_owned()); - cx.expr_if(span, cmp, - cx.expr_bool(span, true), Some(elseif)) + let not_cmp = cx.expr_binary(span, op, + cx.expr_deref(span, other_f), + cx.expr_deref(span, self_f)); + let not_cmp = cx.expr_unary(span, ast::not, not_cmp); + + let and = cx.expr_binary(span, ast::and, + not_cmp, subexpr); + cx.expr_binary(span, ast::or, cmp, and) }, - base, + cx.expr_bool(span, false), |cx, span, args, _| { // nonmatching enums, order by the order the variants are // written @@ -112,13 +98,29 @@ fn cs_ord(less: bool, equal: bool, [(self_var, _, _), (other_var, _, _)] => cx.expr_bool(span, - if less { - self_var < other_var - } else { - self_var > other_var - }), + if less { + self_var < other_var + } else { + self_var > other_var + }), _ => cx.span_bug(span, "Not exactly 2 arguments in `deriving(Ord)`") } }, cx, span, substr) } + +fn cs_nonstrict(less: bool, cx: @ExtCtxt, span: span, substr: &Substructure) -> @expr { + // Example: ge becomes !(*self < *other), le becomes !(*self > *other) + + let inverse_op = if less {ast::gt} else {ast::lt}; + match substr.self_args { + [self_, other] => { + let inverse_cmp = cx.expr_binary(span, inverse_op, + cx.expr_deref(span, self_), + cx.expr_deref(span, other)); + + cx.expr_unary(span, ast::not, inverse_cmp) + } + _ => cx.span_bug(span, "Not exactly 2 arguments in `deriving(Ord)`") + } +} From a965f4981a2adedb6ba10cf210cea6b7887e8267 Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Fri, 7 Jun 2013 21:53:04 +1000 Subject: [PATCH 4/4] syntax: correct the modifications to deriving(Ord) so that it works. --- src/libsyntax/ext/deriving/cmp/ord.rs | 43 ++++++++------------------- 1 file changed, 13 insertions(+), 30 deletions(-) diff --git a/src/libsyntax/ext/deriving/cmp/ord.rs b/src/libsyntax/ext/deriving/cmp/ord.rs index 759576814d5..c60b589dfc3 100644 --- a/src/libsyntax/ext/deriving/cmp/ord.rs +++ b/src/libsyntax/ext/deriving/cmp/ord.rs @@ -22,7 +22,7 @@ pub fn expand_deriving_ord(cx: @ExtCtxt, mitem: @meta_item, in_items: ~[@item]) -> ~[@item] { macro_rules! md ( - ($name:expr, $func:expr, $op:expr) => { + ($name:expr, $op:expr, $equal:expr) => { MethodDef { name: $name, generics: LifetimeBounds::empty(), @@ -30,7 +30,7 @@ pub fn expand_deriving_ord(cx: @ExtCtxt, args: ~[borrowed_self()], ret_ty: Literal(Path::new(~["bool"])), const_nonmatching: false, - combine_substructure: |cx, span, substr| $func($op, cx, span, substr) + combine_substructure: |cx, span, substr| cs_op($op, $equal, cx, span, substr) } } ); @@ -40,17 +40,17 @@ pub fn expand_deriving_ord(cx: @ExtCtxt, additional_bounds: ~[], generics: LifetimeBounds::empty(), methods: ~[ - md!("lt", cs_strict, true), - md!("le", cs_nonstrict, true), // inverse operation - md!("gt", cs_strict, false), - md!("ge", cs_nonstrict, false) + md!("lt", true, false), + md!("le", true, true), + md!("gt", false, false), + md!("ge", false, true) ] }; trait_def.expand(cx, span, mitem, in_items) } /// Strict inequality. -fn cs_strict(less: bool, cx: @ExtCtxt, span: span, substr: &Substructure) -> @expr { +fn cs_op(less: bool, equal: bool, cx: @ExtCtxt, span: span, substr: &Substructure) -> @expr { let op = if less {ast::lt} else {ast::gt}; cs_fold( false, // need foldr, @@ -81,16 +81,15 @@ fn cs_strict(less: bool, cx: @ExtCtxt, span: span, substr: &Substructure) -> @ex cx.expr_deref(span, self_f), cx.expr_deref(span, other_f)); - let not_cmp = cx.expr_binary(span, op, - cx.expr_deref(span, other_f), - cx.expr_deref(span, self_f)); - let not_cmp = cx.expr_unary(span, ast::not, not_cmp); + let not_cmp = cx.expr_unary(span, ast::not, + cx.expr_binary(span, op, + cx.expr_deref(span, other_f), + cx.expr_deref(span, self_f))); - let and = cx.expr_binary(span, ast::and, - not_cmp, subexpr); + let and = cx.expr_binary(span, ast::and, not_cmp, subexpr); cx.expr_binary(span, ast::or, cmp, and) }, - cx.expr_bool(span, false), + cx.expr_bool(span, equal), |cx, span, args, _| { // nonmatching enums, order by the order the variants are // written @@ -108,19 +107,3 @@ fn cs_strict(less: bool, cx: @ExtCtxt, span: span, substr: &Substructure) -> @ex }, cx, span, substr) } - -fn cs_nonstrict(less: bool, cx: @ExtCtxt, span: span, substr: &Substructure) -> @expr { - // Example: ge becomes !(*self < *other), le becomes !(*self > *other) - - let inverse_op = if less {ast::gt} else {ast::lt}; - match substr.self_args { - [self_, other] => { - let inverse_cmp = cx.expr_binary(span, inverse_op, - cx.expr_deref(span, self_), - cx.expr_deref(span, other)); - - cx.expr_unary(span, ast::not, inverse_cmp) - } - _ => cx.span_bug(span, "Not exactly 2 arguments in `deriving(Ord)`") - } -}