From a7527adf0855002dcbd1cb12309d4f7fbfd93e16 Mon Sep 17 00:00:00 2001 From: llogiq Date: Wed, 12 Aug 2015 13:49:28 +0200 Subject: [PATCH 01/16] First (incomplete) const folding --- src/const.rs | 131 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 131 insertions(+) create mode 100644 src/const.rs diff --git a/src/const.rs b/src/const.rs new file mode 100644 index 000000000000..924a47f2e5a7 --- /dev/null +++ b/src/const.rs @@ -0,0 +1,131 @@ +use rustc::lint::Context; +use rustc::middle::const_eval::lookup_const_by_id; +use syntax::ast::*; +use syntax::ptr::P; + +/// a Lit_-like enum to fold constant `Expr`s into +#[derive(PartialEq, Eq, Debug, Clone)] +pub enum Constant { + ConstantStr(&'static str, StrStyle), + ConstantBinary(Rc>), + ConstantByte(u8), + ConstantChar(char), + ConstantInt(u64, LitIntType), + ConstantFloat(Cow<'static, str>, FloatTy), + ConstantFloatUnsuffixed(Cow<'static, str>), + ConstantBool(bool), + ConstantVec(Vec), + ConstantTuple(Vec), +} + +/// simple constant folding +pub fn constant(cx: &Context, e: &Expr) -> Option { + match e { + &ExprParen(ref inner) => constant(cx, inner), + &ExprPath(_, _) => fetch_path(cx, e), + &ExprBlock(ref block) => constant_block(cx, inner), + &ExprIf(ref cond, ref then, ref otherwise) => + match constant(cx, cond) { + Some(LitBool(true)) => constant(cx, then), + Some(LitBool(false)) => constant(cx, otherwise), + _ => None, + }, + &ExprLit(ref lit) => Some(lit_to_constant(lit)), + &ExprVec(ref vec) => constant_vec(cx, vec), + &ExprTup(ref tup) => constant_tup(cx, tup), + &ExprUnary(op, ref operand) => constant(cx, operand).and_then( + |o| match op { + UnNot => + if let ConstantBool(b) = o { + Some(ConstantBool(!b)) + } else { None }, + UnNeg => + match o { + &ConstantInt(value, ty) => + Some(ConstantInt(value, match ty { + SignedIntLit(ity, sign) => + SignedIntLit(ity, neg_sign(sign)), + UnsuffixedIntLit(sign) => + UnsuffixedIntLit(neg_sign(sign)), + _ => { return None; }, + })), + &LitFloat(ref is, ref ty) => + Some(ConstantFloat(neg_float_str(is), ty)), + &LitFloatUnsuffixed(ref is) => + Some(ConstantFloatUnsuffixed(neg_float_str(is))), + _ => None, + }, + UnUniq | UnDeref => o, + }), + //TODO: add other expressions + _ => None, + } +} + +fn lit_to_constant(lit: &Lit_) -> Constant { + match lit { + &LitStr(ref is, style) => ConstantStr(&*is, style), + &LitBinary(ref blob) => ConstantBinary(blob.clone()), + &LitByte(b) => ConstantByte(b), + &LitChar(c) => ConstantChar(c), + &LitInt(value, ty) => ConstantInt(value, ty), + &LitFloat(ref is, ty) => ConstantFloat(Cow::Borrowed(&*is), ty), + &LitFloatUnsuffixed(InternedString) => + ConstantFloatUnsuffixed(Cow::Borrowed(&*is)), + &LitBool(b) => ConstantBool(b), + } +} + +/// create `Some(ConstantVec(..))` of all constants, unless there is any +/// non-constant part +fn constant_vec(cx: &Context, vec: &[&Expr]) -> Option { + Vec parts = Vec::new(); + for opt_part in vec { + match constant(cx, opt_part) { + Some(ref p) => parts.push(p), + None => { return None; }, + } + } + Some(ConstantVec(parts)) +} + +fn constant_tup(cx, &Context, tup: &[&Expr]) -> Option { + Vec parts = Vec::new(); + for opt_part in vec { + match constant(cx, opt_part) { + Some(ref p) => parts.push(p), + None => { return None; }, + } + } + Some(ConstantTuple(parts)) +} + +/// lookup a possibly constant expression from a ExprPath +fn fetch_path(cx: &Context, e: &Expr) -> Option { + if let Some(&PathResolution { base_def: DefConst(id), ..}) = + cx.tcx.def_map.borrow().get(&e.id) { + lookup_const_by_id(cx.tcx, id, None).map(|l| constant(cx, l)) + } else { None } +} + +/// A block can only yield a constant if it only has one constant expression +fn constant_block(cx: &Context, block: &Block) -> Option { + if block.stmts.is_empty() { + block.expr.map(|b| constant(cx, b)) + } else { None } +} + +fn neg_sign(s: Sign) -> Sign { + match s: + Sign::Plus => Sign::Minus, + Sign::Minus => Sign::Plus, + } +} + +fn neg_float_str(s: &InternedString) -> Cow<'static, str> { + if s.startsWith('-') { + Cow::Borrowed(s[1..]) + } else { + Cow::Owned(format!("-{}", &*s)) + } +} From fc6dfafc306568def3b58a6a91fe56289b53242f Mon Sep 17 00:00:00 2001 From: llogiq Date: Wed, 12 Aug 2015 14:02:13 +0200 Subject: [PATCH 02/16] fixed if-condition match --- src/const.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/const.rs b/src/const.rs index 924a47f2e5a7..a353de218f48 100644 --- a/src/const.rs +++ b/src/const.rs @@ -26,8 +26,8 @@ pub fn constant(cx: &Context, e: &Expr) -> Option { &ExprBlock(ref block) => constant_block(cx, inner), &ExprIf(ref cond, ref then, ref otherwise) => match constant(cx, cond) { - Some(LitBool(true)) => constant(cx, then), - Some(LitBool(false)) => constant(cx, otherwise), + Some(ConstantBool(true)) => constant(cx, then), + Some(ConstantBool(false)) => constant(cx, otherwise), _ => None, }, &ExprLit(ref lit) => Some(lit_to_constant(lit)), From 38e8d2bc060607ebf9a37919752e1de15f1794fa Mon Sep 17 00:00:00 2001 From: Georg Brandl Date: Wed, 12 Aug 2015 13:58:55 +0200 Subject: [PATCH 03/16] methods: move misc.StrToStringPass to MethodsPass --- src/lib.rs | 4 ++-- src/methods.rs | 12 ++++++++++-- src/misc.rs | 30 ------------------------------ 3 files changed, 12 insertions(+), 34 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index fef678f668d8..7b47edaa4965 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -37,7 +37,6 @@ pub mod returns; pub fn plugin_registrar(reg: &mut Registry) { reg.register_lint_pass(box types::TypePass as LintPassObject); reg.register_lint_pass(box misc::MiscPass as LintPassObject); - reg.register_lint_pass(box misc::StrToStringPass as LintPassObject); reg.register_lint_pass(box misc::TopLevelRefPass as LintPassObject); reg.register_lint_pass(box misc::CmpNan as LintPassObject); reg.register_lint_pass(box eq_op::EqOp as LintPassObject); @@ -61,7 +60,7 @@ pub fn plugin_registrar(reg: &mut Registry) { reg.register_lint_pass(box methods::MethodsPass as LintPassObject); reg.register_lint_group("clippy", vec![types::BOX_VEC, types::LINKEDLIST, - misc::SINGLE_MATCH, misc::STR_TO_STRING, + misc::SINGLE_MATCH, misc::TOPLEVEL_REF_ARG, eq_op::EQ_OP, bit_mask::BAD_BIT_MASK, bit_mask::INEFFECTIVE_BIT_MASK, @@ -83,5 +82,6 @@ pub fn plugin_registrar(reg: &mut Registry) { misc::MODULO_ONE, methods::OPTION_UNWRAP_USED, methods::RESULT_UNWRAP_USED, + methods::STR_TO_STRING, ]); } diff --git a/src/methods.rs b/src/methods.rs index 3d9aa8c6ffc1..f02e06640923 100644 --- a/src/methods.rs +++ b/src/methods.rs @@ -11,16 +11,19 @@ declare_lint!(pub OPTION_UNWRAP_USED, Warn, "Warn on using unwrap() on an Option value"); declare_lint!(pub RESULT_UNWRAP_USED, Allow, "Warn on using unwrap() on a Result value"); +declare_lint!(pub STR_TO_STRING, Warn, + "Warn when a String could use to_owned() instead of to_string()"); impl LintPass for MethodsPass { fn get_lints(&self) -> LintArray { - lint_array!(OPTION_UNWRAP_USED, RESULT_UNWRAP_USED) + lint_array!(OPTION_UNWRAP_USED, RESULT_UNWRAP_USED, STR_TO_STRING) } fn check_expr(&mut self, cx: &Context, expr: &Expr) { if let ExprMethodCall(ref ident, _, ref args) = expr.node { + let ref obj_ty = walk_ptrs_ty(cx.tcx.expr_ty(&*args[0])).sty; if ident.node.name == "unwrap" { - if let ty::TyEnum(did, _) = walk_ptrs_ty(cx.tcx.expr_ty(&*args[0])).sty { + if let ty::TyEnum(did, _) = *obj_ty { if match_def_path(cx, did.did, &["core", "option", "Option"]) { span_lint(cx, OPTION_UNWRAP_USED, expr.span, "used unwrap() on an Option value. If you don't want \ @@ -34,6 +37,11 @@ impl LintPass for MethodsPass { } } } + else if ident.node.name == "to_string" { + if let ty::TyStr = *obj_ty { + span_lint(cx, STR_TO_STRING, expr.span, "`str.to_owned()` is faster"); + } + } } } } diff --git a/src/misc.rs b/src/misc.rs index fa1847aad9a2..d5e1efe2cc3a 100644 --- a/src/misc.rs +++ b/src/misc.rs @@ -59,36 +59,6 @@ impl LintPass for MiscPass { } -declare_lint!(pub STR_TO_STRING, Warn, "Warn when a String could use to_owned() instead of to_string()"); - -#[allow(missing_copy_implementations)] -pub struct StrToStringPass; - -impl LintPass for StrToStringPass { - fn get_lints(&self) -> LintArray { - lint_array!(STR_TO_STRING) - } - - fn check_expr(&mut self, cx: &Context, expr: &ast::Expr) { - match expr.node { - ast::ExprMethodCall(ref method, _, ref args) - if method.node.name == "to_string" - && is_str(cx, &*args[0]) => { - span_lint(cx, STR_TO_STRING, expr.span, "`str.to_owned()` is faster"); - }, - _ => () - } - - fn is_str(cx: &Context, expr: &ast::Expr) -> bool { - match walk_ptrs_ty(cx.tcx.expr_ty(expr)).sty { - ty::TyStr => true, - _ => false - } - } - } -} - - declare_lint!(pub TOPLEVEL_REF_ARG, Warn, "Warn about pattern matches with top-level `ref` bindings"); #[allow(missing_copy_implementations)] From 45b95537574182be0b7ef6301cdbafe11891ec95 Mon Sep 17 00:00:00 2001 From: llogiq Date: Thu, 13 Aug 2015 09:25:44 +0200 Subject: [PATCH 04/16] added follow flag --- src/const.rs | 154 +++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 113 insertions(+), 41 deletions(-) diff --git a/src/const.rs b/src/const.rs index a353de218f48..38044bbea5cc 100644 --- a/src/const.rs +++ b/src/const.rs @@ -3,60 +3,74 @@ use rustc::middle::const_eval::lookup_const_by_id; use syntax::ast::*; use syntax::ptr::P; +pub enum FloatWidth { + Fw32, + Fw64, + FwAny +} + +impl From for FloatWidth { + fn from(ty: FloatTy) -> FloatWidth { + match ty { + TyF32 => Fw32, + TyF64 => Fw64, + } + } +} + /// a Lit_-like enum to fold constant `Expr`s into #[derive(PartialEq, Eq, Debug, Clone)] pub enum Constant { + /// a String "abc" ConstantStr(&'static str, StrStyle), + /// a Binary String b"abc" ConstantBinary(Rc>), + /// a single byte b'a' ConstantByte(u8), + /// a single char 'a' ConstantChar(char), + /// an integer ConstantInt(u64, LitIntType), - ConstantFloat(Cow<'static, str>, FloatTy), - ConstantFloatUnsuffixed(Cow<'static, str>), + /// a float with given type + ConstantFloat(Cow<'static, str>, FloatWidth), + /// true or false ConstantBool(bool), + /// an array of constants ConstantVec(Vec), + /// also an array, but with only one constant, repeated N times + ConstantRepeat(Constant, usize), + /// a tuple of constants ConstantTuple(Vec), } /// simple constant folding -pub fn constant(cx: &Context, e: &Expr) -> Option { +pub fn constant(cx: &Context, e: &Expr, follow: bool) -> Option { match e { - &ExprParen(ref inner) => constant(cx, inner), - &ExprPath(_, _) => fetch_path(cx, e), - &ExprBlock(ref block) => constant_block(cx, inner), - &ExprIf(ref cond, ref then, ref otherwise) => + &ExprParen(ref inner) => constant(cx, inner, follow), + &ExprPath(_, _) => if follow { fetch_path(cx, e) } else { None }, + &ExprBlock(ref block) => constant_block(cx, inner, follow), + &ExprIf(ref cond, ref then, ref otherwise) => match constant(cx, cond) { - Some(ConstantBool(true)) => constant(cx, then), - Some(ConstantBool(false)) => constant(cx, otherwise), + Some(ConstantBool(true)) => constant(cx, then, follow), + Some(ConstantBool(false)) => constant(cx, otherwise, follow), _ => None, }, &ExprLit(ref lit) => Some(lit_to_constant(lit)), - &ExprVec(ref vec) => constant_vec(cx, vec), - &ExprTup(ref tup) => constant_tup(cx, tup), - &ExprUnary(op, ref operand) => constant(cx, operand).and_then( + &ExprVec(ref vec) => constant_vec(cx, vec, follow), + &ExprTup(ref tup) => constant_tup(cx, tup, follow), + &ExprRepeat(ref value, ref number) => + constant_binop_apply(cx, value, number,|v, n| ConstantRepeat(v, n)), + &ExprUnary(op, ref operand) => constant(cx, operand, follow).and_then( |o| match op { UnNot => if let ConstantBool(b) = o { Some(ConstantBool(!b)) } else { None }, - UnNeg => - match o { - &ConstantInt(value, ty) => - Some(ConstantInt(value, match ty { - SignedIntLit(ity, sign) => - SignedIntLit(ity, neg_sign(sign)), - UnsuffixedIntLit(sign) => - UnsuffixedIntLit(neg_sign(sign)), - _ => { return None; }, - })), - &LitFloat(ref is, ref ty) => - Some(ConstantFloat(neg_float_str(is), ty)), - &LitFloatUnsuffixed(ref is) => - Some(ConstantFloatUnsuffixed(neg_float_str(is))), - _ => None, - }, + UnNeg => constant_negate(o), UnUniq | UnDeref => o, }), + &ExprBinary(op, ref left, ref right) => + constant_binop(cx, op, left, right, follow), //TODO: add other expressions _ => None, } @@ -69,19 +83,19 @@ fn lit_to_constant(lit: &Lit_) -> Constant { &LitByte(b) => ConstantByte(b), &LitChar(c) => ConstantChar(c), &LitInt(value, ty) => ConstantInt(value, ty), - &LitFloat(ref is, ty) => ConstantFloat(Cow::Borrowed(&*is), ty), - &LitFloatUnsuffixed(InternedString) => - ConstantFloatUnsuffixed(Cow::Borrowed(&*is)), + &LitFloat(ref is, ty) => ConstantFloat(Cow::Borrowed(&*is), ty.into()), + &LitFloatUnsuffixed(InternedString) => + ConstantFloat(Cow::Borrowed(&*is), FwAny), &LitBool(b) => ConstantBool(b), } } /// create `Some(ConstantVec(..))` of all constants, unless there is any /// non-constant part -fn constant_vec(cx: &Context, vec: &[&Expr]) -> Option { - Vec parts = Vec::new(); +fn constant_vec(cx: &Context, vec: &[&Expr], follow: bool) -> Option { + parts = Vec::new(); for opt_part in vec { - match constant(cx, opt_part) { + match constant(cx, opt_part, follow) { Some(ref p) => parts.push(p), None => { return None; }, } @@ -89,10 +103,10 @@ fn constant_vec(cx: &Context, vec: &[&Expr]) -> Option { Some(ConstantVec(parts)) } -fn constant_tup(cx, &Context, tup: &[&Expr]) -> Option { - Vec parts = Vec::new(); +fn constant_tup(cx: &Context, tup: &[&Expr], follow: bool) -> Option { + parts = Vec::new(); for opt_part in vec { - match constant(cx, opt_part) { + match constant(cx, opt_part, follow) { Some(ref p) => parts.push(p), None => { return None; }, } @@ -109,23 +123,81 @@ fn fetch_path(cx: &Context, e: &Expr) -> Option { } /// A block can only yield a constant if it only has one constant expression -fn constant_block(cx: &Context, block: &Block) -> Option { +fn constant_block(cx: &Context, block: &Block, follow: bool) -> Option { if block.stmts.is_empty() { - block.expr.map(|b| constant(cx, b)) + block.expr.map(|b| constant(cx, b, follow)) } else { None } } +fn constant_negate(o: Constant) -> Option { + match o { + &ConstantInt(value, ty) => + Some(ConstantInt(value, match ty { + SignedIntLit(ity, sign) => SignedIntLit(ity, neg_sign(sign)), + UnsuffixedIntLit(sign) => UnsuffixedIntLit(neg_sign(sign)), + _ => { return None; }, + })), + &LitFloat(ref is, ref ty) => Some(ConstantFloat(neg_float_str(is), ty)), + _ => None, + } +} + fn neg_sign(s: Sign) -> Sign { - match s: + match s { Sign::Plus => Sign::Minus, Sign::Minus => Sign::Plus, } } fn neg_float_str(s: &InternedString) -> Cow<'static, str> { - if s.startsWith('-') { + if s.startsWith('-') { Cow::Borrowed(s[1..]) } else { Cow::Owned(format!("-{}", &*s)) } } + +fn constant_binop(cx: &Context, op: BinOp, left: &Expr, right: &Expr, + follow: bool) -> Option { + match op.node { + //BiAdd, + //BiSub, + //BiMul, + //BiDiv, + //BiRem, + BiAnd => constant_short_circuit(cx, left, right, false, follow), + BiOr => constant_short_circuit(cx, left, right, true, follow), + //BiBitXor, + //BiBitAnd, + //BiBitOr, + //BiShl, + //BiShr, + //BiEq, + //BiLt, + //BiLe, + //BiNe, + //BiGe, + //BiGt, + _ => None, + } +} + +fn constant_binop_apply(cx: &Context, left: &Expr, right: &Expr, op: F, + follow: bool) -> Option +where F: FnMut(Constant, Constant) -> Option { + constant(cx, left, follow).and_then(|l| constant(cx, right, follow) + .and_then(|r| op(l, r))) +} + +fn constant_short_circuit(cx: &Context, left: &Expr, right: &Expr, b: bool, + follow: bool) -> Option { + if let ConstantBool(lbool) = constant(cx, left, follow) { + if l == b { + Some(ConstantBool(b)) + } else { + if let ConstantBool(rbool) = constant(cx, right, follow) { + Some(ConstantBool(rbool)) + } else { None } + } + } else { None } +} From 12c974e21abc70689c59027adcf42ff3f4b29e84 Mon Sep 17 00:00:00 2001 From: llogiq Date: Thu, 13 Aug 2015 10:45:30 +0200 Subject: [PATCH 05/16] changed Constant to a struct with 'needed_resolution' bool --- src/const.rs | 202 +++++++++++++++++++++++++++++++++++---------------- 1 file changed, 138 insertions(+), 64 deletions(-) diff --git a/src/const.rs b/src/const.rs index 38044bbea5cc..cef2cd7f9fc0 100644 --- a/src/const.rs +++ b/src/const.rs @@ -18,9 +18,25 @@ impl From for FloatWidth { } } +#[derive(PartialEq, Eq, Debug, Clone)] +pub struct Constant { + constant: ConstantVariant, + needed_resolution: bool +} + +impl Constant { + fn new(variant: ConstantVariant) -> Constant { + Constant { constant: variant, needed_resolution: false } + } + + fn new_resolved(variant: ConstantVariant) -> Constant { + Constant { constant: variant, needed_resolution: true } + } +} + /// a Lit_-like enum to fold constant `Expr`s into #[derive(PartialEq, Eq, Debug, Clone)] -pub enum Constant { +pub enum ConstantVariant { /// a String "abc" ConstantStr(&'static str, StrStyle), /// a Binary String b"abc" @@ -43,34 +59,51 @@ pub enum Constant { ConstantTuple(Vec), } -/// simple constant folding -pub fn constant(cx: &Context, e: &Expr, follow: bool) -> Option { +impl ConstantVariant { + /// convert to u64 if possible + /// + /// # panics + /// + /// if the constant could not be converted to u64 losslessly + fn as_u64(&self) -> u64 { + if let &ConstantInt(val, _) = self { + val // TODO we may want to check the sign if any + } else { + panic!("Could not convert a {:?} to u64"); + } + } +} + +/// simple constant folding: Insert an expression, get a constant or none. +pub fn constant(cx: &Context, e: &Expr) -> Option { match e { - &ExprParen(ref inner) => constant(cx, inner, follow), - &ExprPath(_, _) => if follow { fetch_path(cx, e) } else { None }, - &ExprBlock(ref block) => constant_block(cx, inner, follow), + &ExprParen(ref inner) => constant(cx, inner), + &ExprPath(_, _) => fetch_path(cx, e), + &ExprBlock(ref block) => constant_block(cx, inner), &ExprIf(ref cond, ref then, ref otherwise) => - match constant(cx, cond) { - Some(ConstantBool(true)) => constant(cx, then, follow), - Some(ConstantBool(false)) => constant(cx, otherwise, follow), - _ => None, - }, + constant_if(cx, cond, then, otherwise), &ExprLit(ref lit) => Some(lit_to_constant(lit)), - &ExprVec(ref vec) => constant_vec(cx, vec, follow), - &ExprTup(ref tup) => constant_tup(cx, tup, follow), + &ExprVec(ref vec) => constant_vec(cx, vec), + &ExprTup(ref tup) => constant_tup(cx, tup), &ExprRepeat(ref value, ref number) => - constant_binop_apply(cx, value, number,|v, n| ConstantRepeat(v, n)), - &ExprUnary(op, ref operand) => constant(cx, operand, follow).and_then( + constant_binop_apply(cx, value, number,|v, n| Constant { + constant: ConstantRepeat(v, n.constant.as_u64()), + needed_resolution: v.needed_resolution || n.needed_resolution + }), + &ExprUnary(op, ref operand) => constant(cx, operand).and_then( |o| match op { UnNot => - if let ConstantBool(b) = o { - Some(ConstantBool(!b)) + if let ConstantBool(b) = o.variant { + Some(Constant{ + needed_resolution: o.needed_resolution, + constant: ConstantBool(!b), + }) } else { None }, UnNeg => constant_negate(o), UnUniq | UnDeref => o, }), &ExprBinary(op, ref left, ref right) => - constant_binop(cx, op, left, right, follow), + constant_binop(op, left, right), //TODO: add other expressions _ => None, } @@ -78,68 +111,100 @@ pub fn constant(cx: &Context, e: &Expr, follow: bool) -> Option { fn lit_to_constant(lit: &Lit_) -> Constant { match lit { - &LitStr(ref is, style) => ConstantStr(&*is, style), - &LitBinary(ref blob) => ConstantBinary(blob.clone()), - &LitByte(b) => ConstantByte(b), - &LitChar(c) => ConstantChar(c), - &LitInt(value, ty) => ConstantInt(value, ty), - &LitFloat(ref is, ty) => ConstantFloat(Cow::Borrowed(&*is), ty.into()), + &LitStr(ref is, style) => Constant::new(ConstantStr(&*is, style)), + &LitBinary(ref blob) => Constant::new(ConstantBinary(blob.clone())), + &LitByte(b) => Constant::new(ConstantByte(b)), + &LitChar(c) => Constant::new(ConstantChar(c)), + &LitInt(value, ty) => Constant::new(ConstantInt(value, ty)), + &LitFloat(ref is, ty) => + Constant::new(ConstantFloat(Cow::Borrowed(&*is), ty.into())), &LitFloatUnsuffixed(InternedString) => - ConstantFloat(Cow::Borrowed(&*is), FwAny), - &LitBool(b) => ConstantBool(b), + Constant::new(ConstantFloat(Cow::Borrowed(&*is), FwAny)), + &LitBool(b) => Constant::new(ConstantBool(b)), } } /// create `Some(ConstantVec(..))` of all constants, unless there is any /// non-constant part -fn constant_vec(cx: &Context, vec: &[&Expr], follow: bool) -> Option { - parts = Vec::new(); +fn constant_vec(cx: &Context, vec: &[&Expr]) -> Option { + let mut parts = Vec::new(); + let mut resolved = false; for opt_part in vec { - match constant(cx, opt_part, follow) { - Some(ref p) => parts.push(p), + match constant(cx, opt_part) { + Some(ref p) => { + resolved |= p.needed_resolution; + parts.push(p) + }, None => { return None; }, } } - Some(ConstantVec(parts)) + Some(Constant { + constant: ConstantVec(parts), + needed_resolution: resolved + }) } -fn constant_tup(cx: &Context, tup: &[&Expr], follow: bool) -> Option { - parts = Vec::new(); +fn constant_tup(cx: &Context, tup: &[&Expr]) -> Option { + let mut parts = Vec::new(); + let mut resolved = false; for opt_part in vec { - match constant(cx, opt_part, follow) { - Some(ref p) => parts.push(p), + match constant(cx, opt_part) { + Some(ref p) => { + resolved |= p.needed_resolution; + parts.push(p) + }, None => { return None; }, } } - Some(ConstantTuple(parts)) + Some(Constant { + constant: ConstantTuple(parts), + needed_resolution: resolved + }) } /// lookup a possibly constant expression from a ExprPath fn fetch_path(cx: &Context, e: &Expr) -> Option { if let Some(&PathResolution { base_def: DefConst(id), ..}) = cx.tcx.def_map.borrow().get(&e.id) { - lookup_const_by_id(cx.tcx, id, None).map(|l| constant(cx, l)) + lookup_const_by_id(cx.tcx, id, None).map( + |l| Constant::new_resolved(constant(cx, l).constant)) } else { None } } /// A block can only yield a constant if it only has one constant expression -fn constant_block(cx: &Context, block: &Block, follow: bool) -> Option { +fn constant_block(cx: &Context, block: &Block) -> Option { if block.stmts.is_empty() { - block.expr.map(|b| constant(cx, b, follow)) + block.expr.map(|b| constant(cx, b)) + } else { None } +} + +fn constant_if(cx: &Context, cond: &Expr, then: &Expr, otherwise: &Expr) -> + Option { + if let Some(Constant{ constant: ConstantBool(b), needed_resolution: res }) = + constant(cx, cond) { + let part = constant(cx, if b { then } else { otherwise }); + Some(Constant { + constant: part.constant, + needed_resolution: res || part.needed_resolution, + }) } else { None } } fn constant_negate(o: Constant) -> Option { - match o { - &ConstantInt(value, ty) => - Some(ConstantInt(value, match ty { - SignedIntLit(ity, sign) => SignedIntLit(ity, neg_sign(sign)), - UnsuffixedIntLit(sign) => UnsuffixedIntLit(neg_sign(sign)), - _ => { return None; }, - })), - &LitFloat(ref is, ref ty) => Some(ConstantFloat(neg_float_str(is), ty)), - _ => None, - } + Some(Constant{ + needed_resolution: o.needed_resolution, + constant: match o.constant { + &ConstantInt(value, ty) => + ConstantInt(value, match ty { + SignedIntLit(ity, sign) => + SignedIntLit(ity, neg_sign(sign)), + UnsuffixedIntLit(sign) => UnsuffixedIntLit(neg_sign(sign)), + _ => { return None; }, + }), + &LitFloat(ref is, ref ty) => ConstantFloat(neg_float_str(is), ty), + _ => { return None; }, + } + }) } fn neg_sign(s: Sign) -> Sign { @@ -157,16 +222,16 @@ fn neg_float_str(s: &InternedString) -> Cow<'static, str> { } } -fn constant_binop(cx: &Context, op: BinOp, left: &Expr, right: &Expr, - follow: bool) -> Option { +fn constant_binop(cx: &Context, op: BinOp, left: &Expr, right: &Expr) + -> Option { match op.node { //BiAdd, //BiSub, //BiMul, //BiDiv, //BiRem, - BiAnd => constant_short_circuit(cx, left, right, false, follow), - BiOr => constant_short_circuit(cx, left, right, true, follow), + BiAnd => constant_short_circuit(cx, left, right, false), + BiOr => constant_short_circuit(cx, left, right, true), //BiBitXor, //BiBitAnd, //BiBitOr, @@ -182,21 +247,30 @@ fn constant_binop(cx: &Context, op: BinOp, left: &Expr, right: &Expr, } } -fn constant_binop_apply(cx: &Context, left: &Expr, right: &Expr, op: F, - follow: bool) -> Option -where F: FnMut(Constant, Constant) -> Option { - constant(cx, left, follow).and_then(|l| constant(cx, right, follow) - .and_then(|r| op(l, r))) +fn constant_binop_apply(cx: &Context, left: &Expr, right: &Expr, op: F) + -> Option +where F: FnMut(ConstantVariant, ConstantVariant) -> Option { + constant(cx, left).and_then(|l| constant(cx, right).and_then( + |r| Constant { + needed_resolution: l.needed_resolution || r.needed_resolution, + constant: op(l.constant, r.constant) + })) } -fn constant_short_circuit(cx: &Context, left: &Expr, right: &Expr, b: bool, - follow: bool) -> Option { - if let ConstantBool(lbool) = constant(cx, left, follow) { +fn constant_short_circuit(cx: &Context, left: &Expr, right: &Expr, b: bool) -> + Option { + let leftconst = constant(cx, left); + if let ConstantBool(lbool) = leftconst.constant { if l == b { - Some(ConstantBool(b)) + Some(leftconst) } else { - if let ConstantBool(rbool) = constant(cx, right, follow) { - Some(ConstantBool(rbool)) + let rightconst = constant(cx, right); + if let ConstantBool(rbool) = rightconst.constant { + Some(Constant { + constant: rightconst.constant, + needed_resolution: leftconst.needed_resolution || + rightconst.needed_resolution, + }) } else { None } } } else { None } From 8b9c2a79ed9bf4d25f61513017e9a254d0b02d93 Mon Sep 17 00:00:00 2001 From: llogiq Date: Wed, 12 Aug 2015 13:49:28 +0200 Subject: [PATCH 06/16] First (incomplete) const folding --- src/const.rs | 131 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 131 insertions(+) create mode 100644 src/const.rs diff --git a/src/const.rs b/src/const.rs new file mode 100644 index 000000000000..924a47f2e5a7 --- /dev/null +++ b/src/const.rs @@ -0,0 +1,131 @@ +use rustc::lint::Context; +use rustc::middle::const_eval::lookup_const_by_id; +use syntax::ast::*; +use syntax::ptr::P; + +/// a Lit_-like enum to fold constant `Expr`s into +#[derive(PartialEq, Eq, Debug, Clone)] +pub enum Constant { + ConstantStr(&'static str, StrStyle), + ConstantBinary(Rc>), + ConstantByte(u8), + ConstantChar(char), + ConstantInt(u64, LitIntType), + ConstantFloat(Cow<'static, str>, FloatTy), + ConstantFloatUnsuffixed(Cow<'static, str>), + ConstantBool(bool), + ConstantVec(Vec), + ConstantTuple(Vec), +} + +/// simple constant folding +pub fn constant(cx: &Context, e: &Expr) -> Option { + match e { + &ExprParen(ref inner) => constant(cx, inner), + &ExprPath(_, _) => fetch_path(cx, e), + &ExprBlock(ref block) => constant_block(cx, inner), + &ExprIf(ref cond, ref then, ref otherwise) => + match constant(cx, cond) { + Some(LitBool(true)) => constant(cx, then), + Some(LitBool(false)) => constant(cx, otherwise), + _ => None, + }, + &ExprLit(ref lit) => Some(lit_to_constant(lit)), + &ExprVec(ref vec) => constant_vec(cx, vec), + &ExprTup(ref tup) => constant_tup(cx, tup), + &ExprUnary(op, ref operand) => constant(cx, operand).and_then( + |o| match op { + UnNot => + if let ConstantBool(b) = o { + Some(ConstantBool(!b)) + } else { None }, + UnNeg => + match o { + &ConstantInt(value, ty) => + Some(ConstantInt(value, match ty { + SignedIntLit(ity, sign) => + SignedIntLit(ity, neg_sign(sign)), + UnsuffixedIntLit(sign) => + UnsuffixedIntLit(neg_sign(sign)), + _ => { return None; }, + })), + &LitFloat(ref is, ref ty) => + Some(ConstantFloat(neg_float_str(is), ty)), + &LitFloatUnsuffixed(ref is) => + Some(ConstantFloatUnsuffixed(neg_float_str(is))), + _ => None, + }, + UnUniq | UnDeref => o, + }), + //TODO: add other expressions + _ => None, + } +} + +fn lit_to_constant(lit: &Lit_) -> Constant { + match lit { + &LitStr(ref is, style) => ConstantStr(&*is, style), + &LitBinary(ref blob) => ConstantBinary(blob.clone()), + &LitByte(b) => ConstantByte(b), + &LitChar(c) => ConstantChar(c), + &LitInt(value, ty) => ConstantInt(value, ty), + &LitFloat(ref is, ty) => ConstantFloat(Cow::Borrowed(&*is), ty), + &LitFloatUnsuffixed(InternedString) => + ConstantFloatUnsuffixed(Cow::Borrowed(&*is)), + &LitBool(b) => ConstantBool(b), + } +} + +/// create `Some(ConstantVec(..))` of all constants, unless there is any +/// non-constant part +fn constant_vec(cx: &Context, vec: &[&Expr]) -> Option { + Vec parts = Vec::new(); + for opt_part in vec { + match constant(cx, opt_part) { + Some(ref p) => parts.push(p), + None => { return None; }, + } + } + Some(ConstantVec(parts)) +} + +fn constant_tup(cx, &Context, tup: &[&Expr]) -> Option { + Vec parts = Vec::new(); + for opt_part in vec { + match constant(cx, opt_part) { + Some(ref p) => parts.push(p), + None => { return None; }, + } + } + Some(ConstantTuple(parts)) +} + +/// lookup a possibly constant expression from a ExprPath +fn fetch_path(cx: &Context, e: &Expr) -> Option { + if let Some(&PathResolution { base_def: DefConst(id), ..}) = + cx.tcx.def_map.borrow().get(&e.id) { + lookup_const_by_id(cx.tcx, id, None).map(|l| constant(cx, l)) + } else { None } +} + +/// A block can only yield a constant if it only has one constant expression +fn constant_block(cx: &Context, block: &Block) -> Option { + if block.stmts.is_empty() { + block.expr.map(|b| constant(cx, b)) + } else { None } +} + +fn neg_sign(s: Sign) -> Sign { + match s: + Sign::Plus => Sign::Minus, + Sign::Minus => Sign::Plus, + } +} + +fn neg_float_str(s: &InternedString) -> Cow<'static, str> { + if s.startsWith('-') { + Cow::Borrowed(s[1..]) + } else { + Cow::Owned(format!("-{}", &*s)) + } +} From 6aeb9552148315641e49d7ab791ef6ffa638fcbe Mon Sep 17 00:00:00 2001 From: llogiq Date: Wed, 12 Aug 2015 14:02:13 +0200 Subject: [PATCH 07/16] fixed if-condition match --- src/const.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/const.rs b/src/const.rs index 924a47f2e5a7..a353de218f48 100644 --- a/src/const.rs +++ b/src/const.rs @@ -26,8 +26,8 @@ pub fn constant(cx: &Context, e: &Expr) -> Option { &ExprBlock(ref block) => constant_block(cx, inner), &ExprIf(ref cond, ref then, ref otherwise) => match constant(cx, cond) { - Some(LitBool(true)) => constant(cx, then), - Some(LitBool(false)) => constant(cx, otherwise), + Some(ConstantBool(true)) => constant(cx, then), + Some(ConstantBool(false)) => constant(cx, otherwise), _ => None, }, &ExprLit(ref lit) => Some(lit_to_constant(lit)), From a2f19f2a380d1027436c30edc52992edfcd50cb1 Mon Sep 17 00:00:00 2001 From: llogiq Date: Thu, 13 Aug 2015 09:25:44 +0200 Subject: [PATCH 08/16] added follow flag --- src/const.rs | 154 +++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 113 insertions(+), 41 deletions(-) diff --git a/src/const.rs b/src/const.rs index a353de218f48..38044bbea5cc 100644 --- a/src/const.rs +++ b/src/const.rs @@ -3,60 +3,74 @@ use rustc::middle::const_eval::lookup_const_by_id; use syntax::ast::*; use syntax::ptr::P; +pub enum FloatWidth { + Fw32, + Fw64, + FwAny +} + +impl From for FloatWidth { + fn from(ty: FloatTy) -> FloatWidth { + match ty { + TyF32 => Fw32, + TyF64 => Fw64, + } + } +} + /// a Lit_-like enum to fold constant `Expr`s into #[derive(PartialEq, Eq, Debug, Clone)] pub enum Constant { + /// a String "abc" ConstantStr(&'static str, StrStyle), + /// a Binary String b"abc" ConstantBinary(Rc>), + /// a single byte b'a' ConstantByte(u8), + /// a single char 'a' ConstantChar(char), + /// an integer ConstantInt(u64, LitIntType), - ConstantFloat(Cow<'static, str>, FloatTy), - ConstantFloatUnsuffixed(Cow<'static, str>), + /// a float with given type + ConstantFloat(Cow<'static, str>, FloatWidth), + /// true or false ConstantBool(bool), + /// an array of constants ConstantVec(Vec), + /// also an array, but with only one constant, repeated N times + ConstantRepeat(Constant, usize), + /// a tuple of constants ConstantTuple(Vec), } /// simple constant folding -pub fn constant(cx: &Context, e: &Expr) -> Option { +pub fn constant(cx: &Context, e: &Expr, follow: bool) -> Option { match e { - &ExprParen(ref inner) => constant(cx, inner), - &ExprPath(_, _) => fetch_path(cx, e), - &ExprBlock(ref block) => constant_block(cx, inner), - &ExprIf(ref cond, ref then, ref otherwise) => + &ExprParen(ref inner) => constant(cx, inner, follow), + &ExprPath(_, _) => if follow { fetch_path(cx, e) } else { None }, + &ExprBlock(ref block) => constant_block(cx, inner, follow), + &ExprIf(ref cond, ref then, ref otherwise) => match constant(cx, cond) { - Some(ConstantBool(true)) => constant(cx, then), - Some(ConstantBool(false)) => constant(cx, otherwise), + Some(ConstantBool(true)) => constant(cx, then, follow), + Some(ConstantBool(false)) => constant(cx, otherwise, follow), _ => None, }, &ExprLit(ref lit) => Some(lit_to_constant(lit)), - &ExprVec(ref vec) => constant_vec(cx, vec), - &ExprTup(ref tup) => constant_tup(cx, tup), - &ExprUnary(op, ref operand) => constant(cx, operand).and_then( + &ExprVec(ref vec) => constant_vec(cx, vec, follow), + &ExprTup(ref tup) => constant_tup(cx, tup, follow), + &ExprRepeat(ref value, ref number) => + constant_binop_apply(cx, value, number,|v, n| ConstantRepeat(v, n)), + &ExprUnary(op, ref operand) => constant(cx, operand, follow).and_then( |o| match op { UnNot => if let ConstantBool(b) = o { Some(ConstantBool(!b)) } else { None }, - UnNeg => - match o { - &ConstantInt(value, ty) => - Some(ConstantInt(value, match ty { - SignedIntLit(ity, sign) => - SignedIntLit(ity, neg_sign(sign)), - UnsuffixedIntLit(sign) => - UnsuffixedIntLit(neg_sign(sign)), - _ => { return None; }, - })), - &LitFloat(ref is, ref ty) => - Some(ConstantFloat(neg_float_str(is), ty)), - &LitFloatUnsuffixed(ref is) => - Some(ConstantFloatUnsuffixed(neg_float_str(is))), - _ => None, - }, + UnNeg => constant_negate(o), UnUniq | UnDeref => o, }), + &ExprBinary(op, ref left, ref right) => + constant_binop(cx, op, left, right, follow), //TODO: add other expressions _ => None, } @@ -69,19 +83,19 @@ fn lit_to_constant(lit: &Lit_) -> Constant { &LitByte(b) => ConstantByte(b), &LitChar(c) => ConstantChar(c), &LitInt(value, ty) => ConstantInt(value, ty), - &LitFloat(ref is, ty) => ConstantFloat(Cow::Borrowed(&*is), ty), - &LitFloatUnsuffixed(InternedString) => - ConstantFloatUnsuffixed(Cow::Borrowed(&*is)), + &LitFloat(ref is, ty) => ConstantFloat(Cow::Borrowed(&*is), ty.into()), + &LitFloatUnsuffixed(InternedString) => + ConstantFloat(Cow::Borrowed(&*is), FwAny), &LitBool(b) => ConstantBool(b), } } /// create `Some(ConstantVec(..))` of all constants, unless there is any /// non-constant part -fn constant_vec(cx: &Context, vec: &[&Expr]) -> Option { - Vec parts = Vec::new(); +fn constant_vec(cx: &Context, vec: &[&Expr], follow: bool) -> Option { + parts = Vec::new(); for opt_part in vec { - match constant(cx, opt_part) { + match constant(cx, opt_part, follow) { Some(ref p) => parts.push(p), None => { return None; }, } @@ -89,10 +103,10 @@ fn constant_vec(cx: &Context, vec: &[&Expr]) -> Option { Some(ConstantVec(parts)) } -fn constant_tup(cx, &Context, tup: &[&Expr]) -> Option { - Vec parts = Vec::new(); +fn constant_tup(cx: &Context, tup: &[&Expr], follow: bool) -> Option { + parts = Vec::new(); for opt_part in vec { - match constant(cx, opt_part) { + match constant(cx, opt_part, follow) { Some(ref p) => parts.push(p), None => { return None; }, } @@ -109,23 +123,81 @@ fn fetch_path(cx: &Context, e: &Expr) -> Option { } /// A block can only yield a constant if it only has one constant expression -fn constant_block(cx: &Context, block: &Block) -> Option { +fn constant_block(cx: &Context, block: &Block, follow: bool) -> Option { if block.stmts.is_empty() { - block.expr.map(|b| constant(cx, b)) + block.expr.map(|b| constant(cx, b, follow)) } else { None } } +fn constant_negate(o: Constant) -> Option { + match o { + &ConstantInt(value, ty) => + Some(ConstantInt(value, match ty { + SignedIntLit(ity, sign) => SignedIntLit(ity, neg_sign(sign)), + UnsuffixedIntLit(sign) => UnsuffixedIntLit(neg_sign(sign)), + _ => { return None; }, + })), + &LitFloat(ref is, ref ty) => Some(ConstantFloat(neg_float_str(is), ty)), + _ => None, + } +} + fn neg_sign(s: Sign) -> Sign { - match s: + match s { Sign::Plus => Sign::Minus, Sign::Minus => Sign::Plus, } } fn neg_float_str(s: &InternedString) -> Cow<'static, str> { - if s.startsWith('-') { + if s.startsWith('-') { Cow::Borrowed(s[1..]) } else { Cow::Owned(format!("-{}", &*s)) } } + +fn constant_binop(cx: &Context, op: BinOp, left: &Expr, right: &Expr, + follow: bool) -> Option { + match op.node { + //BiAdd, + //BiSub, + //BiMul, + //BiDiv, + //BiRem, + BiAnd => constant_short_circuit(cx, left, right, false, follow), + BiOr => constant_short_circuit(cx, left, right, true, follow), + //BiBitXor, + //BiBitAnd, + //BiBitOr, + //BiShl, + //BiShr, + //BiEq, + //BiLt, + //BiLe, + //BiNe, + //BiGe, + //BiGt, + _ => None, + } +} + +fn constant_binop_apply(cx: &Context, left: &Expr, right: &Expr, op: F, + follow: bool) -> Option +where F: FnMut(Constant, Constant) -> Option { + constant(cx, left, follow).and_then(|l| constant(cx, right, follow) + .and_then(|r| op(l, r))) +} + +fn constant_short_circuit(cx: &Context, left: &Expr, right: &Expr, b: bool, + follow: bool) -> Option { + if let ConstantBool(lbool) = constant(cx, left, follow) { + if l == b { + Some(ConstantBool(b)) + } else { + if let ConstantBool(rbool) = constant(cx, right, follow) { + Some(ConstantBool(rbool)) + } else { None } + } + } else { None } +} From 1a19d5ef65061338b90336626999eef3497ecbb7 Mon Sep 17 00:00:00 2001 From: llogiq Date: Thu, 13 Aug 2015 10:45:30 +0200 Subject: [PATCH 09/16] changed Constant to a struct with 'needed_resolution' bool --- src/const.rs | 202 +++++++++++++++++++++++++++++++++++---------------- 1 file changed, 138 insertions(+), 64 deletions(-) diff --git a/src/const.rs b/src/const.rs index 38044bbea5cc..cef2cd7f9fc0 100644 --- a/src/const.rs +++ b/src/const.rs @@ -18,9 +18,25 @@ impl From for FloatWidth { } } +#[derive(PartialEq, Eq, Debug, Clone)] +pub struct Constant { + constant: ConstantVariant, + needed_resolution: bool +} + +impl Constant { + fn new(variant: ConstantVariant) -> Constant { + Constant { constant: variant, needed_resolution: false } + } + + fn new_resolved(variant: ConstantVariant) -> Constant { + Constant { constant: variant, needed_resolution: true } + } +} + /// a Lit_-like enum to fold constant `Expr`s into #[derive(PartialEq, Eq, Debug, Clone)] -pub enum Constant { +pub enum ConstantVariant { /// a String "abc" ConstantStr(&'static str, StrStyle), /// a Binary String b"abc" @@ -43,34 +59,51 @@ pub enum Constant { ConstantTuple(Vec), } -/// simple constant folding -pub fn constant(cx: &Context, e: &Expr, follow: bool) -> Option { +impl ConstantVariant { + /// convert to u64 if possible + /// + /// # panics + /// + /// if the constant could not be converted to u64 losslessly + fn as_u64(&self) -> u64 { + if let &ConstantInt(val, _) = self { + val // TODO we may want to check the sign if any + } else { + panic!("Could not convert a {:?} to u64"); + } + } +} + +/// simple constant folding: Insert an expression, get a constant or none. +pub fn constant(cx: &Context, e: &Expr) -> Option { match e { - &ExprParen(ref inner) => constant(cx, inner, follow), - &ExprPath(_, _) => if follow { fetch_path(cx, e) } else { None }, - &ExprBlock(ref block) => constant_block(cx, inner, follow), + &ExprParen(ref inner) => constant(cx, inner), + &ExprPath(_, _) => fetch_path(cx, e), + &ExprBlock(ref block) => constant_block(cx, inner), &ExprIf(ref cond, ref then, ref otherwise) => - match constant(cx, cond) { - Some(ConstantBool(true)) => constant(cx, then, follow), - Some(ConstantBool(false)) => constant(cx, otherwise, follow), - _ => None, - }, + constant_if(cx, cond, then, otherwise), &ExprLit(ref lit) => Some(lit_to_constant(lit)), - &ExprVec(ref vec) => constant_vec(cx, vec, follow), - &ExprTup(ref tup) => constant_tup(cx, tup, follow), + &ExprVec(ref vec) => constant_vec(cx, vec), + &ExprTup(ref tup) => constant_tup(cx, tup), &ExprRepeat(ref value, ref number) => - constant_binop_apply(cx, value, number,|v, n| ConstantRepeat(v, n)), - &ExprUnary(op, ref operand) => constant(cx, operand, follow).and_then( + constant_binop_apply(cx, value, number,|v, n| Constant { + constant: ConstantRepeat(v, n.constant.as_u64()), + needed_resolution: v.needed_resolution || n.needed_resolution + }), + &ExprUnary(op, ref operand) => constant(cx, operand).and_then( |o| match op { UnNot => - if let ConstantBool(b) = o { - Some(ConstantBool(!b)) + if let ConstantBool(b) = o.variant { + Some(Constant{ + needed_resolution: o.needed_resolution, + constant: ConstantBool(!b), + }) } else { None }, UnNeg => constant_negate(o), UnUniq | UnDeref => o, }), &ExprBinary(op, ref left, ref right) => - constant_binop(cx, op, left, right, follow), + constant_binop(op, left, right), //TODO: add other expressions _ => None, } @@ -78,68 +111,100 @@ pub fn constant(cx: &Context, e: &Expr, follow: bool) -> Option { fn lit_to_constant(lit: &Lit_) -> Constant { match lit { - &LitStr(ref is, style) => ConstantStr(&*is, style), - &LitBinary(ref blob) => ConstantBinary(blob.clone()), - &LitByte(b) => ConstantByte(b), - &LitChar(c) => ConstantChar(c), - &LitInt(value, ty) => ConstantInt(value, ty), - &LitFloat(ref is, ty) => ConstantFloat(Cow::Borrowed(&*is), ty.into()), + &LitStr(ref is, style) => Constant::new(ConstantStr(&*is, style)), + &LitBinary(ref blob) => Constant::new(ConstantBinary(blob.clone())), + &LitByte(b) => Constant::new(ConstantByte(b)), + &LitChar(c) => Constant::new(ConstantChar(c)), + &LitInt(value, ty) => Constant::new(ConstantInt(value, ty)), + &LitFloat(ref is, ty) => + Constant::new(ConstantFloat(Cow::Borrowed(&*is), ty.into())), &LitFloatUnsuffixed(InternedString) => - ConstantFloat(Cow::Borrowed(&*is), FwAny), - &LitBool(b) => ConstantBool(b), + Constant::new(ConstantFloat(Cow::Borrowed(&*is), FwAny)), + &LitBool(b) => Constant::new(ConstantBool(b)), } } /// create `Some(ConstantVec(..))` of all constants, unless there is any /// non-constant part -fn constant_vec(cx: &Context, vec: &[&Expr], follow: bool) -> Option { - parts = Vec::new(); +fn constant_vec(cx: &Context, vec: &[&Expr]) -> Option { + let mut parts = Vec::new(); + let mut resolved = false; for opt_part in vec { - match constant(cx, opt_part, follow) { - Some(ref p) => parts.push(p), + match constant(cx, opt_part) { + Some(ref p) => { + resolved |= p.needed_resolution; + parts.push(p) + }, None => { return None; }, } } - Some(ConstantVec(parts)) + Some(Constant { + constant: ConstantVec(parts), + needed_resolution: resolved + }) } -fn constant_tup(cx: &Context, tup: &[&Expr], follow: bool) -> Option { - parts = Vec::new(); +fn constant_tup(cx: &Context, tup: &[&Expr]) -> Option { + let mut parts = Vec::new(); + let mut resolved = false; for opt_part in vec { - match constant(cx, opt_part, follow) { - Some(ref p) => parts.push(p), + match constant(cx, opt_part) { + Some(ref p) => { + resolved |= p.needed_resolution; + parts.push(p) + }, None => { return None; }, } } - Some(ConstantTuple(parts)) + Some(Constant { + constant: ConstantTuple(parts), + needed_resolution: resolved + }) } /// lookup a possibly constant expression from a ExprPath fn fetch_path(cx: &Context, e: &Expr) -> Option { if let Some(&PathResolution { base_def: DefConst(id), ..}) = cx.tcx.def_map.borrow().get(&e.id) { - lookup_const_by_id(cx.tcx, id, None).map(|l| constant(cx, l)) + lookup_const_by_id(cx.tcx, id, None).map( + |l| Constant::new_resolved(constant(cx, l).constant)) } else { None } } /// A block can only yield a constant if it only has one constant expression -fn constant_block(cx: &Context, block: &Block, follow: bool) -> Option { +fn constant_block(cx: &Context, block: &Block) -> Option { if block.stmts.is_empty() { - block.expr.map(|b| constant(cx, b, follow)) + block.expr.map(|b| constant(cx, b)) + } else { None } +} + +fn constant_if(cx: &Context, cond: &Expr, then: &Expr, otherwise: &Expr) -> + Option { + if let Some(Constant{ constant: ConstantBool(b), needed_resolution: res }) = + constant(cx, cond) { + let part = constant(cx, if b { then } else { otherwise }); + Some(Constant { + constant: part.constant, + needed_resolution: res || part.needed_resolution, + }) } else { None } } fn constant_negate(o: Constant) -> Option { - match o { - &ConstantInt(value, ty) => - Some(ConstantInt(value, match ty { - SignedIntLit(ity, sign) => SignedIntLit(ity, neg_sign(sign)), - UnsuffixedIntLit(sign) => UnsuffixedIntLit(neg_sign(sign)), - _ => { return None; }, - })), - &LitFloat(ref is, ref ty) => Some(ConstantFloat(neg_float_str(is), ty)), - _ => None, - } + Some(Constant{ + needed_resolution: o.needed_resolution, + constant: match o.constant { + &ConstantInt(value, ty) => + ConstantInt(value, match ty { + SignedIntLit(ity, sign) => + SignedIntLit(ity, neg_sign(sign)), + UnsuffixedIntLit(sign) => UnsuffixedIntLit(neg_sign(sign)), + _ => { return None; }, + }), + &LitFloat(ref is, ref ty) => ConstantFloat(neg_float_str(is), ty), + _ => { return None; }, + } + }) } fn neg_sign(s: Sign) -> Sign { @@ -157,16 +222,16 @@ fn neg_float_str(s: &InternedString) -> Cow<'static, str> { } } -fn constant_binop(cx: &Context, op: BinOp, left: &Expr, right: &Expr, - follow: bool) -> Option { +fn constant_binop(cx: &Context, op: BinOp, left: &Expr, right: &Expr) + -> Option { match op.node { //BiAdd, //BiSub, //BiMul, //BiDiv, //BiRem, - BiAnd => constant_short_circuit(cx, left, right, false, follow), - BiOr => constant_short_circuit(cx, left, right, true, follow), + BiAnd => constant_short_circuit(cx, left, right, false), + BiOr => constant_short_circuit(cx, left, right, true), //BiBitXor, //BiBitAnd, //BiBitOr, @@ -182,21 +247,30 @@ fn constant_binop(cx: &Context, op: BinOp, left: &Expr, right: &Expr, } } -fn constant_binop_apply(cx: &Context, left: &Expr, right: &Expr, op: F, - follow: bool) -> Option -where F: FnMut(Constant, Constant) -> Option { - constant(cx, left, follow).and_then(|l| constant(cx, right, follow) - .and_then(|r| op(l, r))) +fn constant_binop_apply(cx: &Context, left: &Expr, right: &Expr, op: F) + -> Option +where F: FnMut(ConstantVariant, ConstantVariant) -> Option { + constant(cx, left).and_then(|l| constant(cx, right).and_then( + |r| Constant { + needed_resolution: l.needed_resolution || r.needed_resolution, + constant: op(l.constant, r.constant) + })) } -fn constant_short_circuit(cx: &Context, left: &Expr, right: &Expr, b: bool, - follow: bool) -> Option { - if let ConstantBool(lbool) = constant(cx, left, follow) { +fn constant_short_circuit(cx: &Context, left: &Expr, right: &Expr, b: bool) -> + Option { + let leftconst = constant(cx, left); + if let ConstantBool(lbool) = leftconst.constant { if l == b { - Some(ConstantBool(b)) + Some(leftconst) } else { - if let ConstantBool(rbool) = constant(cx, right, follow) { - Some(ConstantBool(rbool)) + let rightconst = constant(cx, right); + if let ConstantBool(rbool) = rightconst.constant { + Some(Constant { + constant: rightconst.constant, + needed_resolution: leftconst.needed_resolution || + rightconst.needed_resolution, + }) } else { None } } } else { None } From 6aa36e9deb9191ae8414b433d49c71ac37475cea Mon Sep 17 00:00:00 2001 From: llogiq Date: Thu, 13 Aug 2015 14:22:05 +0200 Subject: [PATCH 10/16] initial addition and subtraction for bytes and ints --- src/const.rs | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 81 insertions(+), 2 deletions(-) diff --git a/src/const.rs b/src/const.rs index cef2cd7f9fc0..bc53c1062dc0 100644 --- a/src/const.rs +++ b/src/const.rs @@ -222,11 +222,75 @@ fn neg_float_str(s: &InternedString) -> Cow<'static, str> { } } +fn is_negative(ty: LitIntType) -> bool { + match ty { + SignedIntLit(_, sign) | UnsuffixedIntLit(sign) => sign == Minus, + UnsignedIntLit(_) => false, + } +} + +fn unify_int_type(l: LitIntType, r: LitIntType, s: Sign) -> Option(LitIntType) { + match (l, r) { + (SignedIntLit(lty, _), SignedIntLit(rty, _)) => if lty == rty { + Some(SignedIntLit(lty, s)) } else { None }, + (UnsignedIntLit(lty), UnsignedIntLit(rty)) => + if Sign == Plus && lty == rty { + Some(UnsignedIntLit(lty)) + } else { None }, + (UnsuffixedIntLit(_), UnsuffixedIntLit(_)) => UnsuffixedIntLit(s), + (SignedIntLit(lty, _), UnsuffixedIntLit(_)) => SignedIntLit(lty, s), + (UnsignedIntLit(lty), UnsuffixedIntLit(rs)) => if rs == Plus { + Some(UnsignedIntLit(lty)) } else { None }, + (UnsuffixedIntLit(_), SignedIntLit(rty, _)) => SignedIntLit(rty, s), + (UnsuffixedIntLit(ls), UnsignedIntLit(rty)) => if ls == Plus { + Some(UnsignedIntLit(rty)) } else { None }, + _ => None, + } +} + fn constant_binop(cx: &Context, op: BinOp, left: &Expr, right: &Expr) -> Option { match op.node { - //BiAdd, - //BiSub, + BiAdd => constant_binop_apply(cx, left, right, |l, r| + match (l, r) { + (ConstantByte(l8), ConstantByte(r8)) => + l8.checked_add(r8).map(|v| ConstantByte(v)), + (ConstantInt(l64, lty), ConstantInt(r64, rty)) => { + let (ln, rn) = (is_negative(lty), is_negative(rty)); + if ln == rn { + unify_int_type(lty, rty, if ln { Minus } else { Plus }) + .and_then(|ty| l64.checked_add(r64).map( + |v| ConstantInt(v, ty))) + } else { + if ln { + add_neg_int(r64, rty, l64, lty) + } else { + add_neg_int(l64, lty, r64, rty) + } + } + }, + // TODO: float + _ => None + }), + BiSub => constant_binop_apply(cx, left, right, |l, r| + match (l, r) { + (ConstantByte(l8), ConstantByte(r8)) => if r8 > l8 { + None } else { Some(ConstantByte(l8 - r8)) }, + (ConstantInt(l64, lty), ConstantInt(r64, rty)) => { + let (ln, rn) = (is_negative(lty), is_negative(rty)); + match (ln, rn) { + (false, false) => sub_int(l64, lty, r64, rty, r64 > l64), + (true, true) => sub_int(l64, lty, r64, rty, l64 > r64), + (true, false) => unify_int_type(lty, rty, Minus) + .and_then(|ty| l64.checked_add(r64).map( + |v| ConstantInt(v, ty))), + (false, true) => unify_int_type(lty, rty, Plus) + .and_then(|ty| l64.checked_add(r64).map( + |v| ConstantInt(v, ty))), + } + }, + _ => None, + }), //BiMul, //BiDiv, //BiRem, @@ -247,6 +311,21 @@ fn constant_binop(cx: &Context, op: BinOp, left: &Expr, right: &Expr) } } +fn add_neg_int(pos: u64, pty: LitIntType, neg: u64, nty: LitIntType) -> + Some(Constant) { + if neg > pos { + unify_int_type(nty, pty, Minus).map(|ty| ConstantInt(neg - pos, ty)) + } else { + unify_int_type(nty, pty, Plus).map(|ty| ConstantInt(pos - neg, ty)) + } +} + +fn sub_int(l: u64, lty: LitIntType, r: u64, rty: LitIntType, neg: Bool) -> + Option { + unify_int_type(lty, rty, if neg { Minus } else { Plus }).and_then( + |ty| l64.checked_sub(r64).map(|v| ConstantInt(v, ty))) +} + fn constant_binop_apply(cx: &Context, left: &Expr, right: &Expr, op: F) -> Option where F: FnMut(ConstantVariant, ConstantVariant) -> Option { From f23af0cfd5cb25b1988d628fc41ad4515693c91d Mon Sep 17 00:00:00 2001 From: llogiq Date: Fri, 14 Aug 2015 17:14:54 +0200 Subject: [PATCH 11/16] changed const to consts to avoid keyword, added test, fixed a lot of bugs --- src/{const.rs => consts.rs} | 208 +++++++++++++++++++++--------------- src/lib.rs | 1 + tests/consts.rs | 11 ++ 3 files changed, 136 insertions(+), 84 deletions(-) rename src/{const.rs => consts.rs} (63%) create mode 100644 tests/consts.rs diff --git a/src/const.rs b/src/consts.rs similarity index 63% rename from src/const.rs rename to src/consts.rs index bc53c1062dc0..c8d5262abab7 100644 --- a/src/const.rs +++ b/src/consts.rs @@ -1,8 +1,21 @@ +#[cfg(test)] use rustc::lint::Context; -use rustc::middle::const_eval::lookup_const_by_id; -use syntax::ast::*; -use syntax::ptr::P; +use rustc::middle::const_eval::lookup_const_by_id; +use rustc::middle::def::PathResolution; +use rustc::middle::def::Def::*; +use syntax::ast::*; +use syntax::parse::token::InternedString; +use syntax::ptr::P; +use std::rc::Rc; +use std::ops::Deref; +use self::ConstantVariant::*; +use self::FloatWidth::*; + +#[cfg(not(test))] +pub struct Context; + +#[derive(PartialEq, Eq, Debug, Copy, Clone)] pub enum FloatWidth { Fw32, Fw64, @@ -25,20 +38,31 @@ pub struct Constant { } impl Constant { - fn new(variant: ConstantVariant) -> Constant { + pub fn new(variant: ConstantVariant) -> Constant { Constant { constant: variant, needed_resolution: false } } - fn new_resolved(variant: ConstantVariant) -> Constant { + pub fn new_resolved(variant: ConstantVariant) -> Constant { Constant { constant: variant, needed_resolution: true } } + + // convert this constant to a f64, if possible + pub fn as_float(&self) -> Option { + match &self.constant { + &ConstantByte(b) => Some(b as f64), + &ConstantFloat(ref s, _) => s.parse().ok(), + &ConstantInt(i, ty) => Some(if is_negative(ty) { + -(i as f64) } else { i as f64 }), + _ => None + } + } } /// a Lit_-like enum to fold constant `Expr`s into #[derive(PartialEq, Eq, Debug, Clone)] pub enum ConstantVariant { /// a String "abc" - ConstantStr(&'static str, StrStyle), + ConstantStr(String, StrStyle), /// a Binary String b"abc" ConstantBinary(Rc>), /// a single byte b'a' @@ -48,15 +72,15 @@ pub enum ConstantVariant { /// an integer ConstantInt(u64, LitIntType), /// a float with given type - ConstantFloat(Cow<'static, str>, FloatWidth), + ConstantFloat(String, FloatWidth), /// true or false ConstantBool(bool), /// an array of constants - ConstantVec(Vec), + ConstantVec(Box>), /// also an array, but with only one constant, repeated N times - ConstantRepeat(Constant, usize), + ConstantRepeat(Box, usize), /// a tuple of constants - ConstantTuple(Vec), + ConstantTuple(Box>), } impl ConstantVariant { @@ -76,34 +100,32 @@ impl ConstantVariant { /// simple constant folding: Insert an expression, get a constant or none. pub fn constant(cx: &Context, e: &Expr) -> Option { - match e { + match &e.node { &ExprParen(ref inner) => constant(cx, inner), &ExprPath(_, _) => fetch_path(cx, e), - &ExprBlock(ref block) => constant_block(cx, inner), + &ExprBlock(ref block) => constant_block(cx, block), &ExprIf(ref cond, ref then, ref otherwise) => - constant_if(cx, cond, then, otherwise), - &ExprLit(ref lit) => Some(lit_to_constant(lit)), - &ExprVec(ref vec) => constant_vec(cx, vec), - &ExprTup(ref tup) => constant_tup(cx, tup), + constant_if(cx, &*cond, &*then, &*otherwise), + &ExprLit(ref lit) => Some(lit_to_constant(&lit.node)), + &ExprVec(ref vec) => constant_vec(cx, &vec[..]), + &ExprTup(ref tup) => constant_tup(cx, &tup[..]), &ExprRepeat(ref value, ref number) => - constant_binop_apply(cx, value, number,|v, n| Constant { - constant: ConstantRepeat(v, n.constant.as_u64()), - needed_resolution: v.needed_resolution || n.needed_resolution - }), + constant_binop_apply(cx, value, number,|v, n| + Some(ConstantRepeat(Box::new(v), n.as_u64() as usize))), &ExprUnary(op, ref operand) => constant(cx, operand).and_then( |o| match op { UnNot => - if let ConstantBool(b) = o.variant { + if let ConstantBool(b) = o.constant { Some(Constant{ needed_resolution: o.needed_resolution, constant: ConstantBool(!b), }) } else { None }, UnNeg => constant_negate(o), - UnUniq | UnDeref => o, + UnUniq | UnDeref => Some(o), }), &ExprBinary(op, ref left, ref right) => - constant_binop(op, left, right), + constant_binop(cx, op, left, right), //TODO: add other expressions _ => None, } @@ -111,82 +133,93 @@ pub fn constant(cx: &Context, e: &Expr) -> Option { fn lit_to_constant(lit: &Lit_) -> Constant { match lit { - &LitStr(ref is, style) => Constant::new(ConstantStr(&*is, style)), + &LitStr(ref is, style) => + Constant::new(ConstantStr(is.to_string(), style)), &LitBinary(ref blob) => Constant::new(ConstantBinary(blob.clone())), &LitByte(b) => Constant::new(ConstantByte(b)), &LitChar(c) => Constant::new(ConstantChar(c)), &LitInt(value, ty) => Constant::new(ConstantInt(value, ty)), - &LitFloat(ref is, ty) => - Constant::new(ConstantFloat(Cow::Borrowed(&*is), ty.into())), - &LitFloatUnsuffixed(InternedString) => - Constant::new(ConstantFloat(Cow::Borrowed(&*is), FwAny)), + &LitFloat(ref is, ty) => { + Constant::new(ConstantFloat(is.to_string(), ty.into())) + }, + &LitFloatUnsuffixed(ref is) => { + Constant::new(ConstantFloat(is.to_string(), FwAny)) + }, &LitBool(b) => Constant::new(ConstantBool(b)), } } /// create `Some(ConstantVec(..))` of all constants, unless there is any /// non-constant part -fn constant_vec(cx: &Context, vec: &[&Expr]) -> Option { +fn constant_vec + Sized>(cx: &Context, vec: &[E]) -> Option { let mut parts = Vec::new(); let mut resolved = false; - for opt_part in vec { + for opt_part in vec.iter() { match constant(cx, opt_part) { - Some(ref p) => { - resolved |= p.needed_resolution; + Some(p) => { + resolved |= (&p).needed_resolution; parts.push(p) }, None => { return None; }, } } Some(Constant { - constant: ConstantVec(parts), + constant: ConstantVec(Box::new(parts)), needed_resolution: resolved }) } -fn constant_tup(cx: &Context, tup: &[&Expr]) -> Option { +fn constant_tup + Sized>(cx: &Context, tup: &[E]) -> Option { let mut parts = Vec::new(); let mut resolved = false; - for opt_part in vec { + for opt_part in tup.iter() { match constant(cx, opt_part) { - Some(ref p) => { - resolved |= p.needed_resolution; + Some(p) => { + resolved |= (&p).needed_resolution; parts.push(p) }, None => { return None; }, } } Some(Constant { - constant: ConstantTuple(parts), + constant: ConstantTuple(Box::new(parts)), needed_resolution: resolved }) } +#[cfg(test)] +fn fetch_path(_cx: &Context, _expr: &Expr) -> Option { None } + /// lookup a possibly constant expression from a ExprPath +#[cfg(not(test))] fn fetch_path(cx: &Context, e: &Expr) -> Option { if let Some(&PathResolution { base_def: DefConst(id), ..}) = cx.tcx.def_map.borrow().get(&e.id) { - lookup_const_by_id(cx.tcx, id, None).map( - |l| Constant::new_resolved(constant(cx, l).constant)) + lookup_const_by_id(cx.tcx, id, None).and_then( + |l| constant(cx, l).map(|c| Constant::new_resolved(c.constant))) } else { None } } /// A block can only yield a constant if it only has one constant expression fn constant_block(cx: &Context, block: &Block) -> Option { if block.stmts.is_empty() { - block.expr.map(|b| constant(cx, b)) + block.expr.as_ref().and_then(|b| constant(cx, &*b)) } else { None } } -fn constant_if(cx: &Context, cond: &Expr, then: &Expr, otherwise: &Expr) -> - Option { +fn constant_if(cx: &Context, cond: &Expr, then: &Block, otherwise: + &Option>) -> Option { if let Some(Constant{ constant: ConstantBool(b), needed_resolution: res }) = constant(cx, cond) { - let part = constant(cx, if b { then } else { otherwise }); - Some(Constant { - constant: part.constant, - needed_resolution: res || part.needed_resolution, - }) + if b { + constant_block(cx, then) + } else { + otherwise.as_ref().and_then(|expr| constant(cx, &*expr)) + }.map(|part| + Constant { + constant: part.constant, + needed_resolution: res || part.needed_resolution, + }) } else { None } } @@ -194,14 +227,15 @@ fn constant_negate(o: Constant) -> Option { Some(Constant{ needed_resolution: o.needed_resolution, constant: match o.constant { - &ConstantInt(value, ty) => + ConstantInt(value, ty) => ConstantInt(value, match ty { SignedIntLit(ity, sign) => SignedIntLit(ity, neg_sign(sign)), UnsuffixedIntLit(sign) => UnsuffixedIntLit(neg_sign(sign)), _ => { return None; }, }), - &LitFloat(ref is, ref ty) => ConstantFloat(neg_float_str(is), ty), + ConstantFloat(ref is, ty) => + ConstantFloat(neg_float_str(is.to_string()), ty), _ => { return None; }, } }) @@ -214,11 +248,11 @@ fn neg_sign(s: Sign) -> Sign { } } -fn neg_float_str(s: &InternedString) -> Cow<'static, str> { - if s.startsWith('-') { - Cow::Borrowed(s[1..]) +fn neg_float_str(s: String) -> String { + if s.starts_with('-') { + s[1..].to_owned() } else { - Cow::Owned(format!("-{}", &*s)) + format!("-{}", &*s) } } @@ -229,19 +263,19 @@ fn is_negative(ty: LitIntType) -> bool { } } -fn unify_int_type(l: LitIntType, r: LitIntType, s: Sign) -> Option(LitIntType) { +fn unify_int_type(l: LitIntType, r: LitIntType, s: Sign) -> Option { match (l, r) { (SignedIntLit(lty, _), SignedIntLit(rty, _)) => if lty == rty { Some(SignedIntLit(lty, s)) } else { None }, (UnsignedIntLit(lty), UnsignedIntLit(rty)) => - if Sign == Plus && lty == rty { + if s == Plus && lty == rty { Some(UnsignedIntLit(lty)) } else { None }, - (UnsuffixedIntLit(_), UnsuffixedIntLit(_)) => UnsuffixedIntLit(s), - (SignedIntLit(lty, _), UnsuffixedIntLit(_)) => SignedIntLit(lty, s), + (UnsuffixedIntLit(_), UnsuffixedIntLit(_)) => Some(UnsuffixedIntLit(s)), + (SignedIntLit(lty, _), UnsuffixedIntLit(_)) => Some(SignedIntLit(lty, s)), (UnsignedIntLit(lty), UnsuffixedIntLit(rs)) => if rs == Plus { Some(UnsignedIntLit(lty)) } else { None }, - (UnsuffixedIntLit(_), SignedIntLit(rty, _)) => SignedIntLit(rty, s), + (UnsuffixedIntLit(_), SignedIntLit(rty, _)) => Some(SignedIntLit(rty, s)), (UnsuffixedIntLit(ls), UnsignedIntLit(rty)) => if ls == Plus { Some(UnsignedIntLit(rty)) } else { None }, _ => None, @@ -312,7 +346,7 @@ fn constant_binop(cx: &Context, op: BinOp, left: &Expr, right: &Expr) } fn add_neg_int(pos: u64, pty: LitIntType, neg: u64, nty: LitIntType) -> - Some(Constant) { + Option { if neg > pos { unify_int_type(nty, pty, Minus).map(|ty| ConstantInt(neg - pos, ty)) } else { @@ -320,37 +354,43 @@ fn add_neg_int(pos: u64, pty: LitIntType, neg: u64, nty: LitIntType) -> } } -fn sub_int(l: u64, lty: LitIntType, r: u64, rty: LitIntType, neg: Bool) -> - Option { +fn sub_int(l: u64, lty: LitIntType, r: u64, rty: LitIntType, neg: bool) -> + Option { unify_int_type(lty, rty, if neg { Minus } else { Plus }).and_then( - |ty| l64.checked_sub(r64).map(|v| ConstantInt(v, ty))) + |ty| l.checked_sub(r).map(|v| ConstantInt(v, ty))) } fn constant_binop_apply(cx: &Context, left: &Expr, right: &Expr, op: F) -> Option -where F: FnMut(ConstantVariant, ConstantVariant) -> Option { - constant(cx, left).and_then(|l| constant(cx, right).and_then( - |r| Constant { - needed_resolution: l.needed_resolution || r.needed_resolution, - constant: op(l.constant, r.constant) - })) +where F: Fn(ConstantVariant, ConstantVariant) -> Option { + if let (Some(Constant { constant: lc, needed_resolution: ln }), + Some(Constant { constant: rc, needed_resolution: rn })) = + (constant(cx, left), constant(cx, right)) { + op(lc, rc).map(|c| + Constant { + needed_resolution: ln || rn, + constant: c, + }) + } else { None } } fn constant_short_circuit(cx: &Context, left: &Expr, right: &Expr, b: bool) -> Option { - let leftconst = constant(cx, left); - if let ConstantBool(lbool) = leftconst.constant { - if l == b { - Some(leftconst) - } else { - let rightconst = constant(cx, right); - if let ConstantBool(rbool) = rightconst.constant { - Some(Constant { - constant: rightconst.constant, - needed_resolution: leftconst.needed_resolution || - rightconst.needed_resolution, - }) - } else { None } - } - } else { None } + constant(cx, left).and_then(|left| + if let &ConstantBool(lbool) = &left.constant { + if lbool == b { + Some(left) + } else { + constant(cx, right).and_then(|right| + if let ConstantBool(_) = right.constant { + Some(Constant { + constant: right.constant, + needed_resolution: left.needed_resolution || + right.needed_resolution, + }) + } else { None } + ) + } + } else { None } + ) } diff --git a/src/lib.rs b/src/lib.rs index f788c72db3c6..8e7cc096c5ef 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -15,6 +15,7 @@ use rustc::lint::LintPassObject; #[macro_use] pub mod utils; +pub mod consts; pub mod types; pub mod misc; pub mod eq_op; diff --git a/tests/consts.rs b/tests/consts.rs new file mode 100644 index 000000000000..edbbfa1e2dbc --- /dev/null +++ b/tests/consts.rs @@ -0,0 +1,11 @@ + +extern crate clippy; + +use clippy::consts; +use syntax::ast::*; + +#[test] +fn test_lit() { + assert_eq!(ConstantBool(true), constant(&Context, + Expr{ node_id: 1, node: ExprLit(LitBool(true)), span: default() })); +} From 03c7d7074d4870588769707a1a8775489d29280a Mon Sep 17 00:00:00 2001 From: llogiq Date: Sun, 16 Aug 2015 15:56:09 +0200 Subject: [PATCH 12/16] With working test now --- src/consts.rs | 13 ++----------- tests/consts.rs | 29 ++++++++++++++++++++++++++--- 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/src/consts.rs b/src/consts.rs index c8d5262abab7..fd6479df299b 100644 --- a/src/consts.rs +++ b/src/consts.rs @@ -1,6 +1,4 @@ -#[cfg(test)] use rustc::lint::Context; - use rustc::middle::const_eval::lookup_const_by_id; use rustc::middle::def::PathResolution; use rustc::middle::def::Def::*; @@ -12,9 +10,6 @@ use std::ops::Deref; use self::ConstantVariant::*; use self::FloatWidth::*; -#[cfg(not(test))] -pub struct Context; - #[derive(PartialEq, Eq, Debug, Copy, Clone)] pub enum FloatWidth { Fw32, @@ -33,8 +28,8 @@ impl From for FloatWidth { #[derive(PartialEq, Eq, Debug, Clone)] pub struct Constant { - constant: ConstantVariant, - needed_resolution: bool + pub constant: ConstantVariant, + pub needed_resolution: bool } impl Constant { @@ -187,11 +182,7 @@ fn constant_tup + Sized>(cx: &Context, tup: &[E]) -> Optio }) } -#[cfg(test)] -fn fetch_path(_cx: &Context, _expr: &Expr) -> Option { None } - /// lookup a possibly constant expression from a ExprPath -#[cfg(not(test))] fn fetch_path(cx: &Context, e: &Expr) -> Option { if let Some(&PathResolution { base_def: DefConst(id), ..}) = cx.tcx.def_map.borrow().get(&e.id) { diff --git a/tests/consts.rs b/tests/consts.rs index edbbfa1e2dbc..db309952be43 100644 --- a/tests/consts.rs +++ b/tests/consts.rs @@ -1,11 +1,34 @@ +#![allow(plugin_as_library)] +#![feature(rustc_private)] extern crate clippy; +extern crate syntax; +extern crate rustc; -use clippy::consts; +use clippy::consts::constant; +use clippy::consts::ConstantVariant::*; use syntax::ast::*; +use syntax::ptr::P; +use syntax::codemap::{Spanned, COMMAND_LINE_SP}; +use std::mem; +use rustc::lint::Context; + +fn ctx() -> &'static Context<'static, 'static> { + unsafe { + let x : *const Context<'static, 'static> = std::ptr::null(); + mem::transmute(x) + } +} #[test] fn test_lit() { - assert_eq!(ConstantBool(true), constant(&Context, - Expr{ node_id: 1, node: ExprLit(LitBool(true)), span: default() })); + assert_eq!(Some(ConstantBool(true)), constant(ctx(), + &Expr{ + id: 1, + node: ExprLit(P(Spanned{ + node: LitBool(true), + span: COMMAND_LINE_SP, + })), + span: COMMAND_LINE_SP, + }).map(|x| x.constant)); } From fe0de07b28e2e9c1adcb126eea15ff4de14efb5b Mon Sep 17 00:00:00 2001 From: llogiq Date: Sun, 16 Aug 2015 16:05:51 +0200 Subject: [PATCH 13/16] dogfooded --- src/consts.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/consts.rs b/src/consts.rs index fd6479df299b..b636fe1e2a84 100644 --- a/src/consts.rs +++ b/src/consts.rs @@ -3,7 +3,6 @@ use rustc::middle::const_eval::lookup_const_by_id; use rustc::middle::def::PathResolution; use rustc::middle::def::Def::*; use syntax::ast::*; -use syntax::parse::token::InternedString; use syntax::ptr::P; use std::rc::Rc; use std::ops::Deref; @@ -71,11 +70,11 @@ pub enum ConstantVariant { /// true or false ConstantBool(bool), /// an array of constants - ConstantVec(Box>), + ConstantVec(Vec), /// also an array, but with only one constant, repeated N times ConstantRepeat(Box, usize), /// a tuple of constants - ConstantTuple(Box>), + ConstantTuple(Vec), } impl ConstantVariant { @@ -159,7 +158,7 @@ fn constant_vec + Sized>(cx: &Context, vec: &[E]) -> Optio } } Some(Constant { - constant: ConstantVec(Box::new(parts)), + constant: ConstantVec(parts), needed_resolution: resolved }) } @@ -177,7 +176,7 @@ fn constant_tup + Sized>(cx: &Context, tup: &[E]) -> Optio } } Some(Constant { - constant: ConstantTuple(Box::new(parts)), + constant: ConstantTuple(parts), needed_resolution: resolved }) } From 4244f2479f5959118fa90d99fc098f3c3fc897c3 Mon Sep 17 00:00:00 2001 From: llogiq Date: Sun, 16 Aug 2015 16:09:00 +0200 Subject: [PATCH 14/16] dogfooding, part 2 --- src/consts.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/consts.rs b/src/consts.rs index b636fe1e2a84..95232cd8b73e 100644 --- a/src/consts.rs +++ b/src/consts.rs @@ -148,7 +148,7 @@ fn lit_to_constant(lit: &Lit_) -> Constant { fn constant_vec + Sized>(cx: &Context, vec: &[E]) -> Option { let mut parts = Vec::new(); let mut resolved = false; - for opt_part in vec.iter() { + for opt_part in vec { match constant(cx, opt_part) { Some(p) => { resolved |= (&p).needed_resolution; @@ -166,7 +166,7 @@ fn constant_vec + Sized>(cx: &Context, vec: &[E]) -> Optio fn constant_tup + Sized>(cx: &Context, tup: &[E]) -> Option { let mut parts = Vec::new(); let mut resolved = false; - for opt_part in tup.iter() { + for opt_part in tup { match constant(cx, opt_part) { Some(p) => { resolved |= (&p).needed_resolution; @@ -224,8 +224,8 @@ fn constant_negate(o: Constant) -> Option { UnsuffixedIntLit(sign) => UnsuffixedIntLit(neg_sign(sign)), _ => { return None; }, }), - ConstantFloat(ref is, ty) => - ConstantFloat(neg_float_str(is.to_string()), ty), + ConstantFloat(is, ty) => + ConstantFloat(neg_float_str(is), ty), _ => { return None; }, } }) @@ -278,7 +278,7 @@ fn constant_binop(cx: &Context, op: BinOp, left: &Expr, right: &Expr) BiAdd => constant_binop_apply(cx, left, right, |l, r| match (l, r) { (ConstantByte(l8), ConstantByte(r8)) => - l8.checked_add(r8).map(|v| ConstantByte(v)), + l8.checked_add(r8).map(ConstantByte), (ConstantInt(l64, lty), ConstantInt(r64, rty)) => { let (ln, rn) = (is_negative(lty), is_negative(rty)); if ln == rn { From e1438e701069a6dbb6e71e1ea90f8e45aba865c4 Mon Sep 17 00:00:00 2001 From: llogiq Date: Sun, 16 Aug 2015 16:13:44 +0200 Subject: [PATCH 15/16] copied over cmp_owned fix from master --- tests/compile-fail/cmp_owned.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/tests/compile-fail/cmp_owned.rs b/tests/compile-fail/cmp_owned.rs index 5951dc1bbd74..2e1a8cfd819a 100755 --- a/tests/compile-fail/cmp_owned.rs +++ b/tests/compile-fail/cmp_owned.rs @@ -1,4 +1,4 @@ -#![feature(plugin, collections)] +#![feature(plugin)] #![plugin(clippy)] #[deny(cmp_owned)] @@ -13,11 +13,5 @@ fn main() { x != "foo".to_owned(); //~ERROR this creates an owned instance - #[allow(deprecated)] // for from_str - fn old_timey(x : &str) { - x != String::from_str("foo"); //~ERROR this creates an owned instance - } - old_timey(x); - x != String::from("foo"); //~ERROR this creates an owned instance } From 759b45a46d299f19a10477cc226e70002ac91340 Mon Sep 17 00:00:00 2001 From: llogiq Date: Sun, 16 Aug 2015 23:09:56 +0200 Subject: [PATCH 16/16] made is_negative(..) public (+doctest), fixed identity_op and precedence --- src/consts.rs | 9 +++++- src/identity_op.rs | 46 +++++++++++-------------------- tests/compile-fail/cmp_owned.rs | 3 ++ tests/compile-fail/identity_op.rs | 8 +++--- tests/compile-fail/precedence.rs | 1 + 5 files changed, 32 insertions(+), 35 deletions(-) diff --git a/src/consts.rs b/src/consts.rs index 95232cd8b73e..bb74b3c71d10 100644 --- a/src/consts.rs +++ b/src/consts.rs @@ -246,7 +246,14 @@ fn neg_float_str(s: String) -> String { } } -fn is_negative(ty: LitIntType) -> bool { +/// is the given LitIntType negative? +/// +/// Examples +/// +/// ``` +/// assert!(is_negative(UnsuffixedIntLit(Minus))); +/// ``` +pub fn is_negative(ty: LitIntType) -> bool { match ty { SignedIntLit(_, sign) | UnsuffixedIntLit(sign) => sign == Minus, UnsignedIntLit(_) => false, diff --git a/src/identity_op.rs b/src/identity_op.rs index 8c6940e3df49..445bef0125bc 100644 --- a/src/identity_op.rs +++ b/src/identity_op.rs @@ -7,6 +7,8 @@ use syntax::ast_util::{is_comparison_binop, binop_to_string}; use syntax::ptr::P; use syntax::codemap::Span; +use consts::{constant, Constant, is_negative}; +use consts::ConstantVariant::ConstantInt; use utils::{span_lint, snippet}; declare_lint! { pub IDENTITY_OP, Warn, @@ -47,35 +49,19 @@ impl LintPass for IdentityOp { fn check(cx: &Context, e: &Expr, m: i8, span: Span, arg: Span) { - if have_lit(cx, e, m) { - span_lint(cx, IDENTITY_OP, span, &format!( - "the operation is ineffective. Consider reducing it to `{}`", - snippet(cx, arg, ".."))); - } -} - -fn have_lit(cx: &Context, e : &Expr, m: i8) -> bool { - match &e.node { - &ExprUnary(UnNeg, ref litexp) => have_lit(cx, litexp, -m), - &ExprLit(ref lit) => { - match (&lit.node, m) { - (&LitInt(0, _), 0) => true, - (&LitInt(1, SignedIntLit(_, Plus)), 1) => true, - (&LitInt(1, UnsuffixedIntLit(Plus)), 1) => true, - (&LitInt(1, SignedIntLit(_, Minus)), -1) => true, - (&LitInt(1, UnsuffixedIntLit(Minus)), -1) => true, - _ => false - } - }, - &ExprParen(ref p) => have_lit(cx, p, m), - &ExprPath(_, _) => { - match cx.tcx.def_map.borrow().get(&e.id) { - Some(&PathResolution { base_def: DefConst(id), ..}) => - lookup_const_by_id(cx.tcx, id, Option::None) - .map_or(false, |l| have_lit(cx, l, m)), - _ => false - } - }, - _ => false + if let Some(c) = constant(cx, e) { + if c.needed_resolution { return; } // skip linting w/ lookup for now + if let ConstantInt(v, ty) = c.constant { + if match m { + 0 => v == 0, + -1 => is_negative(ty), + 1 => !is_negative(ty), + _ => unreachable!(), + } { + span_lint(cx, IDENTITY_OP, span, &format!( + "the operation is ineffective. Consider reducing it to `{}`", + snippet(cx, arg, ".."))); + } + } } } diff --git a/tests/compile-fail/cmp_owned.rs b/tests/compile-fail/cmp_owned.rs index 2e1a8cfd819a..2765da5cf23b 100755 --- a/tests/compile-fail/cmp_owned.rs +++ b/tests/compile-fail/cmp_owned.rs @@ -13,5 +13,8 @@ fn main() { x != "foo".to_owned(); //~ERROR this creates an owned instance + // removed String::from_str(..), as it has finally been removed in 1.4.0 + // as of 2015-08-14 + x != String::from("foo"); //~ERROR this creates an owned instance } diff --git a/tests/compile-fail/identity_op.rs b/tests/compile-fail/identity_op.rs index 987bada2eceb..18e683e8a9e8 100755 --- a/tests/compile-fail/identity_op.rs +++ b/tests/compile-fail/identity_op.rs @@ -11,14 +11,14 @@ fn main() { x + 0; //~ERROR the operation is ineffective 0 + x; //~ERROR the operation is ineffective - x - ZERO; //~ERROR the operation is ineffective + x - ZERO; //no error, as we skip lookups (for now) x | (0); //~ERROR the operation is ineffective - ((ZERO)) | x; //~ERROR the operation is ineffective + ((ZERO)) | x; //no error, as we skip lookups (for now) x * 1; //~ERROR the operation is ineffective 1 * x; //~ERROR the operation is ineffective - x / ONE; //~ERROR the operation is ineffective + x / ONE; //no error, as we skip lookups (for now) - x & NEG_ONE; //~ERROR the operation is ineffective + x & NEG_ONE; //no error, as we skip lookups (for now) -1 & x; //~ERROR the operation is ineffective } diff --git a/tests/compile-fail/precedence.rs b/tests/compile-fail/precedence.rs index 4d57f1194794..ebb25f61b758 100755 --- a/tests/compile-fail/precedence.rs +++ b/tests/compile-fail/precedence.rs @@ -2,6 +2,7 @@ #![plugin(clippy)] #[deny(precedence)] +#[allow(identity_op)] #[allow(eq_op)] fn main() { format!("{} vs. {}", 1 << 2 + 3, (1 << 2) + 3); //~ERROR operator precedence can trip