From 880bce1114d9cefcb2b9aad49c9d9d5e0d9ef753 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 11 Mar 2020 11:03:18 +0800 Subject: [PATCH] General cleanup. --- Cargo.toml | 1 + src/builtin.rs | 68 +++++++++++++------------------------- src/engine.rs | 7 ++-- src/fn_register.rs | 82 +++++++++++++++++++++++----------------------- src/optimize.rs | 8 ++++- src/parser.rs | 26 +++++++-------- src/result.rs | 43 ++++++++++++++++-------- src/scope.rs | 1 + tests/math.rs | 4 +++ 9 files changed, 124 insertions(+), 116 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 8fa20102..600705e7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,6 +18,7 @@ include = [ num-traits = "*" [features] +#default = ["no_index", "no_float", "only_i32", "no_stdlib", "unchecked"] default = [] debug_msgs = [] unchecked = [] diff --git a/src/builtin.rs b/src/builtin.rs index 0e8ab757..04a290ce 100644 --- a/src/builtin.rs +++ b/src/builtin.rs @@ -2,45 +2,23 @@ //! _standard library_ of utility functions. use crate::any::Any; -use crate::engine::Engine; -use crate::fn_register::RegisterFn; -use crate::parser::INT; - -#[cfg(not(feature = "unchecked"))] -use crate::{parser::Position, result::EvalAltResult, RegisterResultFn}; - #[cfg(not(feature = "no_index"))] use crate::engine::Array; - -#[cfg(not(feature = "no_float"))] +use crate::engine::Engine; +use crate::fn_register::{RegisterFn, RegisterResultFn}; +use crate::parser::{Position, INT}; +use crate::result::EvalAltResult; use crate::FLOAT; +use num_traits::{ + identities::Zero, CheckedAdd, CheckedDiv, CheckedMul, CheckedNeg, CheckedRem, CheckedShl, + CheckedShr, CheckedSub, +}; + use std::{ fmt::{Debug, Display}, - ops::{BitAnd, BitOr, BitXor, Range}, -}; - -#[cfg(feature = "unchecked")] -use std::ops::{Shl, Shr}; - -#[cfg(not(feature = "unchecked"))] -#[cfg(not(feature = "no_float"))] -use std::{i32, i64}; - -#[cfg(not(feature = "unchecked"))] -#[cfg(not(feature = "only_i32"))] -use std::u32; - -#[cfg(any(feature = "unchecked", not(feature = "no_float")))] -use std::ops::{Add, Div, Mul, Neg, Rem, Sub}; - -#[cfg(not(feature = "unchecked"))] -use { - num_traits::{ - CheckedAdd, CheckedDiv, CheckedMul, CheckedNeg, CheckedRem, CheckedShl, CheckedShr, - CheckedSub, - }, - std::convert::TryFrom, + ops::{Add, BitAnd, BitOr, BitXor, Div, Mul, Neg, Range, Rem, Shl, Shr, Sub}, + {i32, i64, u32}, }; macro_rules! reg_op { @@ -162,12 +140,9 @@ impl Engine<'_> { #[cfg(not(feature = "unchecked"))] fn div(x: T, y: T) -> Result where - T: Display + CheckedDiv + PartialEq + TryFrom, + T: Display + CheckedDiv + PartialEq + Zero, { - if y == >::try_from(0) - .map_err(|_| ()) - .expect("zero should always succeed") - { + if y == T::zero() { return Err(EvalAltResult::ErrorArithmetic( format!("Division by zero: {} / {}", x, y), Position::none(), @@ -191,8 +166,10 @@ impl Engine<'_> { }) } #[cfg(not(feature = "unchecked"))] - fn abs>(x: T) -> Result { - if x >= 0.into() { + fn abs(x: T) -> Result { + // FIX - We don't use Signed::abs() here because, contrary to documentation, it panics + // when the number is ::MIN instead of returning ::MIN itself. + if x >= ::zero() { Ok(x) } else { x.checked_neg().ok_or_else(|| { @@ -224,14 +201,15 @@ impl Engine<'_> { -x } #[cfg(any(feature = "unchecked", not(feature = "no_float")))] - fn abs_u>(x: T) -> T + fn abs_u(x: T) -> ::Output where - ::Output: Into, + T: Neg + PartialOrd + Default + Into<::Output>, { - if x < 0.into() { - (-x).into() + // Numbers should default to zero + if x < Default::default() { + -x } else { - x + x.into() } } fn lt(x: T, y: T) -> bool { diff --git a/src/engine.rs b/src/engine.rs index ebbf4ec7..cba45674 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -4,8 +4,6 @@ use crate::any::{Any, AnyExt, Dynamic, Variant}; use crate::parser::{Expr, FnDef, Position, Stmt}; use crate::result::EvalAltResult; use crate::scope::Scope; - -#[cfg(not(feature = "no_index"))] use crate::INT; use std::{ @@ -788,6 +786,9 @@ impl Engine<'_> { .eval_index_expr(scope, lhs, idx_expr, *idx_pos) .map(|(_, _, _, x)| x), + #[cfg(feature = "no_index")] + Expr::Index(_, _, _) => panic!("encountered an index expression during no_index!"), + // Statement block Expr::Stmt(stmt, _) => self.eval_stmt(scope, stmt), @@ -855,6 +856,8 @@ impl Engine<'_> { Ok(Box::new(arr)) } + #[cfg(feature = "no_index")] + Expr::Array(_, _) => panic!("encountered an array during no_index!"), Expr::FunctionCall(fn_name, args, def_val, pos) => { let mut args = args diff --git a/src/fn_register.rs b/src/fn_register.rs index 9af9ab82..84dbf150 100644 --- a/src/fn_register.rs +++ b/src/fn_register.rs @@ -120,20 +120,20 @@ macro_rules! def_register { const NUM_ARGS: usize = count_args!($($par)*); if args.len() != NUM_ARGS { - Err(EvalAltResult::ErrorFunctionArgsMismatch(fn_name.clone(), NUM_ARGS, args.len(), pos)) - } else { - #[allow(unused_variables, unused_mut)] - let mut drain = args.drain(..); - $( - // Downcast every element, return in case of a type mismatch - let $par = drain.next().unwrap().downcast_mut::<$par>().unwrap(); - )* - - // Call the user-supplied function using ($clone) to - // potentially clone the value, otherwise pass the reference. - let r = f($(($clone)($par)),*); - Ok(Box::new(r) as Dynamic) + return Err(EvalAltResult::ErrorFunctionArgsMismatch(fn_name.clone(), NUM_ARGS, args.len(), pos)); } + + #[allow(unused_variables, unused_mut)] + let mut drain = args.drain(..); + $( + // Downcast every element, return in case of a type mismatch + let $par = drain.next().unwrap().downcast_mut::<$par>().unwrap(); + )* + + // Call the user-supplied function using ($clone) to + // potentially clone the value, otherwise pass the reference. + let r = f($(($clone)($par)),*); + Ok(Box::new(r) as Dynamic) }; self.register_fn_raw(name, Some(vec![$(TypeId::of::<$par>()),*]), Box::new(fun)); } @@ -152,19 +152,19 @@ macro_rules! def_register { const NUM_ARGS: usize = count_args!($($par)*); if args.len() != NUM_ARGS { - Err(EvalAltResult::ErrorFunctionArgsMismatch(fn_name.clone(), NUM_ARGS, args.len(), pos)) - } else { - #[allow(unused_variables, unused_mut)] - let mut drain = args.drain(..); - $( - // Downcast every element, return in case of a type mismatch - let $par = drain.next().unwrap().downcast_mut::<$par>().unwrap(); - )* - - // Call the user-supplied function using ($clone) to - // potentially clone the value, otherwise pass the reference. - Ok(f($(($clone)($par)),*)) + return Err(EvalAltResult::ErrorFunctionArgsMismatch(fn_name.clone(), NUM_ARGS, args.len(), pos)); } + + #[allow(unused_variables, unused_mut)] + let mut drain = args.drain(..); + $( + // Downcast every element, return in case of a type mismatch + let $par = drain.next().unwrap().downcast_mut::<$par>().unwrap(); + )* + + // Call the user-supplied function using ($clone) to + // potentially clone the value, otherwise pass the reference. + Ok(f($(($clone)($par)),*)) }; self.register_fn_raw(name, Some(vec![$(TypeId::of::<$par>()),*]), Box::new(fun)); } @@ -184,23 +184,23 @@ macro_rules! def_register { const NUM_ARGS: usize = count_args!($($par)*); if args.len() != NUM_ARGS { - Err(EvalAltResult::ErrorFunctionArgsMismatch(fn_name.clone(), NUM_ARGS, args.len(), pos)) - } else { - #[allow(unused_variables, unused_mut)] - let mut drain = args.drain(..); - $( - // Downcast every element, return in case of a type mismatch - let $par = drain.next().unwrap().downcast_mut::<$par>().unwrap(); - )* + return Err(EvalAltResult::ErrorFunctionArgsMismatch(fn_name.clone(), NUM_ARGS, args.len(), pos)); + } - // Call the user-supplied function using ($clone) to - // potentially clone the value, otherwise pass the reference. - match f($(($clone)($par)),*) { - Ok(r) => Ok(Box::new(r) as Dynamic), - Err(mut err) => { - err.set_position(pos); - Err(err) - } + #[allow(unused_variables, unused_mut)] + let mut drain = args.drain(..); + $( + // Downcast every element, return in case of a type mismatch + let $par = drain.next().unwrap().downcast_mut::<$par>().unwrap(); + )* + + // Call the user-supplied function using ($clone) to + // potentially clone the value, otherwise pass the reference. + match f($(($clone)($par)),*) { + Ok(r) => Ok(Box::new(r) as Dynamic), + Err(mut err) => { + err.set_position(pos); + Err(err) } } }; diff --git a/src/optimize.rs b/src/optimize.rs index 085b09f9..cb92fb11 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -141,6 +141,7 @@ fn optimize_expr(expr: Expr, changed: &mut bool) -> Expr { Box::new(optimize_expr(*rhs, changed)), pos, ), + #[cfg(not(feature = "no_index"))] Expr::Index(lhs, rhs, pos) => match (*lhs, *rhs) { (Expr::Array(mut items, _), Expr::IntegerConstant(i, _)) @@ -158,6 +159,9 @@ fn optimize_expr(expr: Expr, changed: &mut bool) -> Expr { pos, ), }, + #[cfg(feature = "no_index")] + Expr::Index(_, _, _) => panic!("encountered an index expression during no_index!"), + #[cfg(not(feature = "no_index"))] Expr::Array(items, pos) => { let original_len = items.len(); @@ -172,6 +176,9 @@ fn optimize_expr(expr: Expr, changed: &mut bool) -> Expr { Expr::Array(items, pos) } + #[cfg(feature = "no_index")] + Expr::Array(_, _) => panic!("encountered an array during no_index!"), + Expr::And(lhs, rhs) => match (*lhs, *rhs) { (Expr::True(_), rhs) => { *changed = true; @@ -208,7 +215,6 @@ fn optimize_expr(expr: Expr, changed: &mut bool) -> Expr { Box::new(optimize_expr(rhs, changed)), ), }, - Expr::FunctionCall(id, args, def_value, pos) => { let original_len = args.len(); diff --git a/src/parser.rs b/src/parser.rs index fcbe7e20..cdb8bac9 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -3,13 +3,18 @@ use crate::any::Dynamic; use crate::error::{LexError, ParseError, ParseErrorType}; use crate::optimize::optimize; + use std::{borrow::Cow, char, fmt, iter::Peekable, str::Chars, str::FromStr, usize}; -/// The system integer type +/// The system integer type. +/// +/// If the `only_i32` feature is enabled, this will be `i32` instead. #[cfg(not(feature = "only_i32"))] pub type INT = i64; /// The system integer type +/// +/// If the `only_i32` feature is not enabled, this will be `i64` instead. #[cfg(feature = "only_i32")] pub type INT = i32; @@ -179,9 +184,7 @@ pub enum Expr { FunctionCall(String, Vec, Option, Position), Assignment(Box, Box, Position), Dot(Box, Box, Position), - #[cfg(not(feature = "no_index"))] Index(Box, Box, Position), - #[cfg(not(feature = "no_index"))] Array(Vec, Position), And(Box, Box), Or(Box, Box), @@ -197,24 +200,21 @@ impl Expr { | Expr::Identifier(_, pos) | Expr::CharConstant(_, pos) | Expr::StringConstant(_, pos) - | Expr::FunctionCall(_, _, _, pos) | Expr::Stmt(_, pos) + | Expr::FunctionCall(_, _, _, pos) + | Expr::Array(_, pos) | Expr::True(pos) | Expr::False(pos) | Expr::Unit(pos) => *pos, - Expr::Assignment(e, _, _) | Expr::Dot(e, _, _) | Expr::And(e, _) | Expr::Or(e, _) => { - e.position() - } + Expr::Assignment(e, _, _) + | Expr::Dot(e, _, _) + | Expr::Index(e, _, _) + | Expr::And(e, _) + | Expr::Or(e, _) => e.position(), #[cfg(not(feature = "no_float"))] Expr::FloatConstant(_, pos) => *pos, - - #[cfg(not(feature = "no_index"))] - Expr::Index(e, _, _) => e.position(), - - #[cfg(not(feature = "no_index"))] - Expr::Array(_, pos) => *pos, } } diff --git a/src/result.rs b/src/result.rs index 8034329c..e0bb3734 100644 --- a/src/result.rs +++ b/src/result.rs @@ -3,6 +3,7 @@ use crate::any::Dynamic; use crate::error::ParseError; use crate::parser::{Position, INT}; + use std::{error::Error, fmt}; /// Evaluation result. @@ -75,12 +76,12 @@ impl Error for EvalAltResult { Self::ErrorArrayBounds(_, index, _) if *index < 0 => { "Array access expects non-negative index" } - Self::ErrorArrayBounds(max, _, _) if *max == 0 => "Access of empty array", + Self::ErrorArrayBounds(0, _, _) => "Access of empty array", Self::ErrorArrayBounds(_, _, _) => "Array index out of bounds", Self::ErrorStringBounds(_, index, _) if *index < 0 => { "Indexing a string expects a non-negative index" } - Self::ErrorStringBounds(max, _, _) if *max == 0 => "Indexing of empty string", + Self::ErrorStringBounds(0, _, _) => "Indexing of empty string", Self::ErrorStringBounds(_, _, _) => "String index out of bounds", Self::ErrorIfGuard(_) => "If guard expects boolean expression", Self::ErrorFor(_) => "For loop expects array or range", @@ -128,6 +129,16 @@ impl fmt::Display for EvalAltResult { write!(f, "{} '{}': {}", desc, filename, err) } Self::ErrorParsing(p) => write!(f, "Syntax error: {}", p), + Self::ErrorFunctionArgsMismatch(fun, 0, n, pos) => write!( + f, + "Function '{}' expects no argument but {} found ({})", + fun, n, pos + ), + Self::ErrorFunctionArgsMismatch(fun, 1, n, pos) => write!( + f, + "Function '{}' expects one argument but {} found ({})", + fun, n, pos + ), Self::ErrorFunctionArgsMismatch(fun, need, n, pos) => write!( f, "Function '{}' expects {} argument(s) but {} found ({})", @@ -142,26 +153,30 @@ impl fmt::Display for EvalAltResult { Self::ErrorArrayBounds(_, index, pos) if *index < 0 => { write!(f, "{}: {} < 0 ({})", desc, index, pos) } - Self::ErrorArrayBounds(max, _, pos) if *max == 0 => write!(f, "{} ({})", desc, pos), + Self::ErrorArrayBounds(0, _, pos) => write!(f, "{} ({})", desc, pos), + Self::ErrorArrayBounds(1, index, pos) => write!( + f, + "Array index {} is out of bounds: only one element in the array ({})", + index, pos + ), Self::ErrorArrayBounds(max, index, pos) => write!( f, - "Array index {} is out of bounds: only {} element{} in the array ({})", - index, - max, - if *max > 1 { "s" } else { "" }, - pos + "Array index {} is out of bounds: only {} elements in the array ({})", + index, max, pos ), Self::ErrorStringBounds(_, index, pos) if *index < 0 => { write!(f, "{}: {} < 0 ({})", desc, index, pos) } - Self::ErrorStringBounds(max, _, pos) if *max == 0 => write!(f, "{} ({})", desc, pos), + Self::ErrorStringBounds(0, _, pos) => write!(f, "{} ({})", desc, pos), + Self::ErrorStringBounds(1, index, pos) => write!( + f, + "String index {} is out of bounds: only one character in the string ({})", + index, pos + ), Self::ErrorStringBounds(max, index, pos) => write!( f, - "String index {} is out of bounds: only {} character{} in the string ({})", - index, - max, - if *max > 1 { "s" } else { "" }, - pos + "String index {} is out of bounds: only {} characters in the string ({})", + index, max, pos ), } } diff --git a/src/scope.rs b/src/scope.rs index d097f422..ca3ec4d9 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -1,6 +1,7 @@ //! Module that defines the `Scope` type representing a function call-stack scope. use crate::any::{Any, Dynamic}; + use std::borrow::Cow; /// A type containing information about current scope. diff --git a/tests/math.rs b/tests/math.rs index 4dd7fecc..e9565b39 100644 --- a/tests/math.rs +++ b/tests/math.rs @@ -24,6 +24,10 @@ fn test_math() -> Result<(), EvalAltResult> { { #[cfg(not(feature = "only_i32"))] { + match engine.eval::("(-9223372036854775808).abs()") { + Err(EvalAltResult::ErrorArithmetic(_, _)) => (), + r => panic!("should return overflow error: {:?}", r), + } match engine.eval::("9223372036854775807 + 1") { Err(EvalAltResult::ErrorArithmetic(_, _)) => (), r => panic!("should return overflow error: {:?}", r),