From 5d1bdc320ba5304854f409ba68060f5739bca044 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Sat, 5 Jul 2014 08:04:07 +0200 Subject: [PATCH] Revise the `const_nonmatching` flag with more info about author's intent. In particular, I want authors of deriving modes to understand what they are opting into (namely quadratic code size or worse) when they select NonMatchesExplode. --- src/libsyntax/ext/deriving/clone.rs | 2 +- src/libsyntax/ext/deriving/cmp/eq.rs | 2 +- src/libsyntax/ext/deriving/cmp/ord.rs | 4 +-- src/libsyntax/ext/deriving/cmp/totaleq.rs | 2 +- src/libsyntax/ext/deriving/cmp/totalord.rs | 2 +- src/libsyntax/ext/deriving/decodable.rs | 2 +- src/libsyntax/ext/deriving/default.rs | 2 +- src/libsyntax/ext/deriving/encodable.rs | 2 +- src/libsyntax/ext/deriving/generic/mod.rs | 31 +++++++++++++++++----- src/libsyntax/ext/deriving/hash.rs | 2 +- src/libsyntax/ext/deriving/primitive.rs | 4 +-- src/libsyntax/ext/deriving/rand.rs | 2 +- src/libsyntax/ext/deriving/show.rs | 2 +- src/libsyntax/ext/deriving/zero.rs | 4 +-- 14 files changed, 40 insertions(+), 23 deletions(-) diff --git a/src/libsyntax/ext/deriving/clone.rs b/src/libsyntax/ext/deriving/clone.rs index 93e4920bc1de4..1a296906cc2f9 100644 --- a/src/libsyntax/ext/deriving/clone.rs +++ b/src/libsyntax/ext/deriving/clone.rs @@ -39,7 +39,7 @@ pub fn expand_deriving_clone(cx: &mut ExtCtxt, args: Vec::new(), ret_ty: Self, attributes: attrs, - const_nonmatching: false, + on_nonmatching: NonMatchHandlingIrrelevant, combine_substructure: combine_substructure(|c, s, sub| { cs_clone("Clone", c, s, sub) }), diff --git a/src/libsyntax/ext/deriving/cmp/eq.rs b/src/libsyntax/ext/deriving/cmp/eq.rs index ef8d477a98e67..2eaeb0df7fbb7 100644 --- a/src/libsyntax/ext/deriving/cmp/eq.rs +++ b/src/libsyntax/ext/deriving/cmp/eq.rs @@ -45,7 +45,7 @@ pub fn expand_deriving_eq(cx: &mut ExtCtxt, args: vec!(borrowed_self()), ret_ty: Literal(Path::new(vec!("bool"))), attributes: attrs, - const_nonmatching: true, + on_nonmatching: NonMatchesCollapse, combine_substructure: combine_substructure(|a, b, c| { $f(a, b, c) }) diff --git a/src/libsyntax/ext/deriving/cmp/ord.rs b/src/libsyntax/ext/deriving/cmp/ord.rs index 59cdec1ea88f0..c8edf5c4157af 100644 --- a/src/libsyntax/ext/deriving/cmp/ord.rs +++ b/src/libsyntax/ext/deriving/cmp/ord.rs @@ -35,7 +35,7 @@ pub fn expand_deriving_ord(cx: &mut ExtCtxt, args: vec!(borrowed_self()), ret_ty: Literal(Path::new(vec!("bool"))), attributes: attrs, - const_nonmatching: false, + on_nonmatching: NonMatchesExplode, combine_substructure: combine_substructure(|cx, span, substr| { cs_op($op, $equal, cx, span, substr) }) @@ -59,7 +59,7 @@ pub fn expand_deriving_ord(cx: &mut ExtCtxt, args: vec![borrowed_self()], ret_ty: ret_ty, attributes: attrs, - const_nonmatching: false, + on_nonmatching: NonMatchesExplode, combine_substructure: combine_substructure(|cx, span, substr| { cs_partial_cmp(cx, span, substr) }) diff --git a/src/libsyntax/ext/deriving/cmp/totaleq.rs b/src/libsyntax/ext/deriving/cmp/totaleq.rs index 8b1e0498d253c..09aa24f9bfb5d 100644 --- a/src/libsyntax/ext/deriving/cmp/totaleq.rs +++ b/src/libsyntax/ext/deriving/cmp/totaleq.rs @@ -57,7 +57,7 @@ pub fn expand_deriving_totaleq(cx: &mut ExtCtxt, args: vec!(), ret_ty: nil_ty(), attributes: attrs, - const_nonmatching: true, + on_nonmatching: NonMatchesCollapse, combine_substructure: combine_substructure(|a, b, c| { cs_total_eq_assert(a, b, c) }) diff --git a/src/libsyntax/ext/deriving/cmp/totalord.rs b/src/libsyntax/ext/deriving/cmp/totalord.rs index 271aa90cd24a4..24785a026d18e 100644 --- a/src/libsyntax/ext/deriving/cmp/totalord.rs +++ b/src/libsyntax/ext/deriving/cmp/totalord.rs @@ -41,7 +41,7 @@ pub fn expand_deriving_totalord(cx: &mut ExtCtxt, args: vec!(borrowed_self()), ret_ty: Literal(Path::new(vec!("std", "cmp", "Ordering"))), attributes: attrs, - const_nonmatching: false, + on_nonmatching: NonMatchesExplode, combine_substructure: combine_substructure(|a, b, c| { cs_cmp(a, b, c) }), diff --git a/src/libsyntax/ext/deriving/decodable.rs b/src/libsyntax/ext/deriving/decodable.rs index 6da5f1e2700f1..3422819c4fa95 100644 --- a/src/libsyntax/ext/deriving/decodable.rs +++ b/src/libsyntax/ext/deriving/decodable.rs @@ -54,7 +54,7 @@ pub fn expand_deriving_decodable(cx: &mut ExtCtxt, vec!(box Self, box Literal(Path::new_local("__E"))), true)), attributes: Vec::new(), - const_nonmatching: true, + on_nonmatching: NonMatchHandlingIrrelevant, combine_substructure: combine_substructure(|a, b, c| { decodable_substructure(a, b, c) }), diff --git a/src/libsyntax/ext/deriving/default.rs b/src/libsyntax/ext/deriving/default.rs index dfebc0f5e6421..e6fffaa2a3f0b 100644 --- a/src/libsyntax/ext/deriving/default.rs +++ b/src/libsyntax/ext/deriving/default.rs @@ -39,7 +39,7 @@ pub fn expand_deriving_default(cx: &mut ExtCtxt, args: Vec::new(), ret_ty: Self, attributes: attrs, - const_nonmatching: false, + on_nonmatching: NonMatchHandlingIrrelevant, combine_substructure: combine_substructure(|a, b, c| { default_substructure(a, b, c) }) diff --git a/src/libsyntax/ext/deriving/encodable.rs b/src/libsyntax/ext/deriving/encodable.rs index 3b34407edfeaa..21a4c5fdaaf71 100644 --- a/src/libsyntax/ext/deriving/encodable.rs +++ b/src/libsyntax/ext/deriving/encodable.rs @@ -121,7 +121,7 @@ pub fn expand_deriving_encodable(cx: &mut ExtCtxt, box Literal(Path::new_local("__E"))), true)), attributes: Vec::new(), - const_nonmatching: true, + on_nonmatching: NonMatchHandlingIrrelevant, combine_substructure: combine_substructure(|a, b, c| { encodable_substructure(a, b, c) }), diff --git a/src/libsyntax/ext/deriving/generic/mod.rs b/src/libsyntax/ext/deriving/generic/mod.rs index c9f5936a9bb05..eebf55033b374 100644 --- a/src/libsyntax/ext/deriving/generic/mod.rs +++ b/src/libsyntax/ext/deriving/generic/mod.rs @@ -39,7 +39,7 @@ //! same variant of the enum (e.g. `Some(1)`, `Some(3)` and `Some(4)`) //! - `EnumNonMatching` when `Self` is an enum and the arguments are not //! the same variant (e.g. `None`, `Some(1)` and `None`). If -//! `const_nonmatching` is true, this will contain an empty list. +//! `on_nonmatching == NonMatchesCollapse`, this will contain an empty list. //! - `StaticEnum` and `StaticStruct` for static methods, where the type //! being derived upon is either an enum or struct respectively. (Any //! argument with type Self is just grouped among the non-self @@ -135,7 +135,7 @@ //! }]) //! ~~~ //! -//! For `C1 {x}` and `C1 {x}`, +//! For `C1 {x}` and `C1 {x}` , //! //! ~~~text //! EnumMatching(1, , @@ -172,6 +172,7 @@ //! (, , //! Named(~[(, )]))]) //! ~~~ +//! use std::cell::RefCell; use std::gc::{Gc, GC}; @@ -212,6 +213,12 @@ pub struct TraitDef<'a> { pub methods: Vec>, } +#[deriving(PartialEq, Eq)] +pub enum HandleNonMatchingEnums { + NonMatchesCollapse, // handle all non-matches via one `_ => ..` clause + NonMatchesExplode, // handle via n^k cases for n variants and k self-args + NonMatchHandlingIrrelevant, // cannot encounter two enums of Self type +} pub struct MethodDef<'a> { /// name of the method @@ -232,9 +239,17 @@ pub struct MethodDef<'a> { pub attributes: Vec, - /// if the value of the nonmatching enums is independent of the - /// actual enum variants, i.e. can use _ => .. match. - pub const_nonmatching: bool, + /// How to handle nonmatching enums; `NonMatchesCollapse` + /// indicates value is independent of the actual enum variants, + /// i.e. can use _ => .. match. + /// + /// Note that if this is `NonMatchesExplode`, then deriving will + /// generate `Omega(n^k)` code, where `n` is the number of + /// variants and `k` is the number of arguments of `Self` type for + /// the method (including the `self` argument, if any). Strive to + /// avoid use of `NonMatchesExplode`, to avoid generating + /// quadratic amounts of code (#15375) or worse. + pub on_nonmatching: HandleNonMatchingEnums, pub combine_substructure: RefCell>, } @@ -758,7 +773,7 @@ impl<'a> MethodDef<'a> { A2(int) } - // is equivalent to (with const_nonmatching == false) + // is equivalent to (with on_nonmatching == NonMatchesExplode) impl PartialEq for A { fn eq(&self, __arg_1: &A) { @@ -893,7 +908,9 @@ impl<'a> MethodDef<'a> { // the code for nonmatching variants only matters when // we've seen at least one other variant already - if self.const_nonmatching && match_count > 0 { + assert!(match_count == 0 || + self.on_nonmatching != NonMatchHandlingIrrelevant); + if self.on_nonmatching == NonMatchesCollapse && match_count > 0 { // make a matching-variant match, and a _ match. let index = match matching { Some(i) => i, diff --git a/src/libsyntax/ext/deriving/hash.rs b/src/libsyntax/ext/deriving/hash.rs index 1b3ac47092a2d..253f8de8cdf4d 100644 --- a/src/libsyntax/ext/deriving/hash.rs +++ b/src/libsyntax/ext/deriving/hash.rs @@ -54,7 +54,7 @@ pub fn expand_deriving_hash(cx: &mut ExtCtxt, args: vec!(Ptr(box Literal(args), Borrowed(None, MutMutable))), ret_ty: nil_ty(), attributes: attrs, - const_nonmatching: false, + on_nonmatching: NonMatchHandlingIrrelevant, combine_substructure: combine_substructure(|a, b, c| { hash_substructure(a, b, c) }) diff --git a/src/libsyntax/ext/deriving/primitive.rs b/src/libsyntax/ext/deriving/primitive.rs index 735497d9a2cf6..b1071e106d985 100644 --- a/src/libsyntax/ext/deriving/primitive.rs +++ b/src/libsyntax/ext/deriving/primitive.rs @@ -45,7 +45,7 @@ pub fn expand_deriving_from_primitive(cx: &mut ExtCtxt, true)), // #[inline] liable to cause code-bloat attributes: attrs.clone(), - const_nonmatching: false, + on_nonmatching: NonMatchHandlingIrrelevant, combine_substructure: combine_substructure(|c, s, sub| { cs_from("i64", c, s, sub) }), @@ -62,7 +62,7 @@ pub fn expand_deriving_from_primitive(cx: &mut ExtCtxt, true)), // #[inline] liable to cause code-bloat attributes: attrs, - const_nonmatching: false, + on_nonmatching: NonMatchHandlingIrrelevant, combine_substructure: combine_substructure(|c, s, sub| { cs_from("u64", c, s, sub) }), diff --git a/src/libsyntax/ext/deriving/rand.rs b/src/libsyntax/ext/deriving/rand.rs index 34b5f120d6ab8..f286ddc6f3093 100644 --- a/src/libsyntax/ext/deriving/rand.rs +++ b/src/libsyntax/ext/deriving/rand.rs @@ -45,7 +45,7 @@ pub fn expand_deriving_rand(cx: &mut ExtCtxt, ), ret_ty: Self, attributes: Vec::new(), - const_nonmatching: false, + on_nonmatching: NonMatchHandlingIrrelevant, combine_substructure: combine_substructure(|a, b, c| { rand_substructure(a, b, c) }) diff --git a/src/libsyntax/ext/deriving/show.rs b/src/libsyntax/ext/deriving/show.rs index 05b5131d7e4d3..722541d7dae53 100644 --- a/src/libsyntax/ext/deriving/show.rs +++ b/src/libsyntax/ext/deriving/show.rs @@ -45,7 +45,7 @@ pub fn expand_deriving_show(cx: &mut ExtCtxt, args: vec!(fmtr), ret_ty: Literal(Path::new(vec!("std", "fmt", "Result"))), attributes: Vec::new(), - const_nonmatching: false, + on_nonmatching: NonMatchHandlingIrrelevant, combine_substructure: combine_substructure(|a, b, c| { show_substructure(a, b, c) }) diff --git a/src/libsyntax/ext/deriving/zero.rs b/src/libsyntax/ext/deriving/zero.rs index 93947251223fd..1d4f4c78fc044 100644 --- a/src/libsyntax/ext/deriving/zero.rs +++ b/src/libsyntax/ext/deriving/zero.rs @@ -39,7 +39,7 @@ pub fn expand_deriving_zero(cx: &mut ExtCtxt, args: Vec::new(), ret_ty: Self, attributes: attrs.clone(), - const_nonmatching: false, + on_nonmatching: NonMatchHandlingIrrelevant, combine_substructure: combine_substructure(|a, b, c| { zero_substructure(a, b, c) }) @@ -51,7 +51,7 @@ pub fn expand_deriving_zero(cx: &mut ExtCtxt, args: Vec::new(), ret_ty: Literal(Path::new(vec!("bool"))), attributes: attrs, - const_nonmatching: false, + on_nonmatching: NonMatchHandlingIrrelevant, combine_substructure: combine_substructure(|cx, span, substr| { cs_and(|cx, span, _, _| cx.span_bug(span, "Non-matching enum \