Cleanup

compiler/ast/src/passes/consumer.rs

@@ -15,7 +15,7 @@
 // along with the Leo library. If not, see <https://www.gnu.org/licenses/>.
 
 //! This module contains a Consumer trait for the AST.
-//! Consumers are used to completely transform the AST without any opinion on the output.
+//! Consumers are used to completely transform the AST without any restrictions on the output.
 
 use crate::*;
 

compiler/ast/src/passes/mod.rs

@@ -17,7 +17,7 @@
 //! This module contains both a Reducer and Visitor design pattern.
 //! These both iterate over the AST.
 
-// todo @gluax: Move the files in this module into `leo-passes` in a future PR.
+// TODO: Move the files in this module into `leo-passes` in a future PR.
 
 pub mod consumer;
 pub use consumer::*;

compiler/passes/src/code_generation/visit_expressions.rs

@@ -45,7 +45,6 @@ impl<'a> CodeGenerator<'a> {
     }
 
     fn visit_identifier(&mut self, input: &'a Identifier) -> (String, String) {
-        println!("{:?}", input);
         (self.variable_mapping.get(&input.name).unwrap().clone(), String::new())
     }
 

compiler/passes/src/static_single_assignment/mod.rs

@@ -18,7 +18,7 @@
 //! 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 `ReturnStatement`s into `AssignStatement`s and consolidates the returned values as a single `ReturnStatement` at the end of the function.
-//! The pass also simplifies complex expressions into a sequence of `AssignStatement`s. For example, `(a + b) * c` is rewritten into `expr$1 = a + b; expr$2 = expr$1 * c`.
+//! The pass also simplifies complex expressions into a sequence of `AssignStatement`s. For example, `(a + b) * c` is rewritten into `$var$1 = a + b; $var$2 = $var$1 * c`.
 //!
 //! Consider the following Leo code.
 //! ```leo
@@ -71,8 +71,6 @@ impl<'a> Pass for StaticSingleAssigner<'a> {
         let program = consumer.consume_program(ast.into_repr());
         handler.last_err()?;
 
-        println!("AST AFTER SSA:\n{}", program);
-
         Ok(Ast::new(program))
     }
 }

compiler/passes/src/static_single_assignment/rename_expression.rs

@@ -144,7 +144,7 @@ impl ExpressionConsumer for StaticSingleAssigner<'_> {
                     // In this case, we must consume the expression.
                     true => self.consume_expression(arg.expression.unwrap()),
                 };
-                // Add the output to the additional output.
+                // Accumulate any statements produced.
                 statements.append(&mut stmts);
 
                 // Return the new member.
@@ -244,8 +244,8 @@ impl ExpressionConsumer for StaticSingleAssigner<'_> {
             })
             .collect();
 
-        // Note that we do not construct new assignment statement for the tuple expression, since tuple expressions are not supported.
         // TODO: Fix when tuple expressions are supported.
+        // Note that we do not construct new assignment statement for the tuple expression, since tuple expressions are not supported.
         (
             Expression::Tuple(TupleExpression {
                 elements,

compiler/passes/src/static_single_assignment/rename_program.rs

@@ -46,8 +46,26 @@ impl FunctionConsumer for StaticSingleAssigner<'_> {
         let (_, last_return_expression) = returns.pop().unwrap();
 
         // Produce a chain of ternary expressions and assignments for the set of early returns.
-        // TODO: Can this be simplified?
         let mut stmts = Vec::with_capacity(returns.len());
+
+        // Helper to construct and store ternary assignments. e.g `$ret$0 = $var$0 ? $var$1 : $var$2`
+        let mut construct_ternary_assignment = |guard: Expression, if_true: Expression, if_false: Expression| {
+            let place = Expression::Identifier(Identifier {
+                name: self.unique_symbol("$ret"),
+                span: Default::default(),
+            });
+            stmts.push(Self::simple_assign_statement(
+                place.clone(),
+                Expression::Ternary(TernaryExpression {
+                    condition: Box::new(guard),
+                    if_true: Box::new(if_true),
+                    if_false: Box::new(if_false),
+                    span: Default::default(),
+                }),
+            ));
+            place
+        };
+
         let expression = returns
             .into_iter()
             .rev()
@@ -64,19 +82,7 @@ impl FunctionConsumer for StaticSingleAssigner<'_> {
                                 .into_iter()
                                 .zip_eq(acc_tuple.elements.into_iter())
                                 .map(|(if_true, if_false)| {
-                                    // Create an assignment statement for the element expression in the tuple.
+                                    construct_ternary_assignment(guard.clone(), if_true, if_false)
-                                    let place = Expression::Identifier(Identifier {
-                                        name: self.unique_symbol("$ret"),
-                                        span: Default::default(),
-                                    });
-                                    let value = Expression::Ternary(TernaryExpression {
-                                        condition: Box::new(guard.clone()),
-                                        if_true: Box::new(if_true),
-                                        if_false: Box::new(if_false),
-                                        span: Default::default(),
-                                    });
-                                    stmts.push(Self::simple_assign_statement(place.clone(), value));
-                                    place
                                 })
                                 .collect(),
                             span: Default::default(),
@@ -84,26 +90,12 @@ impl FunctionConsumer for StaticSingleAssigner<'_> {
                     }
                     // Otherwise, fold the return expressions into a single ternary expression.
                     // Note that `expr` and `acc` are correspond to the `if` and `else` cases of the ternary expression respectively.
-                    (expr, acc) => {
+                    (expr, acc) => construct_ternary_assignment(guard, expr, acc),
-                        let place = Expression::Identifier(Identifier {
-                            name: self.unique_symbol("$ret"),
-                            span: Default::default(),
-                        });
-                        let value = Expression::Ternary(TernaryExpression {
-                            condition: Box::new(guard),
-                            if_true: Box::new(expr),
-                            if_false: Box::new(acc),
-                            span: Default::default(),
-                        });
-                        stmts.push(Self::simple_assign_statement(place.clone(), value));
-                        place
-                    }
                 },
             });
 
-        println!("1");
+        // Add all of the accumulated statements to the end of the block.
         statements.extend(stmts);
-        println!("2");
 
         // Add the `ReturnStatement` to the end of the block.
         statements.push(Statement::Return(ReturnStatement {
@@ -137,7 +129,7 @@ impl ProgramConsumer for StaticSingleAssigner<'_> {
             name: input.name,
             network: input.network,
             expected_input: input.expected_input,
-            // TODO: Do inputs need to be processed?
+            // TODO: Do inputs need to be processed? They are not processed in the existing compiler.
             imports: input.imports,
             functions: input
                 .functions

compiler/passes/src/static_single_assignment/rename_statement.rs

@@ -59,8 +59,11 @@ impl StatementConsumer for StaticSingleAssigner<'_> {
 
     /// Consumes the `DefinitionStatement` into an `AssignStatement`, renaming the left-hand-side as appropriate.
     fn consume_definition(&mut self, definition: DefinitionStatement) -> Self::Output {
+        // First consume the right-hand-side of the definition.
         let (value, mut statements) = self.consume_expression(definition.value);
 
+        // Then assign a new unique name to the left-hand-side of the definition.
+        // Note that this order is necessary to ensure that the right-hand-side uses the correct name when consuming a complex assignment.
         self.is_lhs = true;
         let identifier = match self.consume_identifier(definition.variable_name).0 {
             Expression::Identifier(identifier) => identifier,
@@ -99,11 +102,6 @@ impl StatementConsumer for StaticSingleAssigner<'_> {
     fn consume_conditional(&mut self, conditional: ConditionalStatement) -> Self::Output {
         // Simplify the condition and add it into the rename table.
         let (condition, mut statements) = self.consume_expression(conditional.condition);
-        // TODO: Is this needed?
-        println!("Condition:\n{:?}\n", condition);
-        println!("Statements:\n{:?}\n", statements);
-        println!("Rename Table:\n{:?}\n", self.rename_table);
-        // self.rename_table.update(symbol, symbol);
 
         // Instantiate a `RenameTable` for the then-block.
         self.push();
@@ -132,18 +130,7 @@ impl StatementConsumer for StaticSingleAssigner<'_> {
                 span: condition.span(),
             }));
 
-            statements.extend(match *statement {
+            statements.extend(self.consume_statement(*statement));
-                // TODO: Check that the statement below still holds.
-                // The `ConditionalStatement` must be consumed as a `Block` statement to ensure that appropriate statements are produced.
-                Statement::Conditional(stmt) => self.consume_block(Block {
-                    statements: vec![Statement::Conditional(stmt)],
-                    span: Default::default(),
-                }),
-                Statement::Block(stmt) => self.consume_block(stmt),
-                _ => unreachable!(
-                    "`ConditionalStatement`s next statement must be a `ConditionalStatement` or a `Block`."
-                ),
-            });
 
             // Remove the negated condition from the condition stack.
             self.condition_stack.pop();
@@ -192,7 +179,7 @@ impl StatementConsumer for StaticSingleAssigner<'_> {
                 // Update the `RenameTable` with the new name of the variable.
                 self.rename_table.update(*(*symbol), new_name);
 
-                // Store the generate phi functions.
+                // Store the generated phi function.
                 statements.push(assignment);
             }
         }
@@ -200,10 +187,12 @@ impl StatementConsumer for StaticSingleAssigner<'_> {
         statements
     }
 
+    // TODO: Error message
     fn consume_iteration(&mut self, _input: IterationStatement) -> Self::Output {
         unreachable!("`IterationStatement`s should not be in the AST at this phase of compilation.");
     }
 
+    // TODO: Where do we handle console statements.
     fn consume_console(&mut self, input: ConsoleStatement) -> Self::Output {
         vec![Statement::Console(input)]
     }