Address feedback

compiler/ast/src/statements/assign/mod.rs

@@ -55,6 +55,28 @@ pub enum AssignOperation {
     // Mod,
 }
 
+impl AssignOperation {
+    pub fn into_binary_operation(assign_op: AssignOperation) -> Option<BinaryOperation> {
+        match assign_op {
+            AssignOperation::Assign => None,
+            AssignOperation::Add => Some(BinaryOperation::Add),
+            AssignOperation::Sub => Some(BinaryOperation::Sub),
+            AssignOperation::Mul => Some(BinaryOperation::Mul),
+            AssignOperation::Div => Some(BinaryOperation::Div),
+            AssignOperation::Pow => Some(BinaryOperation::Pow),
+            AssignOperation::Or => Some(BinaryOperation::Or),
+            AssignOperation::And => Some(BinaryOperation::And),
+            AssignOperation::BitOr => Some(BinaryOperation::BitwiseOr),
+            AssignOperation::BitAnd => Some(BinaryOperation::BitwiseAnd),
+            AssignOperation::BitXor => Some(BinaryOperation::Xor),
+            AssignOperation::Shr => Some(BinaryOperation::Shr),
+            // AssignOperation::ShrSigned => Some(BinaryOperation::ShrSigned),
+            AssignOperation::Shl => Some(BinaryOperation::Shl),
+            // AssignOperation::Mod => Some(BinaryOperation::Mod),
+        }
+    }
+}
+
 impl AsRef<str> for AssignOperation {
     fn as_ref(&self) -> &'static str {
         match self {
@@ -77,28 +99,6 @@ impl AsRef<str> for AssignOperation {
     }
 }
 
-impl From<AssignOperation> for Option<BinaryOperation> {
-    fn from(assign_op: AssignOperation) -> Self {
-        match assign_op {
-            AssignOperation::Assign => None,
-            AssignOperation::Add => Some(BinaryOperation::Add),
-            AssignOperation::Sub => Some(BinaryOperation::Sub),
-            AssignOperation::Mul => Some(BinaryOperation::Mul),
-            AssignOperation::Div => Some(BinaryOperation::Div),
-            AssignOperation::Pow => Some(BinaryOperation::Pow),
-            AssignOperation::Or => Some(BinaryOperation::Or),
-            AssignOperation::And => Some(BinaryOperation::And),
-            AssignOperation::BitOr => Some(BinaryOperation::BitwiseOr),
-            AssignOperation::BitAnd => Some(BinaryOperation::BitwiseAnd),
-            AssignOperation::BitXor => Some(BinaryOperation::Xor),
-            AssignOperation::Shr => Some(BinaryOperation::Shr),
-            // AssignOperation::ShrSigned => Some(BinaryOperation::ShrSigned),
-            AssignOperation::Shl => Some(BinaryOperation::Shl),
-            // AssignOperation::Mod => Some(BinaryOperation::Mod),
-        }
-    }
-}
-
 /// An assignment statement, `assignee operation? = value`.
 #[derive(Clone, PartialEq, Eq, Serialize, Deserialize, Debug)]
 pub struct AssignStatement {

compiler/passes/src/static_single_assignment/mod.rs

@@ -14,7 +14,7 @@
 // You should have received a copy of the GNU General Public License
 // along with the Leo library. If not, see <https://www.gnu.org/licenses/>.
 
-//! The Static Single Assignment pass traversed the AST and converts it into SSA form.
+//! The Static Single Assignment pass traverses the AST and converts it into SSA form.
 //! See https://en.wikipedia.org/wiki/Static_single-assignment_form for more information.
 //! The pass also flattens `ConditionalStatement`s into a sequence of `AssignStatement`s.
 //! The pass also rewrites complex `AssignStatement`s into simpler ones. e.g x += 1 -> x = x + 1.

compiler/passes/src/static_single_assignment/rename_expression.rs

@@ -51,14 +51,14 @@ impl ExpressionReconstructor for StaticSingleAssigner<'_> {
                     .into_iter()
                     .map(|arg| CircuitVariableInitializer {
                         identifier: arg.identifier,
-                        expression: match &arg.expression.is_some() {
+                        expression: Some(match &arg.expression.is_some() {
                             // If the expression is None, then `arg` is a `CircuitVariableInitializer` of the form `<id>,`.
                             // In this case, we must reconstruct the identifier and produce an initializer of the form `<id>: <renamed_id>`.
-                            false => Some(self.reconstruct_identifier(arg.identifier).0),
+                            false => self.reconstruct_identifier(arg.identifier).0,
                             // If expression is `Some(..)`, then `arg is a `CircuitVariableInitializer` of the form `<id>: <expr>,`.
                             // In this case, we must reconstruct the expression.
-                            true => Some(self.reconstruct_expression(arg.expression.unwrap()).0),
-                        },
+                            true => self.reconstruct_expression(arg.expression.unwrap()).0,
+                        }),
                     })
                     .collect(),
             }),

compiler/passes/src/static_single_assignment/rename_statement.rs

@@ -85,16 +85,13 @@ impl StatementReconstructor for StaticSingleAssigner<'_> {
 
         let value = match assign.operation {
             AssignOperation::Assign => self.reconstruct_expression(assign.value).0,
-            _ => {
-                // Note that all `AssignOperation`s except for the `Assign` variant have an equivalent `BinaryOperation`.
-                let bin_op: Option<BinaryOperation> = assign.operation.into();
-                Expression::Binary(BinaryExpression {
-                    left: Box::new(place.clone()),
-                    right: Box::new(self.reconstruct_expression(assign.value).0),
-                    op: bin_op.unwrap(),
-                    span: assign.span,
-                })
-            }
+            // Note that all `AssignOperation`s except for the `Assign` variant have an equivalent `BinaryOperation`.
+            _ => Expression::Binary(BinaryExpression {
+                left: Box::new(place.clone()),
+                right: Box::new(self.reconstruct_expression(assign.value).0),
+                op: AssignOperation::into_binary_operation(assign.operation).unwrap(),
+                span: assign.span,
+            }),
         };
 
         Self::simple_assign_statement(place, value)
@@ -111,7 +108,7 @@ impl StatementReconstructor for StaticSingleAssigner<'_> {
         self.condition_stack.push(condition.clone());
 
         // Reconstruct the then-block.
-        let block = self.reconstruct_block(conditional.then);
+        let then = self.reconstruct_block(conditional.then);
 
         // Remove condition from the condition stack.
         self.condition_stack.pop();
@@ -123,7 +120,7 @@ impl StatementReconstructor for StaticSingleAssigner<'_> {
         self.push();
 
         // Reconstruct the otherwise-block.
-        let next = conditional.otherwise.map(|statement| {
+        let otherwise = conditional.otherwise.map(|statement| {
             // Add the negated condition to the condition stack.
             self.condition_stack.push(Expression::Unary(UnaryExpression {
                 op: UnaryOperation::Not,
@@ -200,8 +197,8 @@ impl StatementReconstructor for StaticSingleAssigner<'_> {
         // Note that we only produce
         Statement::Conditional(ConditionalStatement {
             condition,
-            then: block,
-            otherwise: next,
+            then,
+            otherwise,
             span: conditional.span,
         })
     }

compiler/passes/src/static_single_assignment/static_single_assigner.rs

@@ -60,7 +60,7 @@ impl<'a> StaticSingleAssigner<'a> {
         Symbol::intern(&format!("{}${}", arg, self.counter - 1))
     }
 
-    /// Constructs a simple `AssignStatement`.
+    /// Constructs the assignment statement `place = expr;`.
     pub(crate) fn simple_assign_statement(place: Expression, value: Expression) -> Statement {
         Statement::Assign(Box::new(AssignStatement {
             operation: AssignOperation::Assign,