Skip to content

Commit

Permalink
Revise the const_nonmatching flag with more info about author's int…
Browse files Browse the repository at this point in the history
…ent.

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.
  • Loading branch information
pnkfelix committed Jul 11, 2014
1 parent c9a77d0 commit 5d1bdc3
Show file tree
Hide file tree
Showing 14 changed files with 40 additions and 23 deletions.
2 changes: 1 addition & 1 deletion src/libsyntax/ext/deriving/clone.rs
Expand Up @@ -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)
}),
Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax/ext/deriving/cmp/eq.rs
Expand Up @@ -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)
})
Expand Down
4 changes: 2 additions & 2 deletions src/libsyntax/ext/deriving/cmp/ord.rs
Expand Up @@ -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)
})
Expand All @@ -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)
})
Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax/ext/deriving/cmp/totaleq.rs
Expand Up @@ -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)
})
Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax/ext/deriving/cmp/totalord.rs
Expand Up @@ -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)
}),
Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax/ext/deriving/decodable.rs
Expand Up @@ -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)
}),
Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax/ext/deriving/default.rs
Expand Up @@ -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)
})
Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax/ext/deriving/encodable.rs
Expand Up @@ -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)
}),
Expand Down
31 changes: 24 additions & 7 deletions src/libsyntax/ext/deriving/generic/mod.rs
Expand Up @@ -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
Expand Down Expand Up @@ -135,7 +135,7 @@
//! }])
//! ~~~
//!
//! For `C1 {x}` and `C1 {x}`,
//! For `C1 {x}` and `C1 {x}` ,
//!
//! ~~~text
//! EnumMatching(1, <ast::Variant for C1>,
Expand Down Expand Up @@ -172,6 +172,7 @@
//! (<ident of C1>, <span of C1>,
//! Named(~[(<ident of x>, <span of x>)]))])
//! ~~~
//!

use std::cell::RefCell;
use std::gc::{Gc, GC};
Expand Down Expand Up @@ -212,6 +213,12 @@ pub struct TraitDef<'a> {
pub methods: Vec<MethodDef<'a>>,
}

#[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
Expand All @@ -232,9 +239,17 @@ pub struct MethodDef<'a> {

pub attributes: Vec<ast::Attribute>,

/// 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<CombineSubstructureFunc<'a>>,
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax/ext/deriving/hash.rs
Expand Up @@ -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)
})
Expand Down
4 changes: 2 additions & 2 deletions src/libsyntax/ext/deriving/primitive.rs
Expand Up @@ -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)
}),
Expand All @@ -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)
}),
Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax/ext/deriving/rand.rs
Expand Up @@ -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)
})
Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax/ext/deriving/show.rs
Expand Up @@ -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)
})
Expand Down
4 changes: 2 additions & 2 deletions src/libsyntax/ext/deriving/zero.rs
Expand Up @@ -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)
})
Expand All @@ -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 \
Expand Down

0 comments on commit 5d1bdc3

Please sign in to comment.