From a91c618fed0ffdc0651768d148985c1594dc9de5 Mon Sep 17 00:00:00 2001 From: Georg Brandl Date: Mon, 26 Oct 2015 07:43:38 +0100 Subject: [PATCH] Fix reverse_range_loop not taking sign into account (fixes #409) Adds a Display impl for Constant, because that might come in handy elsewhere as well. --- src/consts.rs | 64 ++++++++++++++++++++++++++++++++++ src/loops.rs | 6 ++-- tests/compile-fail/for_loop.rs | 4 +++ 3 files changed, 71 insertions(+), 3 deletions(-) mode change 100644 => 100755 src/consts.rs diff --git a/src/consts.rs b/src/consts.rs old mode 100644 new mode 100755 index be681efb257..766d998c256 --- a/src/consts.rs +++ b/src/consts.rs @@ -6,10 +6,12 @@ use rustc::middle::def::PathResolution; use rustc::middle::def::Def::*; use rustc_front::hir::*; use syntax::ptr::P; +use std::char; use std::cmp::PartialOrd; use std::cmp::Ordering::{self, Greater, Less, Equal}; use std::rc::Rc; use std::ops::Deref; +use std::fmt; use self::Constant::*; use self::FloatWidth::*; @@ -21,6 +23,7 @@ use syntax::ast::{UintTy, FloatTy, StrStyle}; use syntax::ast::UintTy::*; use syntax::ast::FloatTy::*; use syntax::ast::Sign::{self, Plus, Minus}; +use syntax::ast_util; #[derive(PartialEq, Eq, Debug, Copy, Clone)] @@ -159,6 +162,67 @@ impl PartialOrd for Constant { } } +fn format_byte(fmt: &mut fmt::Formatter, b: u8) -> fmt::Result { + if b == b'\\' { + write!(fmt, "\\\\") + } else if 0x20 <= b && b <= 0x7e { + write!(fmt, "{}", char::from_u32(b as u32).expect("all u8 are valid char")) + } else if b == 0x0a { + write!(fmt, "\\n") + } else if b == 0x0d { + write!(fmt, "\\r") + } else { + write!(fmt, "\\x{:02x}", b) + } +} + +impl fmt::Display for Constant { + fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { + match *self { + ConstantStr(ref s, _) => write!(fmt, "{:?}", s), + ConstantByte(ref b) => + write!(fmt, "b'").and_then(|_| format_byte(fmt, *b)) + .and_then(|_| write!(fmt, "'")), + ConstantBinary(ref bs) => { + try!(write!(fmt, "b\"")); + for b in bs.iter() { + try!(format_byte(fmt, *b)); + } + write!(fmt, "\"") + } + ConstantChar(ref c) => write!(fmt, "'{}'", c), + ConstantInt(ref i, ref ity) => { + let (sign, suffix) = match *ity { + LitIntType::SignedIntLit(ref sity, ref sign) => + (if let Sign::Minus = *sign { "-" } else { "" }, + ast_util::int_ty_to_string(*sity, None)), + LitIntType::UnsignedIntLit(ref uity) => + ("", ast_util::uint_ty_to_string(*uity, None)), + LitIntType::UnsuffixedIntLit(ref sign) => + (if let Sign::Minus = *sign { "-" } else { "" }, + "".into()), + }; + write!(fmt, "{}{}{}", sign, i, suffix) + } + ConstantFloat(ref s, ref fw) => { + let suffix = match *fw { + FloatWidth::Fw32 => "f32", + FloatWidth::Fw64 => "f64", + FloatWidth::FwAny => "", + }; + write!(fmt, "{}{}", s, suffix) + } + ConstantBool(ref b) => write!(fmt, "{}", b), + ConstantRepeat(ref c, ref n) => write!(fmt, "[{}; {}]", c, n), + ConstantVec(ref v) => write!(fmt, "[{}]", + v.iter().map(|i| format!("{}", i)) + .collect::>().join(", ")), + ConstantTuple(ref t) => write!(fmt, "({})", + t.iter().map(|i| format!("{}", i)) + .collect::>().join(", ")), + } + } +} fn lit_to_constant(lit: &Lit_) -> Constant { diff --git a/src/loops.rs b/src/loops.rs index 7d7c97f1bb1..95289cf347c 100644 --- a/src/loops.rs +++ b/src/loops.rs @@ -44,7 +44,7 @@ pub struct LoopsPass; impl LintPass for LoopsPass { fn get_lints(&self) -> LintArray { lint_array!(NEEDLESS_RANGE_LOOP, EXPLICIT_ITER_LOOP, ITER_NEXT_LOOP, - WHILE_LET_LOOP, UNUSED_COLLECT, REVERSE_RANGE_LOOP, + WHILE_LET_LOOP, UNUSED_COLLECT, REVERSE_RANGE_LOOP, EXPLICIT_COUNTER_LOOP, EMPTY_LOOP) } } @@ -88,8 +88,8 @@ impl LateLintPass for LoopsPass { // if this for loop is iterating over a two-sided range... if let ExprRange(Some(ref start_expr), Some(ref stop_expr)) = arg.node { // ...and both sides are compile-time constant integers... - if let Some(Constant::ConstantInt(start_idx, _)) = constant_simple(start_expr) { - if let Some(Constant::ConstantInt(stop_idx, _)) = constant_simple(stop_expr) { + if let Some(start_idx @ Constant::ConstantInt(..)) = constant_simple(start_expr) { + if let Some(stop_idx @ Constant::ConstantInt(..)) = constant_simple(stop_expr) { // ...and the start index is greater than the stop index, // this loop will never run. This is often confusing for developers // who think that this will iterate from the larger value to the diff --git a/tests/compile-fail/for_loop.rs b/tests/compile-fail/for_loop.rs index 11810242a88..02c8cc56083 100755 --- a/tests/compile-fail/for_loop.rs +++ b/tests/compile-fail/for_loop.rs @@ -46,6 +46,10 @@ fn main() { println!("{}", i); } + for i in -10..0 { // not an error + println!("{}", i); + } + for i in (10..0).rev() { // not an error, this is an established idiom for looping backwards on a range println!("{}", i); }