From d3125a0f9fcfbc903e6eb72eea740574510c936d Mon Sep 17 00:00:00 2001 From: collin Date: Mon, 26 Oct 2020 18:41:51 -0700 Subject: [PATCH] add new dynamic check errors and tests for variables --- compiler/tests/statements/ternary_basic.leo | 2 +- dynamic-check/src/dynamic_check.rs | 25 ++++++---- dynamic-check/src/errors/dynamic_check.rs | 16 ------ dynamic-check/src/errors/frame.rs | 48 ++++++++++++++---- dynamic-check/tests/mod.rs | 28 ++++++++--- .../tests/variables/duplicate_variable.leo | 4 ++ .../variables/duplicate_variable_multi.leo | 3 ++ dynamic-check/tests/variables/mod.rs | 49 +++++++++++++++++++ .../tests/variables/not_enough_values.leo | 3 ++ .../tests/variables/too_many_values.leo | 3 ++ 10 files changed, 137 insertions(+), 44 deletions(-) create mode 100644 dynamic-check/tests/variables/duplicate_variable.leo create mode 100644 dynamic-check/tests/variables/duplicate_variable_multi.leo create mode 100644 dynamic-check/tests/variables/mod.rs create mode 100644 dynamic-check/tests/variables/not_enough_values.leo create mode 100644 dynamic-check/tests/variables/too_many_values.leo diff --git a/compiler/tests/statements/ternary_basic.leo b/compiler/tests/statements/ternary_basic.leo index 89158a3e78..5106363ce2 100644 --- a/compiler/tests/statements/ternary_basic.leo +++ b/compiler/tests/statements/ternary_basic.leo @@ -1,5 +1,5 @@ function main(a: bool, b: bool) { let c = if a ? true : false; - const a = c == b; + const d = c == b; } \ No newline at end of file diff --git a/dynamic-check/src/dynamic_check.rs b/dynamic-check/src/dynamic_check.rs index 3a6cdaee77..c6645e24c4 100644 --- a/dynamic-check/src/dynamic_check.rs +++ b/dynamic-check/src/dynamic_check.rs @@ -299,12 +299,15 @@ impl Frame { /// /// Insert a variable into the symbol table in the current scope. /// - fn insert_variable(&mut self, name: String, type_: Type) -> Option { + fn insert_variable(&mut self, name: String, type_: Type, span: &Span) -> Result<(), FrameError> { // Modify the current scope. let scope = self.scopes.last_mut().unwrap(); // Insert the variable name -> type. - scope.variables.insert(name, type_) + match scope.variables.insert(name.clone(), type_) { + Some(_type) => Err(FrameError::duplicate_variable(&name, span)), + None => Ok(()), + } } /// @@ -459,24 +462,26 @@ impl Frame { // Insert variable into symbol table let variable = variables.names[0].clone(); - // TODO (collinc97) throw error for duplicate variable definitions. - let _expect_none = self.insert_variable(variable.identifier.name, actual_type); + self.insert_variable(variable.identifier.name, actual_type, span)?; } else { // Expect a tuple type. let types = match actual_type { Type::Tuple(types) => types, - _ => unimplemented!("expected a tuple type for multiple defined variables"), + _ => return Err(FrameError::not_enough_values(span)), }; // Check number of variables == number of types. if types.len() != variables.names.len() { - unimplemented!("Incorrect number of defined variables") + return Err(FrameError::invalid_number_of_values( + types.len(), + variables.names.len(), + span, + )); } // Insert variables into symbol table for (variable, type_) in variables.names.iter().zip(types) { - // TODO (collinc97) throw error for duplicate variable definitions. - let _expect_none = self.insert_variable(variable.identifier.name.clone(), type_); + self.insert_variable(variable.identifier.name.clone(), type_, span)?; } } @@ -605,7 +610,7 @@ impl Frame { ) -> Result<(), FrameError> { // Insert variable into symbol table with u32 type. let u32_type = Type::IntegerType(IntegerType::U32); - let _expect_none = self.insert_variable(identifier.name.to_owned(), u32_type.clone()); + let _expect_none = self.insert_variable(identifier.name.to_owned(), u32_type.clone(), span); // Parse `from` and `to` expressions. let from_type = self.parse_expression(from)?; @@ -1043,7 +1048,7 @@ impl Frame { // Check the length of the circuit members. if circuit_type.variables.len() != members.len() { - return Err(FrameError::num_variables( + return Err(FrameError::num_circuit_variables( circuit_type.variables.len(), members.len(), span, diff --git a/dynamic-check/src/errors/dynamic_check.rs b/dynamic-check/src/errors/dynamic_check.rs index bbcfffe457..33cdc2715b 100644 --- a/dynamic-check/src/errors/dynamic_check.rs +++ b/dynamic-check/src/errors/dynamic_check.rs @@ -27,18 +27,6 @@ pub enum DynamicCheckError { #[error("{}", _0)] FrameError(#[from] FrameError), - // - // #[error("{}", _0)] - // ExpressionError(#[from] ExpressionError), - // - // #[error("{}", _0)] - // FunctionError(#[from] FunctionError), - // - // #[error("{}", _0)] - // StatementError(#[from] StatementError), - // - // #[error("{}", _0)] - // ProgramError(#[from] ProgramError), } impl DynamicCheckError { @@ -49,10 +37,6 @@ impl DynamicCheckError { match self { DynamicCheckError::Error(error) => error.set_path(path), DynamicCheckError::FrameError(error) => error.set_path(path), - // DynamicCheckError::ExpressionError(error) => error.set_path(path), - // DynamicCheckError::FunctionError(error) => error.set_path(path), - // DynamicCheckError::StatementError(error) => error.set_path(path), - // DynamicCheckError::ProgramError(error) => error.set_path(path), } } } diff --git a/dynamic-check/src/errors/frame.rs b/dynamic-check/src/errors/frame.rs index 0b9c6a7fee..3a199558ac 100644 --- a/dynamic-check/src/errors/frame.rs +++ b/dynamic-check/src/errors/frame.rs @@ -65,6 +65,15 @@ impl FrameError { Self::new_from_span(message, span.to_owned()) } + /// + /// Two variables have been defined with the same name. + /// + pub fn duplicate_variable(name: &str, span: &Span) -> Self { + let message = format!("Duplicate variable definition found for `{}`", name); + + Self::new_from_span(message, span.to_owned()) + } + /// /// Attempted to call non-static member using `::`. /// @@ -83,6 +92,15 @@ impl FrameError { Self::new_from_span(message, identifier.span.to_owned()) } + /// + /// Attempted to create a circuit with the incorrect number of member variables. + /// + pub fn num_circuit_variables(expected: usize, actual: usize, span: &Span) -> Self { + let message = format!("Circuit expected {} variables, found {} variables.", expected, actual); + + Self::new_from_span(message, span.clone()) + } + /// /// Attempted to call a function with the incorrect number of inputs. /// @@ -95,15 +113,6 @@ impl FrameError { Self::new_from_span(message, span.clone()) } - /// - /// Attempted to create a circuit with the incorrect number of member variables. - /// - pub fn num_variables(expected: usize, actual: usize, span: &Span) -> Self { - let message = format!("Circuit expected {} variables, found {} variables.", expected, actual); - - Self::new_from_span(message, span.clone()) - } - /// /// Attempted to call a circuit type that is not defined in the current context. /// @@ -139,4 +148,25 @@ impl FrameError { Self::new_from_span(message, identifier.span.to_owned()) } + + /// + /// Attempted to assign a tuple of variables to a single value. + /// + pub fn not_enough_values(span: &Span) -> Self { + let message = "Expected a tuple type for multiple defined variables".to_string(); + + Self::new_from_span(message, span.to_owned()) + } + + /// + /// Attempted to assign a tuple with a different number of variables than values. + /// + pub fn invalid_number_of_values(expected: usize, actual: usize, span: &Span) -> Self { + let message = format!( + "Incorrect number of defined variables. Expected `{}`, found `{}`.", + expected, actual + ); + + Self::new_from_span(message, span.to_owned()) + } } diff --git a/dynamic-check/tests/mod.rs b/dynamic-check/tests/mod.rs index 96eac2b74e..71be9af341 100644 --- a/dynamic-check/tests/mod.rs +++ b/dynamic-check/tests/mod.rs @@ -13,13 +13,14 @@ // You should have received a copy of the GNU General Public License // along with the Leo library. If not, see . +pub mod variables; use leo_ast::LeoAst; use leo_dynamic_check::DynamicCheck; use leo_imports::ImportParser; -use leo_static_check::StaticCheck; -use leo_typed::{Input, LeoTypedAst}; +use leo_static_check::{StaticCheck, SymbolTable}; +use leo_typed::{Input, LeoTypedAst, Program}; use std::path::PathBuf; const TEST_PROGRAM_PATH: &str = ""; @@ -27,7 +28,8 @@ const TEST_PROGRAM_NAME: &str = "test"; /// A helper struct to test a `DynamicCheck`. pub struct TestDynamicCheck { - dynamic_check: DynamicCheck, + program: Program, + symbol_table: SymbolTable, } impl TestDynamicCheck { @@ -54,14 +56,24 @@ impl TestDynamicCheck { // Create static check. let symbol_table = StaticCheck::run_with_input(&program, &import_parser, &input).unwrap(); - // Create dynamic check - let dynamic_check = DynamicCheck::new(&program, symbol_table).unwrap(); - - Self { dynamic_check } + // Store fields for new dynamic check. + Self { program, symbol_table } } pub fn solve(self) { - self.dynamic_check.solve().unwrap(); + let dynamic_check = DynamicCheck::new(&self.program, self.symbol_table).unwrap(); + + dynamic_check.solve().unwrap(); + } + + pub fn expect_create_error(self) { + assert!(DynamicCheck::new(&self.program, self.symbol_table).is_err()); + } + + pub fn expect_solve_error(self) { + let dynamic_check = DynamicCheck::new(&self.program, self.symbol_table).unwrap(); + + assert!(dynamic_check.solve().is_err()); } } diff --git a/dynamic-check/tests/variables/duplicate_variable.leo b/dynamic-check/tests/variables/duplicate_variable.leo new file mode 100644 index 0000000000..a748ef4efe --- /dev/null +++ b/dynamic-check/tests/variables/duplicate_variable.leo @@ -0,0 +1,4 @@ +function main() { + let a = 1u8; + let a = 2u8; // Redefining variables with the same name is unsafe in Leo. +} \ No newline at end of file diff --git a/dynamic-check/tests/variables/duplicate_variable_multi.leo b/dynamic-check/tests/variables/duplicate_variable_multi.leo new file mode 100644 index 0000000000..d0fabdea07 --- /dev/null +++ b/dynamic-check/tests/variables/duplicate_variable_multi.leo @@ -0,0 +1,3 @@ +function main() { + let (a, a) = (2u8, 2u8); // Defining multiple variables with the same name is unsafe in Leo. +} \ No newline at end of file diff --git a/dynamic-check/tests/variables/mod.rs b/dynamic-check/tests/variables/mod.rs new file mode 100644 index 0000000000..b60a7217f0 --- /dev/null +++ b/dynamic-check/tests/variables/mod.rs @@ -0,0 +1,49 @@ +// Copyright (C) 2019-2020 Aleo Systems Inc. +// This file is part of the Leo library. + +// The Leo library is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// The Leo library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with the Leo library. If not, see . + +use crate::TestDynamicCheck; + +#[test] +fn test_duplicate_variable() { + let bytes = include_bytes!("duplicate_variable.leo"); + let check = TestDynamicCheck::new(bytes); + + check.expect_create_error(); +} + +#[test] +fn test_duplicate_variable_multi() { + let bytes = include_bytes!("duplicate_variable_multi.leo"); + let check = TestDynamicCheck::new(bytes); + + check.expect_create_error(); +} + +#[test] +fn test_not_enough_values() { + let bytes = include_bytes!("not_enough_values.leo"); + let check = TestDynamicCheck::new(bytes); + + check.expect_create_error(); +} + +#[test] +fn test_too_many_values() { + let bytes = include_bytes!("too_many_values.leo"); + let check = TestDynamicCheck::new(bytes); + + check.expect_create_error(); +} diff --git a/dynamic-check/tests/variables/not_enough_values.leo b/dynamic-check/tests/variables/not_enough_values.leo new file mode 100644 index 0000000000..58b1ad7244 --- /dev/null +++ b/dynamic-check/tests/variables/not_enough_values.leo @@ -0,0 +1,3 @@ +function main() { + let (a, b): (u8, u8) = 1; // A tuple of values must be used when defining two variables. +} \ No newline at end of file diff --git a/dynamic-check/tests/variables/too_many_values.leo b/dynamic-check/tests/variables/too_many_values.leo new file mode 100644 index 0000000000..bd3f551231 --- /dev/null +++ b/dynamic-check/tests/variables/too_many_values.leo @@ -0,0 +1,3 @@ +function main() { + let (a, b): (u8, u8) = (1, 2, 3); // Cannot assign 2 variables to 3 values. +} \ No newline at end of file