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.
This commit is contained in:
parent
c9a77d03dd
commit
5d1bdc320b
14 changed files with 40 additions and 23 deletions
|
@ -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)
|
||||
}),
|
||||
|
|
|
@ -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)
|
||||
})
|
||||
|
|
|
@ -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)
|
||||
})
|
||||
|
|
|
@ -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)
|
||||
})
|
||||
|
|
|
@ -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)
|
||||
}),
|
||||
|
|
|
@ -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)
|
||||
}),
|
||||
|
|
|
@ -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)
|
||||
})
|
||||
|
|
|
@ -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)
|
||||
}),
|
||||
|
|
|
@ -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
|
||||
|
@ -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};
|
||||
|
@ -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
|
||||
|
@ -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>>,
|
||||
}
|
||||
|
@ -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,
|
||||
|
|
|
@ -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)
|
||||
})
|
||||
|
|
|
@ -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)
|
||||
}),
|
||||
|
|
|
@ -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)
|
||||
})
|
||||
|
|
|
@ -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)
|
||||
})
|
||||
|
|
|
@ -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 \
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue