From 988698fc01dee20d39a68435912e877cb1804e42 Mon Sep 17 00:00:00 2001
From: Jeremy Wall <jeremy@marzhillstudios.com>
Date: Sat, 26 May 2018 09:12:00 -0500
Subject: [PATCH] Refactor: Improved expression parsing organization.

Any non-operator expression can be a part of an operator expression.
This cleans up and makes clearer the precendence relationships for parsing.
---
 src/parse/mod.rs  | 68 ++++++++++++++++++++++++++---------------------
 src/parse/test.rs |  4 +--
 2 files changed, 39 insertions(+), 33 deletions(-)

diff --git a/src/parse/mod.rs b/src/parse/mod.rs
index 79d4a33..e8b5d0d 100644
--- a/src/parse/mod.rs
+++ b/src/parse/mod.rs
@@ -254,7 +254,7 @@ fn tuple_to_binary_expression(
 /// the most tightly bound expressions.
 macro_rules! do_binary_expr {
     ($i:expr, $oprule:ident!( $($args:tt)* ), $typ:expr) => {
-        do_binary_expr!($i, $oprule!($($args)*), $typ, alt!(grouped_expression | simple_expression))
+        do_binary_expr!($i, $oprule!($($args)*), $typ, non_op_expression)
     };
 
     ($i:expr, $oprule:ident!( $($args:tt)* ), $typ:expr, $lowerrule:ident) => {
@@ -275,14 +275,6 @@ macro_rules! do_binary_expr {
     };
 }
 
-named!(math_expression<TokenIter, Expression, error::Error>,
-    alt!(sum_expression | product_expression)
-);
-
-named!(sum_expression<TokenIter, Expression, error::Error>,
-    alt!(add_expression | sub_expression)
-);
-
 // trace_macros!(true);
 named!(add_expression<TokenIter, Expression, error::Error>,
        do_binary_expr!(punct!("+"), BinaryExprType::Add, alt!(product_expression | simple_expression | grouped_expression))
@@ -293,8 +285,8 @@ named!(sub_expression<TokenIter, Expression, error::Error>,
        do_binary_expr!(punct!("-"), BinaryExprType::Sub,  alt!(product_expression | simple_expression | grouped_expression))
 );
 
-named!(product_expression<TokenIter, Expression, error::Error>,
-    alt!(mul_expression | div_expression)
+named!(sum_expression<TokenIter, Expression, error::Error>,
+    alt!(add_expression | sub_expression)
 );
 
 named!(mul_expression<TokenIter, Expression, error::Error>,
@@ -305,6 +297,14 @@ named!(div_expression<TokenIter, Expression, error::Error>,
        do_binary_expr!(punct!("/"), BinaryExprType::Div)
 );
 
+named!(product_expression<TokenIter, Expression, error::Error>,
+    alt!(mul_expression | div_expression)
+);
+
+named!(math_expression<TokenIter, Expression, error::Error>,
+    alt!(sum_expression | product_expression)
+);
+
 // TODO(jwall): Change comparison operators to use the do_binary_expr! with precedence?
 fn tuple_to_compare_expression(
     tpl: (Position, CompareType, Expression, Expression),
@@ -356,6 +356,20 @@ named!(lt_expression<TokenIter, Expression, error::Error>,
        do_compare_expr!(punct!("<"), CompareType::LT)
 );
 
+named!(compare_expression<TokenIter, Expression, error::Error>,
+    alt!(
+        eqeq_expression |
+        not_eqeq_expression |
+        lt_eqeq_expression |
+        gt_eqeq_expression |
+        lt_expression |
+        gt_expression)
+);
+
+named!(op_expression<TokenIter, Expression, error::Error>,
+    alt!(math_expression | compare_expression)
+);
+
 fn expression_to_grouped_expression(e: Expression) -> ParseResult<Expression> {
     Ok(Expression::Grouped(Box::new(e)))
 }
@@ -704,6 +718,17 @@ named!(list_op_expression<TokenIter, Expression, error::Error>,
     )
 );
 
+named!(non_op_expression<TokenIter, Expression, error::Error>,
+    alt!(list_op_expression |
+         macro_expression |
+         format_expression |
+         select_expression |
+         call_expression |
+         copy_expression |
+         grouped_expression |
+         simple_expression)
+);
+
 // NOTE(jwall): HERE THERE BE DRAGONS. The order for these matters
 // a lot. We need to process alternatives in order of decreasing
 // specificity.  Unfortunately this means we are required to go in a
@@ -715,26 +740,7 @@ named!(list_op_expression<TokenIter, Expression, error::Error>,
 // It also means this combinator is risky when used with partial
 // inputs. So handle with care.
 named!(expression<TokenIter, Expression, error::Error>,
-    do_parse!(
-        expr: alt!(
-           complete!(list_op_expression) |
-           complete!(math_expression) |
-           complete!(eqeq_expression) |
-           complete!(not_eqeq_expression) |
-           complete!(lt_eqeq_expression) |
-           complete!(gt_eqeq_expression) |
-           complete!(gt_expression) |
-           complete!(lt_expression) |
-           complete!(macro_expression) |
-           complete!(format_expression) |
-           complete!(select_expression) |
-           complete!(call_expression) |
-           complete!(copy_expression) |
-           complete!(grouped_expression) |
-           simple_expression
-       ) >>
-       (expr)
-    )
+    alt!(complete!(op_expression) | complete!(non_op_expression))
 );
 
 fn expression_to_statement(v: Expression) -> ParseResult<Statement> {
diff --git a/src/parse/test.rs b/src/parse/test.rs
index 6a99fc2..84797a3 100644
--- a/src/parse/test.rs
+++ b/src/parse/test.rs
@@ -363,7 +363,7 @@ fn test_expression_parse() {
                                                              1, 1)))
     );
     assert_parse!(
-        math_expression("1 + 1"),
+        expression("1 + 1"),
         Expression::Binary(BinaryOpDef {
             kind: BinaryExprType::Add,
             left: Box::new(Expression::Simple(Value::Int(value_node!(1, 1, 1)))),
@@ -381,7 +381,7 @@ fn test_expression_parse() {
         })
     );
     assert_parse!(
-        expression("1 * 1"),
+        mul_expression("1 * 1"),
         Expression::Binary(BinaryOpDef {
             kind: BinaryExprType::Mul,
             left: Box::new(Expression::Simple(Value::Int(value_node!(1, 1, 1)))),