From 1316ae63d02d53c3968cc3a3db8a6cd8d6bd4f4a Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Fri, 28 May 2021 08:56:21 +0300 Subject: [PATCH 1/3] Update code formatting. --- benches/benchs.rs | 49 +++++++++++++++++++++----------------------- rustfmt.toml | 2 +- src/tree/mod.rs | 6 +++++- tests/integration.rs | 1 - 4 files changed, 29 insertions(+), 29 deletions(-) diff --git a/benches/benchs.rs b/benches/benchs.rs index 1832a22..3ec8aa0 100644 --- a/benches/benchs.rs +++ b/benches/benchs.rs @@ -1,16 +1,14 @@ #![feature(test)] -extern crate test; extern crate rand; extern crate rand_pcg; +extern crate test; -use test::Bencher; -use rand_pcg::Pcg32; -use rand::{Rng, SeedableRng}; -use rand::distributions::Uniform; -use rand::seq::SliceRandom; use evalexpr::build_operator_tree; +use rand::{distributions::Uniform, seq::SliceRandom, Rng, SeedableRng}; +use rand_pcg::Pcg32; use std::hint::black_box; +use test::Bencher; const BENCHMARK_LEN: usize = 100_000; const EXPONENTIAL_TUPLE_ITERATIONS: usize = 12; @@ -69,9 +67,7 @@ fn bench_parse_long_expression_chains(bencher: &mut Bencher) { let mut gen = Pcg32::seed_from_u64(0); let long_expression_chain = generate_expression_chain(BENCHMARK_LEN, &mut gen); - bencher.iter(|| { - build_operator_tree(&long_expression_chain).unwrap() - }); + bencher.iter(|| build_operator_tree(&long_expression_chain).unwrap()); } #[bench] @@ -79,9 +75,7 @@ fn bench_parse_deep_expression_trees(bencher: &mut Bencher) { let mut gen = Pcg32::seed_from_u64(15); let deep_expression_tree = generate_expression(BENCHMARK_LEN, &mut gen); - bencher.iter(|| { - build_operator_tree(&deep_expression_tree).unwrap() - }); + bencher.iter(|| build_operator_tree(&deep_expression_tree).unwrap()); } #[bench] @@ -99,27 +93,28 @@ fn bench_parse_many_small_expressions(bencher: &mut Bencher) { #[bench] fn bench_evaluate_long_expression_chains(bencher: &mut Bencher) { let mut gen = Pcg32::seed_from_u64(0); - let long_expression_chain = build_operator_tree(&generate_expression_chain(BENCHMARK_LEN, &mut gen)).unwrap(); + let long_expression_chain = + build_operator_tree(&generate_expression_chain(BENCHMARK_LEN, &mut gen)).unwrap(); - bencher.iter(|| { - long_expression_chain.eval().unwrap() - }); + bencher.iter(|| long_expression_chain.eval().unwrap()); } #[bench] fn bench_evaluate_deep_expression_trees(bencher: &mut Bencher) { let mut gen = Pcg32::seed_from_u64(15); - let deep_expression_tree = build_operator_tree(&generate_expression(BENCHMARK_LEN, &mut gen)).unwrap(); + let deep_expression_tree = + build_operator_tree(&generate_expression(BENCHMARK_LEN, &mut gen)).unwrap(); - bencher.iter(|| { - deep_expression_tree.eval().unwrap() - }); + bencher.iter(|| deep_expression_tree.eval().unwrap()); } #[bench] fn bench_evaluate_many_small_expressions(bencher: &mut Bencher) { let mut gen = Pcg32::seed_from_u64(33); - let small_expressions: Vec<_> = generate_small_expressions(BENCHMARK_LEN, &mut gen).iter().map(|expression| build_operator_tree(&expression).unwrap()).collect(); + let small_expressions: Vec<_> = generate_small_expressions(BENCHMARK_LEN, &mut gen) + .iter() + .map(|expression| build_operator_tree(&expression).unwrap()) + .collect(); bencher.iter(|| { for expression in &small_expressions { @@ -131,10 +126,12 @@ fn bench_evaluate_many_small_expressions(bencher: &mut Bencher) { #[bench] fn bench_evaluate_large_tuple_expression(bencher: &mut Bencher) { let mut gen = Pcg32::seed_from_u64(44); - let large_tuple_expression = build_operator_tree(&generate_large_tuple_expression(EXPONENTIAL_TUPLE_ITERATIONS, &mut gen)).unwrap(); + let large_tuple_expression = build_operator_tree(&generate_large_tuple_expression( + EXPONENTIAL_TUPLE_ITERATIONS, + &mut gen, + )) + .unwrap(); dbg!(&large_tuple_expression); - bencher.iter(|| { - large_tuple_expression.eval().unwrap() - }); -} \ No newline at end of file + bencher.iter(|| large_tuple_expression.eval().unwrap()); +} diff --git a/rustfmt.toml b/rustfmt.toml index e1b078b..bca1a3d 100644 --- a/rustfmt.toml +++ b/rustfmt.toml @@ -2,5 +2,5 @@ edition = "2018" reorder_imports=true reorder_modules=true format_strings=true -merge_imports=true +imports_granularity="Crate" match_block_trailing_comma=true \ No newline at end of file diff --git a/src/tree/mod.rs b/src/tree/mod.rs index 9d7a78d..11fab5d 100644 --- a/src/tree/mod.rs +++ b/src/tree/mod.rs @@ -1,4 +1,8 @@ -use crate::{token::Token, value::{TupleType, EMPTY_VALUE}, EmptyType, FloatType, IntType, HashMapContext}; +use crate::{ + token::Token, + value::{TupleType, EMPTY_VALUE}, + EmptyType, FloatType, HashMapContext, IntType, +}; use crate::{ context::Context, diff --git a/tests/integration.rs b/tests/integration.rs index 7957c9b..6c3dddc 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -452,7 +452,6 @@ fn test_shortcut_functions() { .eval_string_with_context_mut(&mut context), Ok("a string".to_string()) ); - ; assert_eq!( build_operator_tree("3.3") .unwrap() From 6e5ff8615a8570848df811ec5f2f0ae875e8761f Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Fri, 28 May 2021 09:07:26 +0300 Subject: [PATCH 2/3] Fix lints and benches. --- benches/benchs.rs | 1 + src/context/mod.rs | 2 +- src/function/builtin.rs | 4 +- src/operator/mod.rs | 125 ++++++++++------------------------------ src/token/mod.rs | 21 ++++--- src/value/mod.rs | 35 +++-------- 6 files changed, 54 insertions(+), 134 deletions(-) diff --git a/benches/benchs.rs b/benches/benchs.rs index 3ec8aa0..beb32c3 100644 --- a/benches/benchs.rs +++ b/benches/benchs.rs @@ -1,4 +1,5 @@ #![feature(test)] +#![feature(bench_black_box)] extern crate rand; extern crate rand_pcg; diff --git a/src/context/mod.rs b/src/context/mod.rs index 7a062ac..0925cce 100644 --- a/src/context/mod.rs +++ b/src/context/mod.rs @@ -86,7 +86,7 @@ impl Context for HashMapContext { } fn set_function(&mut self, identifier: String, function: Function) -> EvalexprResult<()> { - self.functions.insert(identifier.into(), function); + self.functions.insert(identifier, function); Ok(()) } } diff --git a/src/function/builtin.rs b/src/function/builtin.rs index a5f76c5..21f54fc 100644 --- a/src/function/builtin.rs +++ b/src/function/builtin.rs @@ -20,7 +20,7 @@ pub fn builtin_function(identifier: &str) -> Option { } else if let Value::Int(int) = argument { min_int = min_int.min(int); } else { - return Err(EvalexprError::expected_number(argument.clone())); + return Err(EvalexprError::expected_number(argument)); } } @@ -42,7 +42,7 @@ pub fn builtin_function(identifier: &str) -> Option { } else if let Value::Int(int) = argument { max_int = max_int.max(int); } else { - return Err(EvalexprError::expected_number(argument.clone())); + return Err(EvalexprError::expected_number(argument)); } } diff --git a/src/operator/mod.rs b/src/operator/mod.rs index cc3aa01..e4cf713 100644 --- a/src/operator/mod.rs +++ b/src/operator/mod.rs @@ -136,21 +136,14 @@ impl Operator { // Make this a const fn once #57563 is resolved pub(crate) fn is_left_to_right(&self) -> bool { use crate::operator::Operator::*; - match self { - Assign => false, - FunctionIdentifier { identifier: _ } => false, - _ => true, - } + !matches!(self, Assign | FunctionIdentifier { identifier: _ }) } /// Returns true if chains of this operator should be flattened into one operator with many arguments. // Make this a const fn once #57563 is resolved pub(crate) fn is_sequence(&self) -> bool { use crate::operator::Operator::*; - match self { - Tuple | Chain => true, - _ => false, - } + matches!(self, Tuple | Chain) } /// True if this operator is a leaf, meaning it accepts no arguments. @@ -327,20 +320,12 @@ impl Operator { Eq => { expect_operator_argument_amount(arguments.len(), 2)?; - if arguments[0] == arguments[1] { - Ok(Value::Boolean(true)) - } else { - Ok(Value::Boolean(false)) - } + Ok(Value::Boolean(arguments[0] == arguments[1])) }, Neq => { expect_operator_argument_amount(arguments.len(), 2)?; - if arguments[0] != arguments[1] { - Ok(Value::Boolean(true)) - } else { - Ok(Value::Boolean(false)) - } + Ok(Value::Boolean(arguments[0] != arguments[1])) }, Gt => { expect_operator_argument_amount(arguments.len(), 2)?; @@ -348,23 +333,13 @@ impl Operator { expect_number_or_string(&arguments[1])?; if let (Ok(a), Ok(b)) = (arguments[0].as_string(), arguments[1].as_string()) { - if a > b { - Ok(Value::Boolean(true)) - } else { - Ok(Value::Boolean(false)) - } + Ok(Value::Boolean(a > b)) } else if let (Ok(a), Ok(b)) = (arguments[0].as_int(), arguments[1].as_int()) { - if a > b { - Ok(Value::Boolean(true)) - } else { - Ok(Value::Boolean(false)) - } + Ok(Value::Boolean(a > b)) } else { - if arguments[0].as_number()? > arguments[1].as_number()? { - Ok(Value::Boolean(true)) - } else { - Ok(Value::Boolean(false)) - } + Ok(Value::Boolean( + arguments[0].as_number()? > arguments[1].as_number()?, + )) } }, Lt => { @@ -373,23 +348,13 @@ impl Operator { expect_number_or_string(&arguments[1])?; if let (Ok(a), Ok(b)) = (arguments[0].as_string(), arguments[1].as_string()) { - if a < b { - Ok(Value::Boolean(true)) - } else { - Ok(Value::Boolean(false)) - } + Ok(Value::Boolean(a < b)) } else if let (Ok(a), Ok(b)) = (arguments[0].as_int(), arguments[1].as_int()) { - if a < b { - Ok(Value::Boolean(true)) - } else { - Ok(Value::Boolean(false)) - } + Ok(Value::Boolean(a < b)) } else { - if arguments[0].as_number()? < arguments[1].as_number()? { - Ok(Value::Boolean(true)) - } else { - Ok(Value::Boolean(false)) - } + Ok(Value::Boolean( + arguments[0].as_number()? < arguments[1].as_number()?, + )) } }, Geq => { @@ -398,23 +363,13 @@ impl Operator { expect_number_or_string(&arguments[1])?; if let (Ok(a), Ok(b)) = (arguments[0].as_string(), arguments[1].as_string()) { - if a >= b { - Ok(Value::Boolean(true)) - } else { - Ok(Value::Boolean(false)) - } + Ok(Value::Boolean(a >= b)) } else if let (Ok(a), Ok(b)) = (arguments[0].as_int(), arguments[1].as_int()) { - if a >= b { - Ok(Value::Boolean(true)) - } else { - Ok(Value::Boolean(false)) - } + Ok(Value::Boolean(a >= b)) } else { - if arguments[0].as_number()? >= arguments[1].as_number()? { - Ok(Value::Boolean(true)) - } else { - Ok(Value::Boolean(false)) - } + Ok(Value::Boolean( + arguments[0].as_number()? >= arguments[1].as_number()?, + )) } }, Leq => { @@ -423,23 +378,13 @@ impl Operator { expect_number_or_string(&arguments[1])?; if let (Ok(a), Ok(b)) = (arguments[0].as_string(), arguments[1].as_string()) { - if a <= b { - Ok(Value::Boolean(true)) - } else { - Ok(Value::Boolean(false)) - } + Ok(Value::Boolean(a <= b)) } else if let (Ok(a), Ok(b)) = (arguments[0].as_int(), arguments[1].as_int()) { - if a <= b { - Ok(Value::Boolean(true)) - } else { - Ok(Value::Boolean(false)) - } + Ok(Value::Boolean(a <= b)) } else { - if arguments[0].as_number()? <= arguments[1].as_number()? { - Ok(Value::Boolean(true)) - } else { - Ok(Value::Boolean(false)) - } + Ok(Value::Boolean( + arguments[0].as_number()? <= arguments[1].as_number()?, + )) } }, And => { @@ -447,32 +392,20 @@ impl Operator { let a = arguments[0].as_boolean()?; let b = arguments[1].as_boolean()?; - if a && b { - Ok(Value::Boolean(true)) - } else { - Ok(Value::Boolean(false)) - } + Ok(Value::Boolean(a && b)) }, Or => { expect_operator_argument_amount(arguments.len(), 2)?; let a = arguments[0].as_boolean()?; let b = arguments[1].as_boolean()?; - if a || b { - Ok(Value::Boolean(true)) - } else { - Ok(Value::Boolean(false)) - } + Ok(Value::Boolean(a || b)) }, Not => { expect_operator_argument_amount(arguments.len(), 1)?; let a = arguments[0].as_boolean()?; - if !a { - Ok(Value::Boolean(true)) - } else { - Ok(Value::Boolean(false)) - } + Ok(Value::Boolean(!a)) }, Assign | AddAssign | SubAssign | MulAssign | DivAssign | ModAssign | ExpAssign | AndAssign | OrAssign => Err(EvalexprError::ContextNotManipulable), @@ -528,7 +461,7 @@ impl Operator { Assign => { expect_operator_argument_amount(arguments.len(), 2)?; let target = arguments[0].as_string()?; - context.set_value(target.into(), arguments[1].clone())?; + context.set_value(target, arguments[1].clone())?; Ok(Value::Empty) }, @@ -557,7 +490,7 @@ impl Operator { self ), }?; - context.set_value(target.into(), result)?; + context.set_value(target, result)?; Ok(Value::Empty) }, diff --git a/src/token/mod.rs b/src/token/mod.rs index e33d34e..9da2c7b 100644 --- a/src/token/mod.rs +++ b/src/token/mod.rs @@ -196,11 +196,18 @@ impl Token { pub(crate) fn is_assignment(&self) -> bool { use Token::*; - match self { - Assign | PlusAssign | MinusAssign | StarAssign | SlashAssign | PercentAssign - | HatAssign | AndAssign | OrAssign => true, - _ => false, - } + matches!( + self, + Assign + | PlusAssign + | MinusAssign + | StarAssign + | SlashAssign + | PercentAssign + | HatAssign + | AndAssign + | OrAssign + ) } } @@ -210,7 +217,7 @@ fn parse_escape_sequence>(iter: &mut Iter) -> Evalex Some('"') => Ok('"'), Some('\\') => Ok('\\'), Some(c) => Err(EvalexprError::IllegalEscapeSequence(format!("\\{}", c))), - None => Err(EvalexprError::IllegalEscapeSequence(format!("\\"))), + None => Err(EvalexprError::IllegalEscapeSequence("\\".to_string())), } } @@ -268,7 +275,7 @@ fn str_to_partial_tokens(string: &str) -> EvalexprResult> { /// Resolves all partial tokens by converting them to complex tokens. fn partial_tokens_to_tokens(mut tokens: &[PartialToken]) -> EvalexprResult> { let mut result = Vec::new(); - while tokens.len() > 0 { + while !tokens.is_empty() { let first = tokens[0].clone(); let second = tokens.get(1).cloned(); let third = tokens.get(2).cloned(); diff --git a/src/value/mod.rs b/src/value/mod.rs index 976664e..b5b14a4 100644 --- a/src/value/mod.rs +++ b/src/value/mod.rs @@ -40,57 +40,36 @@ pub enum Value { impl Value { /// Returns true if `self` is a `Value::String`. pub fn is_string(&self) -> bool { - match self { - Value::String(_) => true, - _ => false, - } + matches!(self, Value::String(_)) } /// Returns true if `self` is a `Value::Int`. pub fn is_int(&self) -> bool { - match self { - Value::Int(_) => true, - _ => false, - } + matches!(self, Value::Int(_)) } /// Returns true if `self` is a `Value::Float`. pub fn is_float(&self) -> bool { - match self { - Value::Float(_) => true, - _ => false, - } + matches!(self, Value::Float(_)) } /// Returns true if `self` is a `Value::Int` or `Value::Float`. pub fn is_number(&self) -> bool { - match self { - Value::Int(_) | Value::Float(_) => true, - _ => false, - } + matches!(self, Value::Int(_) | Value::Float(_)) } /// Returns true if `self` is a `Value::Boolean`. pub fn is_boolean(&self) -> bool { - match self { - Value::Boolean(_) => true, - _ => false, - } + matches!(self, Value::Boolean(_)) } /// Returns true if `self` is a `Value::Tuple`. pub fn is_tuple(&self) -> bool { - match self { - Value::Tuple(_) => true, - _ => false, - } + matches!(self, Value::Tuple(_)) } /// Returns true if `self` is a `Value::Empty`. pub fn is_empty(&self) -> bool { - match self { - Value::Empty => true, - _ => false, - } + matches!(self, Value::Empty) } /// Clones the value stored in `self` as `String`, or returns `Err` if `self` is not a `Value::String`. From 7bc05e0770afea1cc45f8e63b77756f29c6f64d3 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Fri, 28 May 2021 09:09:04 +0300 Subject: [PATCH 3/3] Make node cloneable. --- src/tree/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tree/mod.rs b/src/tree/mod.rs index 11fab5d..7cca5d7 100644 --- a/src/tree/mod.rs +++ b/src/tree/mod.rs @@ -32,7 +32,7 @@ mod iter; /// assert_eq!(node.eval_with_context(&context), Ok(Value::from(3))); /// ``` /// -#[derive(Debug, PartialEq)] +#[derive(Debug, PartialEq, Clone)] pub struct Node { operator: Operator, children: Vec,