From 6f77471354969528cacdc8c3c4a7a6795664f2d5 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Mon, 22 Apr 2019 19:08:55 +0200 Subject: [PATCH] Fixed aggregation operator Manage sequence operators, which currently are `Chain` and `Token` on the stack without ever inserting unfinished sequence operator nodes into another node. Relates to #44 --- src/context/mod.rs | 2 +- src/operator/mod.rs | 19 +++--- src/tree/mod.rs | 140 ++++++++++++++++++++++++++++++++++++++----- src/value/mod.rs | 4 +- tests/integration.rs | 53 +++++++++++++--- 5 files changed, 185 insertions(+), 33 deletions(-) diff --git a/src/context/mod.rs b/src/context/mod.rs index 9988ebd..e07f9c7 100644 --- a/src/context/mod.rs +++ b/src/context/mod.rs @@ -103,7 +103,7 @@ impl Context for HashMapContext { /// /// let ctx = evalexpr::context_map! { /// "x" => 8, -/// "f" => Function::new(None, Box::new(|_| Ok(42.into()) )) +/// "f" => Function::new(None, Box::new(|_| Ok(42.into()) )) /// }.unwrap(); /// /// assert_eq!(eval_with_context("x + f()", &ctx), Ok(50.into())); diff --git a/src/operator/mod.rs b/src/operator/mod.rs index a131b55..8725b5d 100644 --- a/src/operator/mod.rs +++ b/src/operator/mod.rs @@ -4,7 +4,7 @@ use crate::{context::Context, error::*, value::Value}; mod display; -#[derive(Debug)] +#[derive(Debug, PartialEq)] pub enum Operator { RootNode, @@ -93,9 +93,10 @@ impl Operator { /// 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 - fn is_flatten_chains(&self) -> bool { + pub(crate) fn is_sequence(&self) -> bool { + use crate::operator::Operator::*; match self { - Operator::Tuple => true, + Tuple | Chain => true, _ => false, } } @@ -112,8 +113,8 @@ impl Operator { use crate::operator::Operator::*; match self { Add | Sub | Mul | Div | Mod | Exp | Eq | Neq | Gt | Lt | Geq | Leq | And | Or - | Assign | Chain => Some(2), - Tuple => None, + | Assign => Some(2), + Tuple | Chain => None, Not | Neg | RootNode => Some(1), Const { value: _ } => Some(0), VariableIdentifier { identifier: _ } => Some(0), @@ -420,7 +421,9 @@ impl Operator { } } Tuple => { - expect_operator_argument_amount(arguments.len(), 2)?; + Ok(Value::Tuple(arguments.into())) + + /*expect_operator_argument_amount(arguments.len(), 2)?; if let Value::Tuple(tuple) = &arguments[0] { let mut tuple = tuple.clone(); if let Value::Tuple(tuple2) = &arguments[1] { @@ -440,7 +443,7 @@ impl Operator { arguments[1].clone(), ])) } - } + }*/ } Assign => Err(EvalexprError::ContextNotManipulable), Chain => { @@ -448,7 +451,7 @@ impl Operator { return Err(EvalexprError::wrong_operator_argument_amount(0, 1)); } - Ok(arguments.get(1).cloned().unwrap_or(Value::Empty)) + Ok(arguments.last().cloned().unwrap_or(Value::Empty)) } Const { value } => { expect_operator_argument_amount(arguments.len(), 0)?; diff --git a/src/tree/mod.rs b/src/tree/mod.rs index b763e9b..2904440 100644 --- a/src/tree/mod.rs +++ b/src/tree/mod.rs @@ -11,8 +11,7 @@ use crate::{ operator::*, value::Value, }; -use std::error::Error; -use std::any::Any; +use std::mem; mod display; mod iter; @@ -34,10 +33,10 @@ mod iter; /// assert_eq!(node.eval_with_context(&context), Ok(Value::from(3))); /// ``` /// -#[derive(Debug)] +#[derive(Debug, PartialEq)] pub struct Node { - children: Vec, operator: Operator, + children: Vec, } impl Node { @@ -368,6 +367,7 @@ impl Node { } fn insert_back_prioritized(&mut self, node: Node, is_root_node: bool) -> EvalexprResult<()> { + println!("Inserting {:?} into {:?}", node.operator, self.operator()); if self.operator().precedence() < node.operator().precedence() || is_root_node // Right-to-left chaining || (self.operator().precedence() == node.operator().precedence() && !self.operator().is_left_to_right() && !node.operator().is_left_to_right()) @@ -381,14 +381,13 @@ impl Node { || (self.children.last().unwrap().operator().precedence() == node.operator().precedence() && !self.children.last().unwrap().operator().is_left_to_right() && !node.operator().is_left_to_right()) { + println!("Recursing into {:?}", self.children.last().unwrap().operator()); self.children .last_mut() .unwrap() .insert_back_prioritized(node, false) - } else if self.children.last().unwrap().operator().type_id() == node.operator().type_id() && node.operator().is_flatten_chains() && !self.children.last().unwrap().has_enough_children() { - // The operators will be chained together, and the next value will be added to this nodes last child. - Ok(()) } else { + println!("Rotating"); if node.operator().is_leaf() { return Err(EvalexprError::AppendedToLeafNode); } @@ -401,6 +400,7 @@ impl Node { Ok(()) } } else { + println!("Inserting as specified"); self.children.push(node); Ok(()) } @@ -410,8 +410,63 @@ impl Node { } } +fn collapse_root_stack_to(root_stack: &mut Vec, mut root: Node, collapse_goal: &Node) -> EvalexprResult { + loop { + if let Some(mut potential_higher_root) = root_stack.pop() { + // TODO I'm not sure about this >, as I have no example for different sequence operators with the same precedence + if potential_higher_root.operator().precedence() > collapse_goal.operator().precedence() { + potential_higher_root.children.push(root); + root = potential_higher_root; + } else { + root_stack.push(potential_higher_root); + break; + } + } else { + // This is the only way the topmost root node could have been removed + return Err(EvalexprError::UnmatchedRBrace); + } + } + + Ok(root) +} + +fn collapse_all_sequences(root_stack: &mut Vec) -> EvalexprResult<()> { + println!("Collapsing all sequences"); + println!("Initial root stack is: {:?}", root_stack); + let mut root = if let Some(root) = root_stack.pop() { + root + } else { + return Err(EvalexprError::UnmatchedRBrace); + }; + + loop { + println!("Root is: {:?}", root); + if root.operator() == &Operator::RootNode { + root_stack.push(root); + break; + } + + if let Some(mut potential_higher_root) = root_stack.pop() { + if root.operator().is_sequence() { + potential_higher_root.children.push(root); + root = potential_higher_root; + } else { + root_stack.push(potential_higher_root); + root_stack.push(root); + break; + } + } else { + // This is the only way the topmost root node could have been removed + return Err(EvalexprError::UnmatchedRBrace); + } + } + + println!("Root stack after collapsing all sequences is: {:?}", root_stack); + Ok(()) +} + pub(crate) fn tokens_to_operator_tree(tokens: Vec) -> EvalexprResult { - let mut root = vec![Node::root_node()]; + let mut root_stack = vec![Node::root_node()]; let mut last_token_is_rightsided_value = false; let mut token_iter = tokens.iter().peekable(); @@ -443,14 +498,15 @@ pub(crate) fn tokens_to_operator_tree(tokens: Vec) -> EvalexprResult Some(Node::new(Operator::Not)), Token::LBrace => { - root.push(Node::root_node()); + root_stack.push(Node::root_node()); None } Token::RBrace => { - if root.len() < 2 { + if root_stack.len() <= 1 { return Err(EvalexprError::UnmatchedRBrace); } else { - root.pop() + collapse_all_sequences(&mut root_stack)?; + root_stack.pop() } } @@ -475,9 +531,58 @@ pub(crate) fn tokens_to_operator_tree(tokens: Vec) -> EvalexprResult Some(Node::new(Operator::value(Value::String(string)))), }; - if let Some(node) = node { - if let Some(root) = root.last_mut() { - root.insert_back_prioritized(node, true)?; + if let Some(mut node) = node { + // Need to pop and then repush here, because Rust 1.33.0 cannot release the mutable borrow of root_stack before the end of this complete if-statement + if let Some(mut root) = root_stack.pop() { + if node.operator().is_sequence() { + println!("Found a sequence operator"); + println!("Stack before sequence operation: {:?}, {:?}", root_stack, root); + // If root.operator() and node.operator() are of the same variant, ... + if mem::discriminant(root.operator()) == mem::discriminant(node.operator()) { + // ... we create a new root node for the next expression in the sequence + root.children.push(Node::root_node()); + root_stack.push(root); + } else if root.operator() == &Operator::RootNode { + // If the current root is an actual root node, we start a new sequence + node.children.push(root); + node.children.push(Node::root_node()); + root_stack.push(Node::root_node()); + root_stack.push(node); + } else { + // Otherwise, we combine the sequences based on their precedences + // TODO I'm not sure about this <, as I have no example for different sequence operators with the same precedence + if root.operator().precedence() < node.operator().precedence() { + // If the new sequence has a higher precedence, it is part of the last element of the current root sequence + if let Some(last_root_child) = root.children.pop() { + node.children.push(last_root_child); + node.children.push(Node::root_node()); + root_stack.push(root); + root_stack.push(node); + } else { + // Once a sequence has been pushed on top of the stack, it also gets a child + unreachable!() + } + } else { + // If the new sequence doesn't have a higher precedence, then all sequences with a higher precedence are collapsed below this one + root = collapse_root_stack_to(&mut root_stack, root, &node)?; + node.children.push(root); + root_stack.push(node); + } + } + println!("Stack after sequence operation: {:?}", root_stack); + } else if root.operator().is_sequence() { + if let Some(mut last_root_child) = root.children.pop() { + last_root_child.insert_back_prioritized(node, true)?; + root.children.push(last_root_child); + root_stack.push(root); + } else { + // Once a sequence has been pushed on top of the stack, it also gets a child + unreachable!() + } + } else { + root.insert_back_prioritized(node, true)?; + root_stack.push(root); + } } else { return Err(EvalexprError::UnmatchedRBrace); } @@ -486,9 +591,12 @@ pub(crate) fn tokens_to_operator_tree(tokens: Vec) -> EvalexprResult 1 { + // In the end, all sequences are implicitly terminated + collapse_all_sequences(&mut root_stack)?; + + if root_stack.len() > 1 { Err(EvalexprError::UnmatchedLBrace) - } else if let Some(root) = root.pop() { + } else if let Some(root) = root_stack.pop() { Ok(root) } else { Err(EvalexprError::UnmatchedRBrace) diff --git a/src/value/mod.rs b/src/value/mod.rs index 17aafe2..e12bef9 100644 --- a/src/value/mod.rs +++ b/src/value/mod.rs @@ -195,7 +195,9 @@ impl From for EvalexprResult { } impl From<()> for Value { - fn from(_: ()) -> Self { Value::Empty } + fn from(_: ()) -> Self { + Value::Empty + } } #[cfg(test)] diff --git a/tests/integration.rs b/tests/integration.rs index 35428a2..02bc439 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -251,6 +251,7 @@ fn test_n_ary_functions() { context .set_value("five".to_string(), Value::Int(5)) .unwrap(); + context.set_function("function_four".into(), Function::new(Some(0), Box::new(|_| {Ok(Value::Int(4))}))).unwrap(); assert_eq!(eval_with_context("avg(7, 5)", &context), Ok(Value::Int(6))); assert_eq!( @@ -265,16 +266,20 @@ fn test_n_ary_functions() { eval_with_context("sub2 avg(3, 6)", &context), Ok(Value::Int(2)) ); + dbg!(build_operator_tree("muladd(3, 6, -4)").unwrap()); assert_eq!( eval_with_context("muladd(3, 6, -4)", &context), Ok(Value::Int(14)) ); assert_eq!(eval_with_context("count()", &context), Ok(Value::Int(0))); + assert_eq!(eval_with_context("count((1, 2, 3))", &context), Ok(Value::Int(1))); assert_eq!( eval_with_context("count(3, 5.5, 2)", &context), Ok(Value::Int(3)) ); assert_eq!(eval_with_context("count 5", &context), Ok(Value::Int(1))); + assert_eq!(eval_with_context("function_four()", &context), Ok(Value::Int(4))); + assert_eq!(eval_with_context("function_four(())", &context), Err(EvalexprError::WrongFunctionArgumentAmount{expected: 0, actual: 1})); } #[test] @@ -610,12 +615,46 @@ fn test_serde() { fn test_tuple_definitions() { assert_eq!(eval_empty("()"), Ok(())); assert_eq!(eval_int("(3)"), Ok(3)); - assert_eq!(eval_tuple("(3, 4)"), Ok(vec![Value::from(3), Value::from(4)])); - assert_eq!(eval_tuple("2, (5, 6)"), Ok(vec![Value::from(2), Value::from(vec![Value::from(5), Value::from(6)])])); + assert_eq!( + eval_tuple("(3, 4)"), + Ok(vec![Value::from(3), Value::from(4)]) + ); + assert_eq!( + eval_tuple("2, (5, 6)"), + Ok(vec![ + Value::from(2), + Value::from(vec![Value::from(5), Value::from(6)]) + ]) + ); assert_eq!(eval_tuple("1, 2"), Ok(vec![Value::from(1), Value::from(2)])); - assert_eq!(eval_tuple("1, 2, 3, 4"), Ok(vec![Value::from(1), Value::from(2), Value::from(3), Value::from(4)])); - assert_eq!(eval_tuple("(1, 2, 3), 5, 6, (true, false, 0)"), Ok(vec![Value::from(vec![Value::from(1), Value::from(2), Value::from(3)]), Value::from(5), Value::from(6), Value::from(vec![Value::from(true), Value::from(false), Value::from(0)])])); - assert_eq!(eval_tuple("1, (2)"), Ok(vec![Value::from(1), Value::from(2)])); - assert_eq!(eval_tuple("1, ()"), Ok(vec![Value::from(1), Value::from(())])); - assert_eq!(eval_tuple("1, ((2))"), Ok(vec![Value::from(1), Value::from(2)])); + assert_eq!( + eval_tuple("1, 2, 3, 4"), + Ok(vec![ + Value::from(1), + Value::from(2), + Value::from(3), + Value::from(4) + ]) + ); + assert_eq!( + eval_tuple("(1, 2, 3), 5, 6, (true, false, 0)"), + Ok(vec![ + Value::from(vec![Value::from(1), Value::from(2), Value::from(3)]), + Value::from(5), + Value::from(6), + Value::from(vec![Value::from(true), Value::from(false), Value::from(0)]) + ]) + ); + assert_eq!( + eval_tuple("1, (2)"), + Ok(vec![Value::from(1), Value::from(2)]) + ); + assert_eq!( + eval_tuple("1, ()"), + Ok(vec![Value::from(1), Value::from(())]) + ); + assert_eq!( + eval_tuple("1, ((2))"), + Ok(vec![Value::from(1), Value::from(2)]) + ); }