From a39ab9f77fdca6ef2c719d8a20a4ff373ca341a4 Mon Sep 17 00:00:00 2001 From: Pranav Gaddamadugu Date: Tue, 16 Aug 2022 09:00:06 -0700 Subject: [PATCH] Cleanup --- compiler/ast/src/passes/consumer.rs | 2 +- compiler/ast/src/passes/mod.rs | 2 +- .../src/code_generation/visit_expressions.rs | 1 - .../src/static_single_assignment/mod.rs | 4 +- .../rename_expression.rs | 4 +- .../rename_program.rs | 54 ++++++++----------- .../rename_statement.rs | 25 +++------ 7 files changed, 35 insertions(+), 57 deletions(-) diff --git a/compiler/ast/src/passes/consumer.rs b/compiler/ast/src/passes/consumer.rs index c0d3772141..21359e12ea 100644 --- a/compiler/ast/src/passes/consumer.rs +++ b/compiler/ast/src/passes/consumer.rs @@ -15,7 +15,7 @@ // along with the Leo library. If not, see . //! 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::*; diff --git a/compiler/ast/src/passes/mod.rs b/compiler/ast/src/passes/mod.rs index eab7d43595..1dbbeec3db 100644 --- a/compiler/ast/src/passes/mod.rs +++ b/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::*; diff --git a/compiler/passes/src/code_generation/visit_expressions.rs b/compiler/passes/src/code_generation/visit_expressions.rs index 40faa6af17..b48b1a9f85 100644 --- a/compiler/passes/src/code_generation/visit_expressions.rs +++ b/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()) } diff --git a/compiler/passes/src/static_single_assignment/mod.rs b/compiler/passes/src/static_single_assignment/mod.rs index 4acb1382a6..0cc4aef260 100644 --- a/compiler/passes/src/static_single_assignment/mod.rs +++ b/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)) } } diff --git a/compiler/passes/src/static_single_assignment/rename_expression.rs b/compiler/passes/src/static_single_assignment/rename_expression.rs index 82715247ec..7585755e05 100644 --- a/compiler/passes/src/static_single_assignment/rename_expression.rs +++ b/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, diff --git a/compiler/passes/src/static_single_assignment/rename_program.rs b/compiler/passes/src/static_single_assignment/rename_program.rs index ed9af19ca8..3453797d0d 100644 --- a/compiler/passes/src/static_single_assignment/rename_program.rs +++ b/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. - 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 + construct_ternary_assignment(guard.clone(), if_true, if_false) }) .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) => { - 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 - } + (expr, acc) => construct_ternary_assignment(guard, expr, acc), }, }); - 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 diff --git a/compiler/passes/src/static_single_assignment/rename_statement.rs b/compiler/passes/src/static_single_assignment/rename_statement.rs index b4e6839fa4..b669e7326f 100644 --- a/compiler/passes/src/static_single_assignment/rename_statement.rs +++ b/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 { - // 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`." - ), - }); + statements.extend(self.consume_statement(*statement)); // 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)] }