From 0c53110714fe21422668d6c15cf60464fdb98629 Mon Sep 17 00:00:00 2001
From: Markus Westerlind <marwes91@gmail.com>
Date: Tue, 18 Oct 2016 23:40:15 +0200
Subject: [PATCH] feat: Visit and reparse all nested infix expressions

This reparses a few more infix expressions than necessary (`1 + 2 + 3` will reparse `1 + 2 + 3` and `2 + 3`) which is some wasted work but since reparsing is idempotent this is still correct.
---
 parser/src/infix.rs | 60 +++++++++++++++++++++++++++++++++++++++++++--
 parser/src/lib.rs   | 38 ++++++++++++++--------------
 2 files changed, 78 insertions(+), 20 deletions(-)

diff --git a/parser/src/infix.rs b/parser/src/infix.rs
index 8f84832d9c..f3101727f5 100644
--- a/parser/src/infix.rs
+++ b/parser/src/infix.rs
@@ -2,11 +2,15 @@
 //! associative with the same precedence. Therefore we need to rebalance them
 //! after the fact.
 
-use base::ast::{Expr, IdentEnv, SpannedExpr, TypedIdent};
+use base::ast::{Expr, IdentEnv, Literal, MutVisitor, SpannedExpr, TypedIdent, walk_mut_expr};
+use base::error::Errors;
 use base::fnv::FnvMap;
+use base::pos::{BytePos, spanned2};
 use std::cmp::Ordering;
 use std::error::Error;
 use std::fmt;
+use std::marker::PhantomData;
+use std::mem;
 use std::ops::Index;
 
 /// The fixity (associativity) of an infix operator
@@ -122,6 +126,53 @@ impl<'a> Index<&'a str> for OpTable {
     }
 }
 
+pub struct Reparser<'s, Id: 's> {
+    operators: OpTable,
+    symbols: &'s IdentEnv<Ident = Id>,
+    errors: Errors<ReparseError>,
+    _marker: PhantomData<Id>,
+}
+
+impl<'s, Id> Reparser<'s, Id> {
+    pub fn new(operators: OpTable, symbols: &'s IdentEnv<Ident = Id>) -> Reparser<'s, Id> {
+        Reparser {
+            operators: operators,
+            symbols: symbols,
+            errors: Errors::new(),
+            _marker: PhantomData,
+        }
+    }
+
+    pub fn reparse(&mut self, expr: &mut SpannedExpr<Id>) -> Result<(), Errors<ReparseError>> {
+        self.visit_expr(expr);
+        if self.errors.has_errors() {
+            Err(mem::replace(&mut self.errors, Errors::new()))
+        } else {
+            Ok(())
+        }
+    }
+}
+
+impl<'s, Id> MutVisitor for Reparser<'s, Id> {
+    type Ident = Id;
+
+    fn visit_expr(&mut self, e: &mut SpannedExpr<Self::Ident>) {
+        if let Expr::Infix(..) = e.value {
+            let dummy = spanned2(BytePos::from(0),
+                                 BytePos::from(0),
+                                 Expr::Literal(Literal::Int(0)));
+            let expr = mem::replace(e, dummy);
+            match reparse(expr, self.symbols, &self.operators) {
+                Ok(expr) => {
+                    *e = expr;
+                }
+                Err(err) => self.errors.error(err),
+            }
+        }
+        walk_mut_expr(self, e);
+    }
+}
+
 #[derive(Clone, Debug, PartialEq)]
 pub enum ReparseError {
     ConflictingFixities((String, OpMeta), (String, OpMeta)),
@@ -134,7 +185,12 @@ impl fmt::Display for ReparseError {
         match *self {
             ConflictingFixities((ref lhs_name, lhs_meta), (ref rhs_name, rhs_meta)) => {
                 try!(write!(f, "Conflicting fixities at the same precedence level. "));
-                write!(f, "left: `{} {}`, right: `{} {}`", lhs_meta, lhs_name, rhs_meta, rhs_name)
+                write!(f,
+                       "left: `{} {}`, right: `{} {}`",
+                       lhs_meta,
+                       lhs_name,
+                       rhs_meta,
+                       rhs_name)
             }
         }
     }
diff --git a/parser/src/lib.rs b/parser/src/lib.rs
index 4a0fe2319e..cb97d9989c 100644
--- a/parser/src/lib.rs
+++ b/parser/src/lib.rs
@@ -9,6 +9,7 @@ extern crate combine;
 extern crate combine_language;
 
 use std::fmt;
+use std::marker::PhantomData;
 
 use base::ast::{IdentEnv, SpannedExpr};
 use base::error::Errors;
@@ -18,7 +19,7 @@ use base::types::ArcType;
 
 use combine::primitives::{StreamOnce, Error as CombineError};
 
-use infix::OpTable;
+use infix::{OpTable, Reparser};
 use lexer::Lexer;
 
 mod grammar;
@@ -69,36 +70,37 @@ pub enum FieldExpr<Id> {
 // Hack around LALRPOP's limited type syntax
 type MutIdentEnv<'env, Id> = &'env mut IdentEnv<Ident = Id>;
 
-pub fn parse_expr(symbols: &mut IdentEnv<Ident = Symbol>,
-                  input: &str)
-                  -> Result<SpannedExpr<Symbol>, Error> {
+fn parse_expr_<Id>(symbols: &mut IdentEnv<Ident = Id>,
+                   input: &str)
+                   -> Result<SpannedExpr<Id>, Error>
+    where Id: Clone,
+{
     let lexer = Lexer::new(input);
-    let operators = OpTable::default();
 
     match grammar::parse_TopExpr(input, symbols, lexer) {
         // TODO: handle errors
-        Ok(expr) => Ok(infix::reparse(expr, symbols, &operators).unwrap()),
+        Ok(mut expr) => {
+            Reparser::new(OpTable::default(), symbols).reparse(&mut expr).unwrap();
+            Ok(expr)
+        }
         Err(err) => {
-            let err = ParseError { errors: vec![CombineError::Message(format!("{:?}", err).into())] };
+            let err =
+                ParseError { errors: vec![CombineError::Message(format!("{:?}", err).into())] };
             Err(Errors { errors: vec![pos::spanned2(BytePos::from(0), BytePos::from(0), err)] })
         }
     }
 }
 
+pub fn parse_expr(symbols: &mut IdentEnv<Ident = Symbol>,
+                  input: &str)
+                  -> Result<SpannedExpr<Symbol>, Error> {
+    parse_expr_(symbols, input)
+}
+
 #[cfg(feature = "test")]
 pub fn parse_string<'env, 'input>
     (symbols: &'env mut IdentEnv<Ident = String>,
      input: &'input str)
      -> Result<SpannedExpr<String>, (Option<SpannedExpr<String>>, Error)> {
-    let lexer = Lexer::new(input);
-    let operators = OpTable::default();
-
-    match grammar::parse_TopExpr(input, symbols, lexer) {
-        // TODO: handle errors
-        Ok(expr) => Ok(infix::reparse(expr, symbols, &operators).unwrap()),
-        Err(err) => {
-            let err = ParseError { errors: vec![CombineError::Message(format!("{:?}", err).into())] };
-            Err((None, Errors { errors: vec![pos::spanned2(BytePos::from(0), BytePos::from(0), err)] }))
-        }
-    }
+    parse_expr_(symbols, input).map_err(|err| (None, err))
 }