FEATURE: evaluate dot selectors as a binary operator now.

Also fixed a precedence bug in our parser.

integration_tests/modules_test.ucg

@@ -22,9 +22,9 @@ assert |
 
 let embedded_mod = module {
     deep_value = "None",
-    env = "None",
+    environ = "None",
 } => {
-    let env_name = select mod.env, "qa", {
+    let env_name = select mod.environ, "qa", {
         None = "qa",
         prod = "prod",
         qa = "qa",

integration_tests/operator_precedence_test.ucg

@@ -24,4 +24,19 @@ assert |
 |;
 assert |
     2 - 1 == 1;
+|;
+assert |
+    1 + 1 + 1 + 1 == 4;
+|;
+assert |
+    1 + 1 + 2 * 2 + 1 + 1 == 1 + 1 + (2 * 2) + 1 + 1;
+|;
+let tpl = {
+    one = {
+        two = 12,
+    },
+};
+
+assert |
+    1 + tpl.one.two * 2 + 3 == 28;
 |;

src/ast/mod.rs

@@ -499,6 +499,31 @@ pub enum BinaryExprType {
     DOT,
 }
 
+impl BinaryExprType {
+    /// Returns the precedence level for the binary operator.
+    ///
+    /// Higher values bind tighter than lower values.
+    pub fn precedence_level(&self) -> u32 {
+        match self {
+            // Equality operators are least tightly bound
+            BinaryExprType::Equal => 1,
+            BinaryExprType::NotEqual => 1,
+            BinaryExprType::GTEqual => 1,
+            BinaryExprType::LTEqual => 1,
+            BinaryExprType::GT => 1,
+            BinaryExprType::LT => 1,
+            // Sum operators are next least tightly bound
+            BinaryExprType::Add => 2,
+            BinaryExprType::Sub => 2,
+            // Product operators are next tightly bound
+            BinaryExprType::Mul => 3,
+            BinaryExprType::Div => 3,
+            // Dot operators are most tightly bound.
+            BinaryExprType::DOT => 4,
+        }
+    }
+}
+
 /// Represents an expression with a left and a right side.
 #[derive(Debug, PartialEq, Clone)]
 pub struct BinaryOpDef {

src/build/mod.rs

@@ -28,7 +28,7 @@ use std::string::ToString;
 use simple_error;
 
 use crate::ast::*;
-use crate::build::scope::{Scope, ValueMap};
+use crate::build::scope::{find_in_fieldlist, Scope, ValueMap};
 use crate::error;
 use crate::format;
 use crate::iter::OffsetStrIter;
@@ -459,103 +459,6 @@ impl<'a> FileBuilder<'a> {
         }
     }
 
-    fn find_in_fieldlist(
-        target: &str,
-        fs: &Vec<(PositionedItem<String>, Rc<Val>)>,
-    ) -> Option<Rc<Val>> {
-        for (key, val) in fs.iter().cloned() {
-            if target == &key.val {
-                return Some(val.clone());
-            }
-        }
-        return None;
-    }
-
-    fn lookup_in_env(
-        &self,
-        pos: &Position,
-        field: &Rc<Val>,
-        fs: &Vec<(String, String)>,
-    ) -> Result<Rc<Val>, Box<Error>> {
-        let field = if let &Val::Str(ref name) = field.as_ref() {
-            name
-        } else {
-            return Err(Box::new(error::BuildError::new(
-                format!("Invalid type {} for field lookup in env", field),
-                error::ErrorType::TypeFail,
-                pos.clone(),
-            )));
-        };
-        for &(ref name, ref val) in fs.iter() {
-            if field == name {
-                return Ok(Rc::new(Val::Str(val.clone())));
-            } else if !self.strict {
-                return Ok(Rc::new(Val::Empty));
-            }
-        }
-        return Err(Box::new(error::BuildError::new(
-            format!("Environment Variable {} not set", field),
-            error::ErrorType::NoSuchSymbol,
-            pos.clone(),
-        )));
-    }
-
-    // TODO: Do as part of a binary operator selector lookup.
-    fn lookup_in_tuple(
-        &self,
-        pos: &Position,
-        field: &Val,
-        fs: &Vec<(PositionedItem<String>, Rc<Val>)>,
-    ) -> Result<Rc<Val>, Box<Error>> {
-        let field = if let &Val::Str(ref name) = field {
-            name
-        } else {
-            return Err(Box::new(error::BuildError::new(
-                format!("Invalid type {} for field lookup in tuple", field),
-                error::ErrorType::TypeFail,
-                pos.clone(),
-            )));
-        };
-        if let Some(vv) = Self::find_in_fieldlist(&field, fs) {
-            Ok(vv)
-        } else {
-            Err(Box::new(error::BuildError::new(
-                format!("Unable to {} match element in tuple.", field,),
-                error::ErrorType::NoSuchSymbol,
-                pos.clone(),
-            )))
-        }
-    }
-
-    // TODO: Do as part of a binary operator selector lookup.
-    fn lookup_in_list(
-        &self,
-        pos: &Position,
-        field: &Rc<Val>,
-        elems: &Vec<Rc<Val>>,
-    ) -> Result<Rc<Val>, Box<Error>> {
-        let idx = match field.as_ref() {
-            &Val::Int(i) => i as usize,
-            &Val::Str(ref s) => s.parse::<usize>()?,
-            _ => {
-                return Err(Box::new(error::BuildError::new(
-                    format!("Invalid idx type {} for list lookup", field),
-                    error::ErrorType::TypeFail,
-                    pos.clone(),
-                )))
-            }
-        };
-        if idx < elems.len() {
-            Ok(elems[idx].clone())
-        } else {
-            Err(Box::new(error::BuildError::new(
-                format!("idx {} out of bounds in list", idx),
-                error::ErrorType::NoSuchSymbol,
-                pos.clone(),
-            )))
-        }
-    }
-
     fn add_vals(
         &self,
         pos: &Position,
@@ -809,29 +712,37 @@ impl<'a> FileBuilder<'a> {
         )))
     }
 
-    fn do_dot_lookup(
+    fn do_dot_lookup(&mut self, right: &Expression, scope: &Scope) -> Result<Rc<Val>, Box<Error>> {
-        &mut self,
+        match right {
-        pos: &Position,
+            Expression::Copy(_) => return self.eval_expr(right, scope),
-        left: Rc<Val>,
+            Expression::Call(_) => return self.eval_expr(right, scope),
-        right: Rc<Val>,
+            Expression::Simple(Value::Symbol(ref s)) => {
-        scope: &Scope,
+                self.eval_value(&Value::Symbol(s.clone()), scope)
-    ) -> Result<Rc<Val>, Box<Error>> {
+            }
-        match left.as_ref() {
+            Expression::Simple(Value::Str(ref s)) => {
-            &Val::Tuple(ref fs) => self.lookup_in_tuple(pos, &right, fs),
+                self.eval_value(&Value::Symbol(s.clone()), scope)
-            &Val::List(ref fs) => self.lookup_in_list(pos, &right, fs),
+            }
-            &Val::Env(ref fs) => self.lookup_in_env(pos, &right, fs),
+            Expression::Simple(Value::Int(ref i)) => {
-            _ => Err(Box::new(error::BuildError::new(
+                scope.lookup_idx(right.pos(), &Val::Int(i.val))
-                "Invalid type left operand for dot lookup",
+            }
-                error::ErrorType::TypeFail,
+            _ => self.eval_expr(right, scope),
-                pos.clone(),
-            ))),
         }
     }
 
     fn eval_binary(&mut self, def: &BinaryOpDef, scope: &Scope) -> Result<Rc<Val>, Box<Error>> {
         let kind = &def.kind;
         let left = self.eval_expr(&def.left, scope)?;
-        let right = self.eval_expr(&def.right, scope)?;
+        let mut child_scope = scope.spawn_child();
+        child_scope.set_curr_val(left.clone());
+        child_scope.search_curr_val = true;
+        if let &BinaryExprType::DOT = kind {
+            return self.do_dot_lookup(&def.right, &child_scope);
+        };
+        // TODO(jwall): We need to handle call and copy expressions specially.
+        let right = match self.eval_expr(&def.right, scope) {
+            Ok(v) => v,
+            Err(e) => return Err(e),
+        };
         match kind {
             // Handle math and concatenation operators here
             &BinaryExprType::Add => self.add_vals(&def.pos, left, right),
@@ -846,7 +757,7 @@ impl<'a> FileBuilder<'a> {
             &BinaryExprType::LTEqual => self.do_ltequal(&def.pos, left, right),
             &BinaryExprType::NotEqual => self.do_not_deep_equal(&def.pos, left, right),
             // TODO Handle the whole selector lookup logic here.
-            &BinaryExprType::DOT => self.do_dot_lookup(&def.pos, left, right, scope),
+            &BinaryExprType::DOT => panic!("Unraeachable"),
         }
     }
 
@@ -1120,12 +1031,12 @@ impl<'a> FileBuilder<'a> {
             }
         };
         let mac_sym = Value::Symbol(def.mac.clone());
-        if let &Val::Macro(ref macdef) = self.eval_value(&mac_sym, &self.scope)?.as_ref() {
+        if let &Val::Macro(ref macdef) = self.eval_value(&mac_sym, &self.scope.clone())?.as_ref() {
             let mut out = Vec::new();
             for item in l.iter() {
                 let argvals = vec![item.clone()];
                 let fields = macdef.eval(self.file.clone(), self, argvals)?;
-                if let Some(v) = Self::find_in_fieldlist(&def.field.val, &fields) {
+                if let Some(v) = find_in_fieldlist(&def.field.val, &fields) {
                     match def.typ {
                         ListOpType::Map => {
                             out.push(v.clone());

src/build/scope.rs

@@ -1,10 +1,26 @@
 use std::clone::Clone;
 use std::collections::HashMap;
+use std::convert::AsRef;
 use std::convert::Into;
+use std::error::Error;
 use std::rc::Rc;
 
+use crate::ast::Position;
 use crate::ast::PositionedItem;
 use crate::build::ir::Val;
+use crate::error;
+
+pub fn find_in_fieldlist(
+    target: &str,
+    fs: &Vec<(PositionedItem<String>, Rc<Val>)>,
+) -> Option<Rc<Val>> {
+    for (key, val) in fs.iter().cloned() {
+        if target == &key.val {
+            return Some(val.clone());
+        }
+    }
+    return None;
+}
 
 /// Defines a set of values in a parsed file.
 pub type ValueMap = HashMap<PositionedItem<String>, Rc<Val>>;
@@ -23,6 +39,7 @@ pub struct Scope {
     pub env: Rc<Val>,
     pub curr_val: Option<Rc<Val>>,
     pub build_output: ValueMap,
+    pub search_curr_val: bool,
 }
 
 impl Scope {
@@ -35,6 +52,7 @@ impl Scope {
             // (eg: Tuple, List. left side of a dot selection.)
             curr_val: None,
             build_output: HashMap::new(),
+            search_curr_val: false,
         }
     }
 
@@ -47,6 +65,7 @@ impl Scope {
             // Children start with no current val
             curr_val: None,
             build_output: self.build_output.clone(),
+            search_curr_val: false,
         }
     }
 
@@ -57,6 +76,7 @@ impl Scope {
             // Children start with no current val
             curr_val: None,
             build_output: HashMap::new(),
+            search_curr_val: false,
         }
     }
 
@@ -76,6 +96,20 @@ impl Scope {
         self.curr_val = Some(val);
     }
 
+    /// Lookup up a list index in the current value
+    pub fn lookup_idx(&self, pos: &Position, idx: &Val) -> Result<Rc<Val>, Box<Error>> {
+        if self.search_curr_val && self.curr_val.is_some() {
+            if let &Val::List(ref fs) = self.curr_val.as_ref().unwrap().as_ref() {
+                return Self::lookup_in_list(pos, idx, fs);
+            }
+        }
+        Err(Box::new(error::BuildError::new(
+            "Not a list in index lookup.",
+            error::ErrorType::TypeFail,
+            pos.clone(),
+        )))
+    }
+
     /// Lookup a symbol in the current execution context.
     ///
     /// The lookup rules are simple.
@@ -95,6 +129,64 @@ impl Scope {
         if self.build_output.contains_key(sym) {
             return Some(self.build_output[sym].clone());
         }
+        if self.search_curr_val && self.curr_val.is_some() {
+            return match self.curr_val.as_ref().unwrap().as_ref() {
+                &Val::Tuple(ref fs) => match Self::lookup_in_tuple(&sym.pos, &sym.val, fs) {
+                    Ok(v) => Some(v),
+                    Err(_) => None,
+                },
+                &Val::List(ref fs) => {
+                    match Self::lookup_in_list(&sym.pos, &Val::Str(sym.val.clone()), fs) {
+                        Ok(v) => Some(v),
+                        Err(_) => None,
+                    }
+                }
+                _ => None,
+            };
+        }
         None
     }
+
+    fn lookup_in_tuple(
+        pos: &Position,
+        field: &str,
+        fs: &Vec<(PositionedItem<String>, Rc<Val>)>,
+    ) -> Result<Rc<Val>, Box<Error>> {
+        if let Some(vv) = find_in_fieldlist(&field, fs) {
+            Ok(vv)
+        } else {
+            Err(Box::new(error::BuildError::new(
+                format!("Unable to {} match element in tuple.", field,),
+                error::ErrorType::NoSuchSymbol,
+                pos.clone(),
+            )))
+        }
+    }
+
+    fn lookup_in_list(
+        pos: &Position,
+        field: &Val,
+        elems: &Vec<Rc<Val>>,
+    ) -> Result<Rc<Val>, Box<Error>> {
+        let idx = match field {
+            &Val::Int(i) => i as usize,
+            &Val::Str(ref s) => s.parse::<usize>()?,
+            _ => {
+                return Err(Box::new(error::BuildError::new(
+                    format!("Invalid idx type {} for list lookup", field),
+                    error::ErrorType::TypeFail,
+                    pos.clone(),
+                )))
+            }
+        };
+        if idx < elems.len() {
+            Ok(elems[idx].clone())
+        } else {
+            Err(Box::new(error::BuildError::new(
+                format!("idx {} out of bounds in list", idx),
+                error::ErrorType::NoSuchSymbol,
+                pos.clone(),
+            )))
+        }
+    }
 }

src/parse/mod.rs

@@ -277,8 +277,6 @@ make_fn!(
 make_fn!(
     value<SliceIter<Token>, Value>,
     either!(
-        // TODO This should move to op_expression instead of a value now.
-        // We probably still need a bareword parser though?
         trace_parse!(symbol),
         trace_parse!(compound_value),
         trace_parse!(boolean_value),
@@ -296,7 +294,7 @@ make_fn!(
     simple_expression<SliceIter<Token>, Expression>,
     do_each!(
         val => trace_parse!(value),
-        _ => not!(either!(punct!("."), punct!("{"), punct!("["), punct!("("))),
+        _ => not!(either!(punct!("{"), punct!("["), punct!("("))),
         (value_to_expression(val))
     )
 );

src/parse/precedence.rs

@@ -15,7 +15,7 @@
 //! Bottom up parser for precedence parsing of expressions separated by binary
 //! operators.
 use abortable_parser::combinators::eoi;
-use abortable_parser::{Error, Result, SliceIter};
+use abortable_parser::{Error, Peekable, Result, SliceIter};
 
 use super::{non_op_expression, ParseResult};
 use crate::ast::*;
@@ -132,20 +132,6 @@ fn parse_sum_operator(i: SliceIter<Element>) -> Result<SliceIter<Element>, Binar
     ));
 }
 
-fn tuple_to_binary_expression(
-    kind: BinaryExprType,
-    left: Expression,
-    right: Expression,
-) -> Expression {
-    let pos = left.pos().clone();
-    Expression::Binary(BinaryOpDef {
-        kind: kind,
-        left: Box::new(left),
-        right: Box::new(right),
-        pos: pos,
-    })
-}
-
 fn parse_product_operator(i: SliceIter<Element>) -> Result<SliceIter<Element>, BinaryExprType> {
     let mut i_ = i.clone();
     if eoi(i_.clone()).is_complete() {
@@ -177,68 +163,6 @@ fn parse_product_operator(i: SliceIter<Element>) -> Result<SliceIter<Element>, B
     ));
 }
 
-/// do_binary_expr implements precedence based parsing where the more tightly bound
-/// parsers are passed in as lowerrule parsers. We default to any non_op_expression
-/// as the most tightly bound expressions.
-macro_rules! do_binary_expr {
-    ($i:expr, $oprule:ident, $lowerrule:ident) => {
-        do_binary_expr!($i, run!($oprule), $lowerrule)
-    };
-
-    ($i:expr, $oprule:ident, $lowerrule:ident!( $($lowerargs:tt)* )) => {
-        do_binary_expr!($i, run!($oprule), $lowerrule!($($lowerargs)*))
-    };
-
-    ($i:expr, $oprule:ident) => {
-        do_binary_expr!($i, run!($oprule))
-    };
-
-    ($i:expr, $oprule:ident!( $($args:tt)* )) => {
-        do_binary_expr!($i, $oprule!($($args)*), parse_expression)
-    };
-
-    ($i:expr, $oprule:ident!( $($args:tt)* ), $lowerrule:ident) => {
-        do_binary_expr!($i, $oprule!($($args)*), run!($lowerrule))
-    };
-
-    ($i:expr, $oprule:ident!( $($args:tt)* ), $lowerrule:ident!( $($lowerargs:tt)* )) => {
-        do_each!($i,
-            left => $lowerrule!($($lowerargs)*),
-                typ => $oprule!($($args)*),
-                right => $lowerrule!($($lowerargs)*),
-                (tuple_to_binary_expression(typ, left, right))
-        )
-    };
-}
-
-make_fn!(
-    sum_expression<SliceIter<Element>, Expression>,
-    do_binary_expr!(
-        parse_sum_operator,
-        either!(
-            trace_parse!(product_expression),
-            trace_parse!(dot_expression),
-            trace_parse!(parse_expression)
-        )
-    )
-);
-
-make_fn!(
-    product_expression<SliceIter<Element>, Expression>,
-    do_binary_expr!(
-        parse_product_operator,
-        either!(trace_parse!(dot_expression), trace_parse!(parse_expression))
-    )
-);
-
-make_fn!(
-    math_expression<SliceIter<Element>, Expression>,
-    either!(
-        trace_parse!(sum_expression),
-        trace_parse!(product_expression)
-    )
-);
-
 make_fn!(
     compare_op_type<SliceIter<Token>, Element>,
     either!(
@@ -284,33 +208,6 @@ fn parse_compare_operator(i: SliceIter<Element>) -> Result<SliceIter<Element>, B
     ));
 }
 
-make_fn!(
-    binary_expression<SliceIter<Element>, Expression>,
-    either!(
-        compare_expression,
-        math_expression,
-        dot_expression,
-        parse_expression
-    )
-);
-
-make_fn!(
-    dot_expression<SliceIter<Element>, Expression>,
-    do_binary_expr!(parse_dot_operator, trace_parse!(parse_expression))
-);
-
-make_fn!(
-    compare_expression<SliceIter<Element>, Expression>,
-    do_binary_expr!(
-        parse_compare_operator,
-        either!(
-            trace_parse!(math_expression),
-            trace_parse!(dot_expression),
-            trace_parse!(parse_expression)
-        )
-    )
-);
-
 /// Parse a list of expressions separated by operators into a Vec<Element>.
 fn parse_operand_list<'a>(i: SliceIter<'a, Token>) -> ParseResult<'a, Vec<Element>> {
     // 1. First try to parse a non_op_expression,
@@ -340,7 +237,6 @@ fn parse_operand_list<'a>(i: SliceIter<'a, Token>) -> ParseResult<'a, Vec<Elemen
             }
         }
         // 3. Parse an operator.
-        // TODO(jwall): Parse the dot operator.
         match either!(_i.clone(), dot_op_type, math_op_type, compare_op_type) {
             Result::Fail(e) => {
                 if firstrun {
@@ -370,8 +266,87 @@ fn parse_operand_list<'a>(i: SliceIter<'a, Token>) -> ParseResult<'a, Vec<Elemen
     return Result::Complete(_i, list);
 }
 
+make_fn!(
+    parse_operator_element<SliceIter<Element>, BinaryExprType>,
+    either!(
+        parse_dot_operator,
+        parse_sum_operator,
+        parse_product_operator,
+        parse_compare_operator
+    )
+);
+
+macro_rules! try_parse {
+    ($r:expr) => {
+        match $r {
+            Result::Abort(e) => return Result::Abort(e),
+            Result::Fail(e) => return Result::Fail(e),
+            Result::Incomplete(i) => return Result::Incomplete(i),
+            Result::Complete(rest, op_type) => (rest, op_type),
+        }
+    };
+}
+
+fn parse_op(
+    mut lhs: Expression,
+    mut i: SliceIter<Element>,
+    min_precedence: u32,
+) -> Result<SliceIter<Element>, Expression> {
+    // Termination condition
+    if eoi(i.clone()).is_complete() {
+        return Result::Complete(i, lhs);
+    }
+    let (_, mut lookahead_op) = try_parse!(parse_operator_element(i.clone()));
+    while !eoi(i.clone()).is_complete() && (lookahead_op.precedence_level() >= min_precedence) {
+        // Stash a copy of our lookahead operator for future use.
+        let op = lookahead_op.clone();
+        // Advance to next element.
+        i.next();
+        let (rest, mut rhs) = try_parse!(parse_expression(i.clone()));
+        i = rest;
+        if !eoi(i.clone()).is_complete() {
+            let (_, peek_op) = try_parse!(parse_operator_element(i.clone()));
+            lookahead_op = peek_op;
+        } else {
+        }
+        while !eoi(i.clone()).is_complete()
+            && (lookahead_op.precedence_level() > op.precedence_level())
+        {
+            let (rest, inner_rhs) =
+                try_parse!(parse_op(rhs, i.clone(), lookahead_op.precedence_level()));
+            i = rest;
+            rhs = inner_rhs;
+            // Before we check for another operator we should see
+            // if we are at the end of the input.
+            if eoi(i.clone()).is_complete() {
+                break;
+            }
+            let (_, peek_op) = try_parse!(parse_operator_element(i.clone()));
+            lookahead_op = peek_op;
+        }
+        let pos = lhs.pos().clone();
+        lhs = Expression::Binary(BinaryOpDef {
+            kind: op.clone(),
+            left: Box::new(lhs.clone()),
+            right: Box::new(rhs),
+            pos: pos,
+        });
+    }
+    return Result::Complete(i, lhs);
+}
+
+pub fn parse_precedence(i: SliceIter<Element>) -> Result<SliceIter<Element>, Expression> {
+    match parse_expression(i) {
+        Result::Abort(e) => Result::Abort(e),
+        Result::Fail(e) => Result::Fail(e),
+        Result::Incomplete(i) => Result::Incomplete(i),
+        Result::Complete(rest, expr) => parse_op(expr, rest, 0),
+    }
+}
+
 /// Parse a binary operator expression.
 pub fn op_expression<'a>(i: SliceIter<'a, Token>) -> Result<SliceIter<Token>, Expression> {
+    // TODO(jwall): We need to implement the full on precedence climbing method here.
     let preparse = parse_operand_list(i.clone());
     match preparse {
         Result::Fail(e) => {
@@ -393,11 +368,9 @@ pub fn op_expression<'a>(i: SliceIter<'a, Token>) -> Result<SliceIter<Token>, Ex
         Result::Incomplete(i) => Result::Incomplete(i),
         Result::Complete(rest, oplist) => {
             let i_ = SliceIter::new(&oplist);
-            let parse_result = binary_expression(i_);
+            let parse_result = parse_precedence(i_);
-
             match parse_result {
                 Result::Fail(_e) => {
-                    // TODO(jwall): It would be good to be able to use caused_by here.
                     let err = Error::new(
                         "Failed while parsing operator expression",
                         Box::new(rest.clone()),
@@ -406,13 +379,18 @@ pub fn op_expression<'a>(i: SliceIter<'a, Token>) -> Result<SliceIter<Token>, Ex
                 }
                 Result::Abort(_e) => {
                     let err = Error::new(
-                        "Failed while parsing operator expression",
+                        "Abort while parsing operator expression",
                         Box::new(rest.clone()),
                     );
                     Result::Abort(err)
                 }
                 Result::Incomplete(_) => Result::Incomplete(i.clone()),
-                Result::Complete(_, expr) => Result::Complete(rest.clone(), expr),
+                Result::Complete(_rest_ops, expr) => {
+                    if _rest_ops.peek_next().is_some() {
+                        panic!("premature abort parsing Operator expression!");
+                    }
+                    Result::Complete(rest.clone(), expr)
+                }
             }
         }
     }