From b94c70588eeb31bc08051d6498f50347b07fdf03 Mon Sep 17 00:00:00 2001 From: collin Date: Thu, 3 Dec 2020 13:24:23 -0500 Subject: [PATCH] refactor execution of branch indicators and function returns --- compiler/src/console/assert.rs | 18 +-- compiler/src/console/console.rs | 23 +--- compiler/src/errors/statement.rs | 16 +++ compiler/src/function/function.rs | 30 +---- compiler/src/function/result/result.rs | 115 ++++++++++++------ compiler/src/statement/assign/array.rs | 8 +- compiler/src/statement/assign/assign.rs | 5 +- .../src/statement/assign/circuit_variable.rs | 6 +- compiler/src/statement/assign/tuple.rs | 7 +- compiler/src/statement/branch/branch.rs | 2 +- .../src/statement/conditional/conditional.rs | 16 +-- compiler/src/statement/iteration/iteration.rs | 2 +- compiler/src/statement/statement.rs | 15 ++- 13 files changed, 145 insertions(+), 118 deletions(-) diff --git a/compiler/src/console/assert.rs b/compiler/src/console/assert.rs index 66465bd11f..e3df68ba9d 100644 --- a/compiler/src/console/assert.rs +++ b/compiler/src/console/assert.rs @@ -16,7 +16,13 @@ //! Enforces an assert equals statement in a compiled Leo program. -use crate::{errors::ConsoleError, program::ConstrainedProgram, value::ConstrainedValue, GroupType}; +use crate::{ + errors::ConsoleError, + get_indicator_value, + program::ConstrainedProgram, + value::ConstrainedValue, + GroupType, +}; use leo_ast::{Expression, Span, Type}; use snarkos_models::{ @@ -30,7 +36,7 @@ impl> ConstrainedProgram { cs: &mut CS, file_scope: &str, function_scope: &str, - indicator: Option, + indicator: &Boolean, expression: Expression, span: &Span, ) -> Result<(), ConsoleError> { @@ -42,12 +48,8 @@ impl> ConstrainedProgram { // If the indicator bit is false, do not evaluate the assertion // This is okay since we are not enforcing any constraints - let false_boolean = Boolean::Constant(false); - - if let Some(indicator_bool) = indicator { - if indicator_bool.eq(&false_boolean) { - return Ok(()); // continue execution - } + if !get_indicator_value(indicator) { + return Ok(()); // Continue execution. } // Unwrap assertion value and handle errors diff --git a/compiler/src/console/console.rs b/compiler/src/console/console.rs index 1be1d801cb..23a7276cfa 100644 --- a/compiler/src/console/console.rs +++ b/compiler/src/console/console.rs @@ -16,7 +16,7 @@ //! Evaluates a macro in a compiled Leo program. -use crate::{errors::ConsoleError, program::ConstrainedProgram, GroupType}; +use crate::{errors::ConsoleError, program::ConstrainedProgram, statement::get_indicator_value, GroupType}; use leo_ast::{ConsoleFunction, ConsoleFunctionCall}; use snarkos_models::{ @@ -30,7 +30,7 @@ impl> ConstrainedProgram { cs: &mut CS, file_scope: &str, function_scope: &str, - indicator: Option, + indicator: &Boolean, console: ConsoleFunctionCall, ) -> Result<(), ConsoleError> { match console.function { @@ -40,21 +40,21 @@ impl> ConstrainedProgram { ConsoleFunction::Debug(string) => { let string = self.format(cs, file_scope, function_scope, string)?; - if unwrap_indicator_value(indicator) { + if get_indicator_value(indicator) { tracing::debug!("{}", string); } } ConsoleFunction::Error(string) => { let string = self.format(cs, file_scope, function_scope, string)?; - if unwrap_indicator_value(indicator) { + if get_indicator_value(indicator) { tracing::error!("{}", string); } } ConsoleFunction::Log(string) => { let string = self.format(cs, file_scope, function_scope, string)?; - if unwrap_indicator_value(indicator) { + if get_indicator_value(indicator) { tracing::info!("{}", string); } } @@ -63,16 +63,3 @@ impl> ConstrainedProgram { Ok(()) } } - -// Return the indicator boolean gadget value or true if it is None -// This is okay since we are not enforcing any constraints -fn unwrap_indicator_value(indicator: Option) -> bool { - let false_boolean = Boolean::constant(false); - - if let Some(indicator_bool) = indicator { - if indicator_bool.eq(&false_boolean) { - return false; - } - } - true -} diff --git a/compiler/src/errors/statement.rs b/compiler/src/errors/statement.rs index a70770ca53..4fd0dc7d5e 100644 --- a/compiler/src/errors/statement.rs +++ b/compiler/src/errors/statement.rs @@ -135,6 +135,22 @@ impl StatementError { Self::new_from_span(message, span) } + pub fn multiple_returns(span: Span) -> Self { + let message = + format!("This function returns multiple times and produces unreachable circuits with undefined behavior."); + + Self::new_from_span(message, span) + } + + pub fn no_returns(expected: Type, span: Span) -> Self { + let message = format!( + "function expected `{}` return type but no valid branches returned a result", + expected + ); + + Self::new_from_span(message, span) + } + pub fn select_fail(first: String, second: String, span: Span) -> Self { let message = format!( "Conditional select gadget failed to select between `{}` or `{}`", diff --git a/compiler/src/function/function.rs b/compiler/src/function/function.rs index ab944103f0..ea79ddeb46 100644 --- a/compiler/src/function/function.rs +++ b/compiler/src/function/function.rs @@ -23,11 +23,11 @@ use crate::{ GroupType, }; -use leo_ast::{Expression, Function, FunctionInput, Span, Type}; +use leo_ast::{Expression, Function, FunctionInput, Span}; use snarkos_models::{ curves::{Field, PrimeField}, - gadgets::r1cs::ConstraintSystem, + gadgets::{r1cs::ConstraintSystem, utilities::boolean::Boolean}, }; pub fn check_arguments_length(expected: usize, actual: usize, span: &Span) -> Result<(), FunctionError> { @@ -89,13 +89,14 @@ impl> ConstrainedProgram { // Evaluate every statement in the function and save all potential results let mut results = vec![]; + let indicator = Boolean::constant(true); for statement in function.statements.iter() { let mut result = self.enforce_statement( cs, scope, &function_name, - None, + &indicator, statement.clone(), function.output.clone(), declared_circuit_reference, @@ -105,26 +106,7 @@ impl> ConstrainedProgram { } // Conditionally select a result based on returned indicators - let mut return_values = ConstrainedValue::Tuple(vec![]); - - Self::conditionally_select_result(cs, &mut return_values, results, &function.span)?; - - if let ConstrainedValue::Tuple(ref returns) = return_values { - let return_types = match function.output { - Some(Type::Tuple(types)) => types.len(), - Some(_) => 1usize, - None => 0usize, - }; - - if return_types != returns.len() { - return Err(FunctionError::return_arguments_length( - return_types, - returns.len(), - function.span.clone(), - )); - } - } - - Ok(return_values) + Self::conditionally_select_result(cs, function.output, results, &function.span) + .map_err(|err| FunctionError::StatementError(err)) } } diff --git a/compiler/src/function/result/result.rs b/compiler/src/function/result/result.rs index cec2a05810..4bfa4a8165 100644 --- a/compiler/src/function/result/result.rs +++ b/compiler/src/function/result/result.rs @@ -16,9 +16,15 @@ //! Enforces that one return value is produced in a compiled Leo program. -use crate::{errors::StatementError, program::ConstrainedProgram, value::ConstrainedValue, GroupType}; +use crate::{ + errors::StatementError, + get_indicator_value, + program::ConstrainedProgram, + value::ConstrainedValue, + GroupType, +}; -use leo_ast::Span; +use leo_ast::{Span, Type}; use snarkos_models::{ curves::{Field, PrimeField}, @@ -29,49 +35,84 @@ use snarkos_models::{ }; impl> ConstrainedProgram { - /// iterates through a vector of results and selects one based off of indicators + /// + /// Returns a conditionally selected result from the given possible function returns and + /// given function return type. + /// pub fn conditionally_select_result>( cs: &mut CS, - return_value: &mut ConstrainedValue, - results: Vec<(Option, ConstrainedValue)>, + expected_return: Option, + results: Vec<(Boolean, ConstrainedValue)>, span: &Span, - ) -> Result<(), StatementError> { - // if there are no results, continue - if results.is_empty() { - return Ok(()); + ) -> Result, StatementError> { + // Initialize empty return value. + let mut return_value = ConstrainedValue::Tuple(vec![]); + + // If the function does not expect a return type, then make sure there are no returned results. + let return_type = match expected_return { + Some(return_type) => return_type, + None => { + if results.is_empty() { + // If the function has no returns, then return an empty tuple. + return Ok(return_value); + } else { + return Err(StatementError::invalid_number_of_returns( + 0, + results.len(), + span.to_owned(), + )); + } + } + }; + + // Error if the function or one of its branches does not return. + if let None = results.iter().find(|(indicator, _res)| get_indicator_value(indicator)) { + return Err(StatementError::no_returns(return_type, span.to_owned())); } - // If all indicators are none, then there are no branch conditions in the function. - // We simply return the last result. - - if results.iter().all(|(indicator, _res)| indicator.is_none()) { - let result = &results[results.len() - 1].1; - - *return_value = result.clone(); - - return Ok(()); - } - - // If there are branches in the function we need to use the `ConditionalSelectGadget` to parse through and select the correct one. - // This can be thought of as de-multiplexing all previous wires that may have returned results into one. - for (i, (indicator, result)) in results.into_iter().enumerate() { - // Set the first value as the starting point - if i == 0 { - *return_value = result.clone(); + // Find the return value + let mut ignored = vec![]; + let mut found_return = false; + for (indicator, result) in results.into_iter() { + // Error if a statement returned a result with an incorrect type + let result_type = result.to_type(span)?; + if return_type != result_type { + return Err(StatementError::arguments_type( + &return_type, + &result_type, + span.to_owned(), + )); } - let condition = indicator.unwrap_or(Boolean::Constant(true)); - let selected_value = ConstrainedValue::conditionally_select( - cs.ns(|| format!("select {} {}:{}", result, span.line, span.start)), - &condition, - &result, - return_value, - ) - .map_err(|_| StatementError::select_fail(result.to_string(), return_value.to_string(), span.to_owned()))?; - - *return_value = selected_value; + if get_indicator_value(&indicator) { + // Error if we already have a return value. + if found_return { + return Err(StatementError::multiple_returns(span.to_owned())); + } else { + // Set the function return value. + return_value = result; + found_return = true; + } + } else { + // Ignore a possible function return value. + ignored.push((indicator, result)) + } } - Ok(()) + // Conditionally select out the ignored results in the circuit. + // + // If there are branches in the function we need to use the `ConditionalSelectGadget` to parse through and select the correct one. + // This can be thought of as de-multiplexing all previous wires that may have returned results into one. + for (i, (indicator, result)) in ignored.into_iter().enumerate() { + return_value = ConstrainedValue::conditionally_select( + cs.ns(|| format!("select result {} {}:{}", i, span.line, span.start)), + &indicator, + &result, + &return_value, + ) + .map_err(|_| StatementError::select_fail(result.to_string(), return_value.to_string(), span.to_owned()))?; + } + + Ok(return_value) } } diff --git a/compiler/src/statement/assign/array.rs b/compiler/src/statement/assign/array.rs index 842cdefbfe..0f56d9461b 100644 --- a/compiler/src/statement/assign/array.rs +++ b/compiler/src/statement/assign/array.rs @@ -34,14 +34,12 @@ impl> ConstrainedProgram { cs: &mut CS, file_scope: &str, function_scope: &str, - indicator: Option, + indicator: &Boolean, name: &str, range_or_expression: RangeOrExpression, mut new_value: ConstrainedValue, span: &Span, ) -> Result<(), StatementError> { - let condition = indicator.unwrap_or(Boolean::Constant(true)); - // Resolve index so we know if we are assigning to a single value or a range of values match range_or_expression { RangeOrExpression::Expression(index) => { @@ -54,7 +52,7 @@ impl> ConstrainedProgram { let selected_value = ConstrainedValue::conditionally_select( cs.ns(|| format!("select {} {}:{}", new_value, span.line, span.start)), - &condition, + indicator, &new_value, &old[index], ) @@ -90,7 +88,7 @@ impl> ConstrainedProgram { }; let selected_array = ConstrainedValue::conditionally_select( cs.ns(|| format!("select {} {}:{}", new_array, span.line, span.start)), - &condition, + indicator, &new_array, old_array, ) diff --git a/compiler/src/statement/assign/assign.rs b/compiler/src/statement/assign/assign.rs index 8899c9b286..25006a05e6 100644 --- a/compiler/src/statement/assign/assign.rs +++ b/compiler/src/statement/assign/assign.rs @@ -42,7 +42,7 @@ impl> ConstrainedProgram { file_scope: &str, function_scope: &str, declared_circuit_reference: &str, - indicator: Option, + indicator: &Boolean, assignee: Assignee, expression: Expression, span: &Span, @@ -55,14 +55,13 @@ impl> ConstrainedProgram { // Mutate the old value into the new value if assignee.accesses.is_empty() { - let condition = indicator.unwrap_or(Boolean::Constant(true)); let old_value = self.get_mutable_assignee(&variable_name, span)?; new_value.resolve_type(Some(old_value.to_type(&span)?), span)?; let selected_value = ConstrainedValue::conditionally_select( cs.ns(|| format!("select {} {}:{}", new_value, span.line, span.start)), - &condition, + indicator, &new_value, old_value, ) diff --git a/compiler/src/statement/assign/circuit_variable.rs b/compiler/src/statement/assign/circuit_variable.rs index 146c95f983..60d0ebb7f6 100644 --- a/compiler/src/statement/assign/circuit_variable.rs +++ b/compiler/src/statement/assign/circuit_variable.rs @@ -31,14 +31,12 @@ impl> ConstrainedProgram { pub fn mutate_circuit_variable>( &mut self, cs: &mut CS, - indicator: Option, + indicator: &Boolean, circuit_name: &str, variable_name: Identifier, mut new_value: ConstrainedValue, span: &Span, ) -> Result, StatementError> { - let condition = indicator.unwrap_or(Boolean::Constant(true)); - // Get the mutable circuit by name match self.get_mutable_assignee(circuit_name, span)? { ConstrainedValue::CircuitExpression(_variable, members) => { @@ -70,7 +68,7 @@ impl> ConstrainedProgram { // Conditionally select the value if this branch is executed. let mut selected_value = ConstrainedValue::conditionally_select( cs.ns(|| format!("select {} {}:{}", new_value, span.line, span.start)), - &condition, + indicator, &new_value, &member.1, ) diff --git a/compiler/src/statement/assign/tuple.rs b/compiler/src/statement/assign/tuple.rs index 5ecb0dd767..4209a1d570 100644 --- a/compiler/src/statement/assign/tuple.rs +++ b/compiler/src/statement/assign/tuple.rs @@ -31,15 +31,12 @@ impl> ConstrainedProgram { pub fn assign_tuple>( &mut self, cs: &mut CS, - indicator: Option, + indicator: &Boolean, name: &str, index: PositiveNumber, mut new_value: ConstrainedValue, span: &Span, ) -> Result<(), StatementError> { - // Get the indicator value. - let condition = indicator.unwrap_or(Boolean::Constant(true)); - // Parse the index. let index_usize = parse_index(&index, &span)?; @@ -50,7 +47,7 @@ impl> ConstrainedProgram { let selected_value = ConstrainedValue::conditionally_select( cs.ns(|| format!("select {} {}:{}", new_value, span.line, span.start)), - &condition, + indicator, &new_value, &old[index_usize], ) diff --git a/compiler/src/statement/branch/branch.rs b/compiler/src/statement/branch/branch.rs index 9d0d208036..f948c85a73 100644 --- a/compiler/src/statement/branch/branch.rs +++ b/compiler/src/statement/branch/branch.rs @@ -30,7 +30,7 @@ impl> ConstrainedProgram { cs: &mut CS, file_scope: &str, function_scope: &str, - indicator: Option, + indicator: &Boolean, statements: Vec, return_type: Option, ) -> StatementResult>> { diff --git a/compiler/src/statement/conditional/conditional.rs b/compiler/src/statement/conditional/conditional.rs index 48bad283b8..94e3a8726e 100644 --- a/compiler/src/statement/conditional/conditional.rs +++ b/compiler/src/statement/conditional/conditional.rs @@ -49,15 +49,15 @@ impl> ConstrainedProgram { cs: &mut CS, file_scope: &str, function_scope: &str, - indicator: Option, + indicator: &Boolean, statement: ConditionalStatement, return_type: Option, span: &Span, ) -> StatementResult>> { let statement_string = statement.to_string(); - // Inherit the indicator from a previous conditional statement or assume that we are the outer parent - let outer_indicator = indicator.unwrap_or(Boolean::Constant(true)); + // Inherit an indicator from a previous statement. + let outer_indicator = indicator; // Evaluate the conditional boolean as the inner indicator let inner_indicator = match self.enforce_expression( @@ -72,7 +72,7 @@ impl> ConstrainedProgram { }; // If outer_indicator && inner_indicator, then select branch 1 - let outer_indicator_string = indicator_to_string(&outer_indicator); + let outer_indicator_string = indicator_to_string(outer_indicator); let inner_indicator_string = indicator_to_string(&inner_indicator); let branch_1_name = format!( "branch indicator 1 {} && {}", @@ -80,7 +80,7 @@ impl> ConstrainedProgram { ); let branch_1_indicator = Boolean::and( &mut cs.ns(|| format!("branch 1 {} {}:{}", statement_string, span.line, span.start)), - &outer_indicator, + outer_indicator, &inner_indicator, ) .map_err(|_| StatementError::indicator_calculation(branch_1_name, span.to_owned()))?; @@ -92,7 +92,7 @@ impl> ConstrainedProgram { cs, file_scope, function_scope, - Some(branch_1_indicator), + &branch_1_indicator, statement.statements, return_type.clone(), )?; @@ -120,7 +120,7 @@ impl> ConstrainedProgram { cs, file_scope, function_scope, - Some(branch_2_indicator), + &branch_2_indicator, *nested, return_type, span, @@ -129,7 +129,7 @@ impl> ConstrainedProgram { cs, file_scope, function_scope, - Some(branch_2_indicator), + &branch_2_indicator, statements, return_type, )?, diff --git a/compiler/src/statement/iteration/iteration.rs b/compiler/src/statement/iteration/iteration.rs index 64104cbb45..509d4c22bd 100644 --- a/compiler/src/statement/iteration/iteration.rs +++ b/compiler/src/statement/iteration/iteration.rs @@ -42,7 +42,7 @@ impl> ConstrainedProgram { cs: &mut CS, file_scope: &str, function_scope: &str, - indicator: Option, + indicator: &Boolean, index: Identifier, start: Expression, stop: Expression, diff --git a/compiler/src/statement/statement.rs b/compiler/src/statement/statement.rs index 763ea328a7..16d9577e29 100644 --- a/compiler/src/statement/statement.rs +++ b/compiler/src/statement/statement.rs @@ -25,7 +25,7 @@ use snarkos_models::{ }; pub type StatementResult = Result; -pub type IndicatorAndConstrainedValue = (Option, ConstrainedValue); +pub type IndicatorAndConstrainedValue = (Boolean, ConstrainedValue); impl> ConstrainedProgram { /// @@ -41,7 +41,7 @@ impl> ConstrainedProgram { cs: &mut CS, file_scope: &str, function_scope: &str, - indicator: Option, + indicator: &Boolean, statement: Statement, return_type: Option, declared_circuit_reference: &str, @@ -51,7 +51,7 @@ impl> ConstrainedProgram { match statement { Statement::Return(expression, span) => { let return_value = ( - indicator, + indicator.to_owned(), self.enforce_return_statement(cs, file_scope, function_scope, expression, return_type, &span)?, ); @@ -126,7 +126,7 @@ impl> ConstrainedProgram { _ => return Err(StatementError::unassigned(expression_string, span)), } - let result = (indicator, value); + let result = (indicator.to_owned(), value); results.push(result); } @@ -135,3 +135,10 @@ impl> ConstrainedProgram { Ok(results) } } + +/// Returns the indicator boolean gadget value. +/// We can directly compare a boolean constant to the indicator since we are not enforcing any +/// constraints +pub fn get_indicator_value(indicator: &Boolean) -> bool { + indicator.eq(&Boolean::constant(true)) +}