Reduce Number of Expression Parentheses (#1289)

Adds binary operation precedence support to avoid unnecessary
parentheses in expression printed format

- #962

---------

Co-authored-by: chriseth <[email protected]>

analysis/README.md

@@ -200,7 +200,7 @@ The diff for our example program is as follows:
 +               _output_0 = ((((read__output_0_pc * pc) + (read__output_0__input_0 * _input_0)) + _output_0_const) + (_output_0_read_free * _output_0_free_value));
 +               pol constant first_step = [1] + [0]*;
 +               ((1 - instr__reset) * _input_0') = ((1 - instr__reset) * _input_0);
-+               pc' = ((1 - first_step') * ((((instr__jump_to_operation * _operation_id) + (instr__loop * pc)) + (instr_return * 0)) + ((1 - ((instr__jump_to_operation + instr__loop) + instr_return)) * (pc + 1))));
++               pc' = (1 - first_step') * (instr__jump_to_operation * _operation_id + instr__loop * pc + instr_return * 0 + (1 - (instr__jump_to_operation + instr__loop + instr_return)) * (pc + 1));
 +               pol constant p_line = [0, 1, 2, 3, 4, 5] + [5]*;
 +               pol commit _output_0_free_value;
 +               pol constant p__output_0_const = [0, 0, 0, 0, 1, 0] + [0]*;

ast/src/parsed/display.rs

@@ -329,15 +329,23 @@ impl<T: Display> Display for Params<T> {
     }
 }
 
-impl<E: Display> Display for IndexAccess<E> {
+impl<E: Display> Display for IndexAccess<Expression<E>> {
     fn fmt(&self, f: &mut Formatter<'_>) -> Result {
-        write!(f, "{}[{}]", self.array, self.index)
+        if self.array.precedence().is_none() {
+            write!(f, "{}[{}]", self.array, self.index)
+        } else {
+            write!(f, "({})[{}]", self.array, self.index)
+        }
     }
 }
 
-impl<E: Display> Display for FunctionCall<E> {
+impl<E: Display> Display for FunctionCall<Expression<E>> {
     fn fmt(&self, f: &mut Formatter<'_>) -> Result {
-        write!(f, "{}({})", self.function, format_list(&self.arguments))
+        if self.function.precedence().is_none() {
+            write!(f, "{}({})", self.function, format_list(&self.arguments))
+        } else {
+            write!(f, "({})({})", self.function, format_list(&self.arguments))
+        }
     }
 }
 
@@ -587,6 +595,80 @@ fn format_list<L: IntoIterator<Item = I>, I: Display>(list: L) -> String {
     format!("{}", list.into_iter().format(", "))
 }
 
+impl<E: Display> Expression<E> {
+    pub fn precedence(&self) -> Option<ExpressionPrecedence> {
+        match self {
+            Expression::UnaryOperation(op, _) => Some(op.precedence()),
+            Expression::BinaryOperation(_, op, _) => Some(op.precedence()),
+            _ => None,
+        }
+    }
+
+    pub fn format_unary_operation(
+        &self,
+        op: &UnaryOperator,
+        exp: &Expression<E>,
+        f: &mut Formatter<'_>,
+    ) -> Result {
+        let exp_string = match (self.precedence(), exp.precedence()) {
+            (Some(precedence), Some(inner_precedence)) if precedence < inner_precedence => {
+                format!("({exp})")
+            }
+            _ => {
+                format!("{exp}")
+            }
+        };
+
+        if op.is_prefix() {
+            write!(f, "{op}{exp_string}")
+        } else {
+            write!(f, "{exp_string}{op}")
+        }
+    }
+
+    pub fn format_binary_operation(
+        left: &Expression<E>,
+        op: &BinaryOperator,
+        right: &Expression<E>,
+        f: &mut Formatter<'_>,
+    ) -> Result {
+        let force_parentheses = matches!(op, BinaryOperator::Pow);
+
+        let use_left_parentheses = match left.precedence() {
+            Some(left_precedence) => {
+                force_parentheses
+                    || left_precedence > op.precedence()
+                    || (left_precedence == op.precedence()
+                        && op.associativity() != BinaryOperatorAssociativity::Left)
+            }
+            None => false,
+        };
+
+        let use_right_parentheses = match right.precedence() {
+            Some(right_precedence) => {
+                force_parentheses
+                    || right_precedence > op.precedence()
+                    || (right_precedence == op.precedence()
+                        && op.associativity() != BinaryOperatorAssociativity::Right)
+            }
+            None => false,
+        };
+
+        let left_string = if use_left_parentheses {
+            format!("({left})")
+        } else {
+            format!("{left}")
+        };
+        let right_string = if use_right_parentheses {
+            format!("({right})")
+        } else {
+            format!("{right}")
+        };
+
+        write!(f, "{left_string} {op} {right_string}")
+    }
+}
+
 impl<Ref: Display> Display for Expression<Ref> {
     fn fmt(&self, f: &mut Formatter<'_>) -> Result {
         match self {
@@ -597,14 +679,10 @@ impl<Ref: Display> Display for Expression<Ref> {
             Expression::Tuple(items) => write!(f, "({})", format_list(items)),
             Expression::LambdaExpression(lambda) => write!(f, "{lambda}"),
             Expression::ArrayLiteral(array) => write!(f, "{array}"),
-            Expression::BinaryOperation(left, op, right) => write!(f, "({left} {op} {right})"),
-            Expression::UnaryOperation(op, exp) => {
-                if op.is_prefix() {
-                    write!(f, "{op}{exp}")
-                } else {
-                    write!(f, "{exp}{op}")
-                }
+            Expression::BinaryOperation(left, op, right) => {
+                Expression::format_binary_operation(left, op, right, f)
             }
+            Expression::UnaryOperation(op, exp) => self.format_unary_operation(op, exp, f),
             Expression::IndexAccess(index_access) => write!(f, "{index_access}"),
             Expression::FunctionCall(fun_call) => write!(f, "{fun_call}"),
             Expression::FreeInput(input) => write!(f, "${{ {input} }}"),

ast/src/parsed/mod.rs

@@ -352,6 +352,8 @@ pub enum Expression<Ref = NamespacedPolynomialReference> {
     BlockExpression(Vec<StatementInsideBlock<Self>>, Box<Self>),
 }
 
+pub type ExpressionPrecedence = u64;
+
 impl<Ref> Expression<Ref> {
     pub fn new_binary(left: Self, op: BinaryOperator, right: Self) -> Self {
         Expression::BinaryOperation(Box::new(left), op, Box::new(right))
@@ -638,6 +640,76 @@ pub enum BinaryOperator {
     Greater,
 }
 
+#[derive(Debug, PartialEq, Eq)]
+pub enum BinaryOperatorAssociativity {
+    Left,
+    Right,
+    RequireParentheses,
+}
+
+trait Precedence {
+    fn precedence(&self) -> ExpressionPrecedence;
+}
+
+impl Precedence for UnaryOperator {
+    fn precedence(&self) -> ExpressionPrecedence {
+        use UnaryOperator::*;
+        match self {
+            // NOTE: Any modification must be done with care to not overlap with BinaryOperator's precedence
+            Next => 1,
+            Minus | LogicalNot => 2,
+        }
+    }
+}
+
+impl Precedence for BinaryOperator {
+    fn precedence(&self) -> ExpressionPrecedence {
+        use BinaryOperator::*;
+        match self {
+            // NOTE: Any modification must be done with care to not overlap with LambdaExpression's precedence
+            // Unary Oprators
+            // **
+            Pow => 3,
+            // * / %
+            Mul | Div | Mod => 4,
+            // + -
+            Add | Sub => 5,
+            // << >>
+            ShiftLeft | ShiftRight => 6,
+            // &
+            BinaryAnd => 7,
+            // ^
+            BinaryXor => 8,
+            // |
+            BinaryOr => 9,
+            // = == != < > <= >=
+            Identity | Equal | NotEqual | Less | Greater | LessEqual | GreaterEqual => 10,
+            // &&
+            LogicalAnd => 11,
+            // ||
+            LogicalOr => 12,
+            // .. ..=
+            // ??
+        }
+    }
+}
+
+impl BinaryOperator {
+    pub fn associativity(&self) -> BinaryOperatorAssociativity {
+        use BinaryOperator::*;
+        use BinaryOperatorAssociativity::*;
+        match self {
+            Identity | Equal | NotEqual | Less | Greater | LessEqual | GreaterEqual => {
+                RequireParentheses
+            }
+            Pow => Right,
+
+            // .. ..= => RequireParentheses,
+            _ => Left,
+        }
+    }
+}
+
 #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Serialize, Deserialize, JsonSchema)]
 pub struct IndexAccess<E = Expression<NamespacedPolynomialReference>> {
     pub array: Box<E>,