Fixes to the value expression parser and evaluator

6 files changed, 81 insertions, 74 deletions

diff --git a/src/format.cc b/src/format.cc
index d949c350..b93a42a4 100644
--- a/src/format.cc
+++ b/src/format.cc
@@ -263,12 +263,15 @@ format_t::element_t * format_t::parse_elements(const string& fmt,
 	args3_node->set_left(call1_node);
 	args3_node->set_right(args2_node);
 
+	expr_t::ptr_op_t seq1_node(new expr_t::op_t(expr_t::op_t::O_SEQ));
+	seq1_node->set_left(args3_node);
+
 	expr_t::ptr_op_t justify_node(new expr_t::op_t(expr_t::op_t::IDENT));
 	justify_node->set_ident("justify");
 
 	expr_t::ptr_op_t call2_node(new expr_t::op_t(expr_t::op_t::O_CALL));
 	call2_node->set_left(justify_node);
-	call2_node->set_right(args3_node);
+	call2_node->set_right(seq1_node);
 
 	string prev_expr = boost::get<expr_t>(current->data).text();
 
@@ -280,9 +283,12 @@ format_t::element_t * format_t::parse_elements(const string& fmt,
 	  args4_node->set_left(call2_node);
 	  args4_node->set_right(colorize_op);
 
+	  expr_t::ptr_op_t seq2_node(new expr_t::op_t(expr_t::op_t::O_SEQ));
+	  seq2_node->set_left(args4_node);
+
 	  expr_t::ptr_op_t call3_node(new expr_t::op_t(expr_t::op_t::O_CALL));
 	  call3_node->set_left(ansify_if_node);
-	  call3_node->set_right(args4_node);
+	  call3_node->set_right(seq2_node);
 
 	  current->data = expr_t(call3_node);
 	} else {
diff --git a/src/op.cc b/src/op.cc
index c4925143..2815ac1a 100644
--- a/src/op.cc
+++ b/src/op.cc
@@ -118,7 +118,8 @@ value_t expr_t::op_t::calc(scope_t& scope, ptr_op_t * locus, const int depth)
     // directly, so we create an empty call_scope_t to reflect the scope for
     // this implicit call.
     call_scope_t call_args(scope);
-    result = left()->calc(call_args, locus, depth + 1);
+    result = left()->compile(call_args, depth + 1)
+                   ->calc(call_args, locus, depth + 1);
     break;
   }
 
@@ -135,7 +136,6 @@ value_t expr_t::op_t::calc(scope_t& scope, ptr_op_t * locus, const int depth)
   }
 
   case O_DEFINE: {
-    symbol_scope_t local_scope;
     call_scope_t&  call_args(downcast<call_scope_t>(scope));
     std::size_t	   args_count = call_args.size();
     std::size_t	   args_index = 0;
@@ -152,38 +152,32 @@ value_t expr_t::op_t::calc(scope_t& scope, ptr_op_t * locus, const int depth)
       if (! varname->is_ident())
 	throw_(calc_error, _("Invalid function definition"));
       else if (args_index == args_count)
-	local_scope.define(symbol_t::FUNCTION, varname->as_ident(),
-			   wrap_value(false));
+	scope.define(symbol_t::FUNCTION, varname->as_ident(),
+		     wrap_value(false));
       else
-	local_scope.define(symbol_t::FUNCTION, varname->as_ident(),
-			   wrap_value(call_args[args_index++]));
+	scope.define(symbol_t::FUNCTION, varname->as_ident(),
+		     wrap_value(call_args[args_index++]));
     }
 
     if (args_index < args_count)
       throw_(calc_error,
 	     _("Too many arguments in function call (saw %1)") << args_count);
 
-    result = right()->compile(local_scope, depth + 1)
-                    ->calc(local_scope, locus, depth + 1);
+    result = right()->calc(scope, locus, depth + 1);
     break;
   }
 
   case O_LOOKUP:
-    if (left()->is_ident() &&
-	left()->left() && left()->left()->is_function()) {
-      call_scope_t call_args(scope);
-      if (value_t obj = left()->left()->as_function()(call_args)) {
-	if (obj.is_scope()) {
-	  if (obj.as_scope() == NULL) {
-	    throw_(calc_error,
-		   _("Left operand of . operator is NULL"));
-	  } else {
-	    scope_t& objscope(*obj.as_scope());
-	    if (ptr_op_t member =
-		objscope.lookup(symbol_t::FUNCTION, right()->as_ident())) {
-	      result = member->calc(objscope, NULL, depth + 1);
-	      break;
-	    }
+    if (value_t obj = left()->calc(scope, locus, depth + 1)) {
+      if (obj.is_scope()) {
+	if (obj.as_scope() == NULL) {
+	  throw_(calc_error, _("Left operand of . operator is NULL"));
+	} else {
+	  scope_t& objscope(*obj.as_scope());
+	  if (ptr_op_t member =
+	      objscope.lookup(symbol_t::FUNCTION, right()->as_ident())) {
+	    result = member->calc(objscope, NULL, depth + 1);
+	    break;
 	  }
 	}
       }
@@ -321,20 +315,28 @@ value_t expr_t::op_t::calc(scope_t& scope, ptr_op_t * locus, const int depth)
     break;
 
   case O_SEQ: {
-    left()->calc(scope, locus, depth + 1);
-    assert(has_right());
-
-    ptr_op_t next = right();
-    while (next) {
-      ptr_op_t value_op;
-      if (next->kind == O_SEQ) {
-	value_op = next->left();
-	next     = next->right();
-      } else {
-	value_op = next;
-	next     = NULL;
+    symbol_scope_t seq_scope(scope);
+
+    // An O_SEQ is very similar to an O_CONS except that only the last result
+    // value in the series is kept.  O_CONS builds up a list.
+    //
+    // Another feature of O_SEQ is that it pushes a new symbol scope onto the
+    // stack.
+    result = left()->calc(seq_scope, locus, depth + 1);
+
+    if (has_right()) {
+      ptr_op_t next = right();
+      while (next) {
+	ptr_op_t value_op;
+	if (next->kind == O_SEQ) {
+	  value_op = next->left();
+	  next     = next->right();
+	} else {
+	  value_op = next;
+	  next     = NULL;
+	}
+	result = value_op->calc(seq_scope, locus, depth + 1);
       }
-      result = value_op->calc(scope, locus, depth + 1);
     }
     break;
   }
@@ -394,13 +396,14 @@ namespace {
     if (op->left()->print(out, context))
       found = true;
 
-    assert(op->has_right());
-    out << "; ";
+    if (op->has_right()) {
+      out << "; ";
 
-    if (op->right()->kind == expr_t::op_t::O_CONS)
-      found = print_cons(out, op->right(), context);
-    else if (op->right()->print(out, context))
-      found = true;
+      if (op->right()->kind == expr_t::op_t::O_CONS)
+	found = print_cons(out, op->right(), context);
+      else if (op->right()->print(out, context))
+	found = true;
+    }
 
     return found;
   }
@@ -563,9 +566,7 @@ bool expr_t::op_t::print(std::ostream& out, const context_t& context) const
     break;
 
   case O_CONS:
-    out << "(";
     found = print_cons(out, this, context);
-    out << ")";
     break;
 
   case O_SEQ:
@@ -594,7 +595,7 @@ bool expr_t::op_t::print(std::ostream& out, const context_t& context) const
     if (left() && left()->print(out, context))
       found = true;
     if (has_right()) {
-      if (right()->kind == O_CONS) {
+      if (right()->kind == O_SEQ) {
 	if (right()->print(out, context))
 	  found = true;
       } else {
diff --git a/src/op.h b/src/op.h
index 48d167b7..347eac1d 100644
--- a/src/op.h
+++ b/src/op.h
@@ -243,9 +243,6 @@ private:
     op->release();
   }
 
-  static ptr_op_t new_node(kind_t _kind, ptr_op_t _left = NULL,
-			   ptr_op_t _right = NULL);
-
   ptr_op_t copy(ptr_op_t _left = NULL, ptr_op_t _right = NULL) const {
     ptr_op_t node(new_node(kind, _left, _right));
     if (kind < TERMINALS)
@@ -254,6 +251,9 @@ private:
   }
 
 public:
+  static ptr_op_t new_node(kind_t _kind, ptr_op_t _left = NULL,
+			   ptr_op_t _right = NULL);
+
   ptr_op_t compile(scope_t& scope, const int depth = 0);
   value_t  calc(scope_t& scope, ptr_op_t * locus = NULL,
 		const int depth = 0);
diff --git a/src/parser.cc b/src/parser.cc
index d94cf37f..ef778411 100644
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -82,7 +82,7 @@ expr_t::parser_t::parse_value_term(std::istream&        in,
 
     if (node->kind == op_t::O_CONS) {
       ptr_op_t prev(node);
-      node = new op_t(op_t::O_CONS);
+      node = new op_t(op_t::O_SEQ);
       node->set_left(prev);
     }
     break;
diff --git a/src/post.cc b/src/post.cc
index a1ed8090..fbbf26de 100644
--- a/src/post.cc
+++ b/src/post.cc
@@ -226,29 +226,30 @@ namespace {
 				    2 /* account_abbrev_length */);
 	else
 	  name = env->reported_account()->fullname();
-      }
-      else if (env.value_at(0).is_string()) {
-	name    = env.get<string>(0);
-	account = env->xact->journal->find_account(name, false);
-	seeking_account = true;
-      }
-      else if (env.value_at(0).is_mask()) {
-	name    = env.get<mask_t>(0).str();
-	account = env->xact->journal->find_account_re(name);
-	seeking_account = true;
-      }
-      else {
-	throw_(std::runtime_error,
-	       _("Expected string or mask for argument 1, but received %1")
-	       << env.value_at(0).label());
-      }
+      } else {
+	account_t * master = env->account;
+	while (master->parent)
+	  master = master->parent;
+
+	if (env.value_at(0).is_string()) {
+	  name    = env.get<string>(0);
+	  account = master->find_account(name, false);
+	}
+	else if (env.value_at(0).is_mask()) {
+	  name    = env.get<mask_t>(0).str();
+	  account = master->find_account_re(name);
+	}
+	else {
+	  throw_(std::runtime_error,
+		 _("Expected string or mask for argument 1, but received %1")
+		 << env.value_at(0).label());
+	}
 
-      if (seeking_account) {
 	if (! account)
 	  throw_(std::runtime_error,
 		 _("Could not find an account matching ") << env.value_at(0));
 	else
-	  return account;	// return a scope object
+	  return value_t(static_cast<scope_t *>(account));
       }
     } else {
       name = env->reported_account()->fullname();
diff --git a/src/query.cc b/src/query.cc
index 98a3de1d..e48e65b5 100644
--- a/src/query.cc
+++ b/src/query.cc
@@ -307,15 +307,14 @@ query_t::parser_t::parse_query_term(query_t::lexer_t::token_t::kind_t tok_contex
 	  throw_(parse_error,
 		 _("Metadata equality operator not followed by term"));
 	
-	expr_t::ptr_op_t cons = new expr_t::op_t(expr_t::op_t::O_CONS);
-
 	expr_t::ptr_op_t arg2 = new expr_t::op_t(expr_t::op_t::VALUE);
 	assert(tok.value);
 	arg2->set_value(mask_t(*tok.value));
 
-	cons->set_left(arg1);
-	cons->set_right(arg2);
-	node->set_right(cons);
+	node->set_right(expr_t::op_t::new_node
+			(expr_t::op_t::O_SEQ,
+			 expr_t::op_t::new_node
+			 (expr_t::op_t::O_CONS, arg1, arg2)));
       } else {
 	node->set_right(arg1);
       }