Feedback from code review

This commit is contained in:
Pranav Gaddamadugu 2022-07-14 14:26:13 -07:00
parent f5eea6b307
commit 923d5924fe
13 changed files with 40 additions and 56 deletions

View File

@ -19,18 +19,18 @@ use std::fmt;
/// The sort of bindings to introduce, either `let` or `const`.
#[derive(Debug, Copy, Clone, PartialEq, Eq, Serialize, Deserialize)]
pub enum Declare {
pub enum DeclarationType {
/// This is a `const` binding.
Const,
/// This is a `let` binding.
Let,
}
impl fmt::Display for Declare {
impl fmt::Display for DeclarationType {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
Declare::Const => write!(f, "const"),
Declare::Let => write!(f, "let"),
DeclarationType::Const => write!(f, "const"),
DeclarationType::Let => write!(f, "let"),
}
}
}

View File

@ -23,14 +23,14 @@ use std::fmt;
mod variable_name;
pub use variable_name::*;
mod declare;
pub use declare::*;
mod declaration_type;
pub use declaration_type::*;
/// A `let` or `const` declaration statement.
#[derive(Clone, PartialEq, Eq, Serialize, Deserialize, Debug)]
pub struct DefinitionStatement {
/// What sort of declaration is this? `let` or `const`?.
pub declaration_type: Declare,
pub declaration_type: DeclarationType,
/// The bindings / variable names to declare.
pub variable_names: Vec<VariableName>,
/// The types of the bindings, if specified, or inferred otherwise.

View File

@ -50,9 +50,7 @@ pub struct Compiler<'a> {
}
impl<'a> Compiler<'a> {
///
/// Returns a new Leo compiler.
///
pub fn new(
handler: &'a Handler,
main_file_path: PathBuf,
@ -69,9 +67,7 @@ impl<'a> Compiler<'a> {
}
}
///
/// Returns a SHA256 checksum of the program file.
///
pub fn checksum(&self) -> Result<String> {
// Read in the main file as string
let unparsed_file = fs::read_to_string(&self.main_file_path)
@ -85,7 +81,7 @@ impl<'a> Compiler<'a> {
Ok(format!("{:x}", hash))
}
// Parses and stores a program file content from a string, constructs a syntax tree, and generates a program.
/// Parses and stores a program file content from a string, constructs a syntax tree, and generates a program.
pub fn parse_program_from_string(&mut self, program_string: &str, name: FileName) -> Result<()> {
// Register the source (`program_string`) in the source map.
let prg_sf = with_session_globals(|s| s.source_map.new_source(program_string, name));
@ -143,23 +139,17 @@ impl<'a> Compiler<'a> {
Ok(())
}
///
/// Runs the symbol table pass.
///
pub fn symbol_table_pass(&self) -> Result<SymbolTable> {
CreateSymbolTable::do_pass((&self.ast, self.handler))
}
///
/// Runs the type checker pass.
///
pub fn type_checker_pass(&'a self, symbol_table: SymbolTable) -> Result<SymbolTable> {
TypeChecker::do_pass((&self.ast, self.handler, symbol_table))
}
///
/// Runs the loop unrolling pass.
///
pub fn loop_unrolling_pass(&mut self, symbol_table: SymbolTable) -> Result<SymbolTable> {
let (ast, symbol_table) = Unroller::do_pass((std::mem::take(&mut self.ast), self.handler, symbol_table))?;
self.ast = ast;
@ -171,9 +161,7 @@ impl<'a> Compiler<'a> {
Ok(symbol_table)
}
///
/// Runs the compiler stages.
///
pub fn compiler_stages(&mut self) -> Result<SymbolTable> {
let st = self.symbol_table_pass()?;
let st = self.type_checker_pass(st)?;
@ -183,17 +171,13 @@ impl<'a> Compiler<'a> {
Ok(st)
}
///
/// Returns a compiled Leo program.
///
pub fn compile(&mut self) -> Result<SymbolTable> {
self.parse_program()?;
self.compiler_stages()
}
///
/// Writes the AST to a JSON file.
///
fn write_ast_to_json(&self, file_name: &str) -> Result<()> {
// Remove `Span`s if they are not enabled.
if self.output_options.spans_enabled {

View File

@ -190,11 +190,11 @@ impl ParserContext<'_> {
/// Returns a [`VariableName`] AST node if the next tokens represent a variable name with
/// valid keywords.
fn parse_variable_name(&mut self, decl_ty: Declare, _span: Span) -> Result<VariableName> {
fn parse_variable_name(&mut self, decl_ty: DeclarationType, _span: Span) -> Result<VariableName> {
let name = self.expect_identifier()?;
Ok(VariableName {
span: name.span,
mutable: matches!(decl_ty, Declare::Let),
mutable: matches!(decl_ty, DeclarationType::Let),
identifier: name,
})
}
@ -204,8 +204,8 @@ impl ParserContext<'_> {
self.expect_any(&[Token::Let, Token::Const])?;
let decl_span = self.prev_token.span;
let decl_type = match &self.prev_token.token {
Token::Let => Declare::Let,
Token::Const => Declare::Const,
Token::Let => DeclarationType::Let,
Token::Const => DeclarationType::Const,
_ => unreachable!("parse_definition_statement_ shouldn't produce this"),
};
// Parse variable names.

View File

@ -18,6 +18,6 @@ use leo_ast::*;
use crate::Unroller;
impl<'a> ExpressionReconstructor for Unroller<'a> {
impl ExpressionReconstructor for Unroller<'_> {
type AdditionalOutput = ();
}

View File

@ -20,7 +20,7 @@ use leo_ast::*;
use crate::Unroller;
impl<'a> ProgramReconstructor for Unroller<'a> {
impl ProgramReconstructor for Unroller<'_> {
fn reconstruct_function(&mut self, function: Function) -> Function {
let function_name = &function.name();

View File

@ -20,16 +20,16 @@ use leo_ast::*;
use leo_errors::FlattenError;
use crate::unroller::Unroller;
use crate::{Declaration, VariableSymbol};
use crate::{VariableType, VariableSymbol};
impl<'a> StatementReconstructor for Unroller<'a> {
impl StatementReconstructor for Unroller<'_> {
fn reconstruct_definition(&mut self, input: DefinitionStatement) -> Statement {
// If we are unrolling a loop, then we need to repopulate the symbol table.
if self.is_unrolling {
let declaration = if input.declaration_type == Declare::Const {
Declaration::Const
let declaration = if input.declaration_type == DeclarationType::Const {
VariableType::Const
} else {
Declaration::Mut
VariableType::Mut
};
input.variable_names.iter().for_each(|v| {
@ -151,7 +151,7 @@ impl<'a> StatementReconstructor for Unroller<'a> {
// The first statement in the block is the assignment of the loop variable to the current iteration count.
let mut statements = vec![
self.reconstruct_definition(DefinitionStatement {
declaration_type: Declare::Const,
declaration_type: DeclarationType::Const,
type_: input.type_.clone(),
value: Expression::Literal(value),
span: Default::default(),

View File

@ -21,13 +21,13 @@ use leo_errors::emitter::Handler;
use crate::SymbolTable;
pub struct Unroller<'a> {
/// the symbol table for the function
/// The symbol table for the function being processed.
pub(crate) symbol_table: RefCell<SymbolTable>,
/// the current block scope index
/// The index of the current block scope.
pub(crate) block_index: usize,
/// error handler
/// An error handler used for any errors found during unrolling.
pub(crate) handler: &'a Handler,
/// A flag indicating whether or not `Unroller` is in the midst of unrolling a loop.
/// Are we in the midst of unrolling a loop?
pub(crate) is_unrolling: bool,
}

View File

@ -19,17 +19,17 @@ use std::fmt::Display;
use leo_ast::{ParamMode, Type};
use leo_span::Span;
/// An enumeration of the different types of declarations.
/// An enumeration of the different types of variable type.
#[derive(Clone, Debug, Eq, PartialEq)]
pub enum Declaration {
pub enum VariableType {
Const,
Input(ParamMode),
Mut,
}
impl Display for Declaration {
impl Display for VariableType {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
use Declaration::*;
use VariableType::*;
match self {
Const => write!(f, "const var"),
@ -47,7 +47,7 @@ pub struct VariableSymbol {
/// The `Span` associated with the variable.
pub span: Span,
/// The type of declaration for the variable.
pub declaration: Declaration,
pub declaration: VariableType,
}
impl Display for VariableSymbol {

View File

@ -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/>.
use crate::{Declaration, TypeChecker, VariableSymbol};
use crate::{VariableType, TypeChecker, VariableSymbol};
use leo_ast::*;
use leo_errors::TypeCheckerError;
@ -43,7 +43,7 @@ impl<'a> ProgramVisitor<'a> for TypeChecker<'a> {
VariableSymbol {
type_: input_var.type_.clone(),
span: input_var.identifier.span(),
declaration: Declaration::Input(input_var.mode()),
declaration: VariableType::Input(input_var.mode()),
},
) {
self.handler.emit_err(err);

View File

@ -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/>.
use crate::{Declaration, TypeChecker, VariableSymbol};
use crate::{VariableType, TypeChecker, VariableSymbol};
use leo_ast::*;
use leo_errors::TypeCheckerError;
@ -35,10 +35,10 @@ impl<'a> StatementVisitor<'a> for TypeChecker<'a> {
}
fn visit_definition(&mut self, input: &'a DefinitionStatement) {
let declaration = if input.declaration_type == Declare::Const {
Declaration::Const
let declaration = if input.declaration_type == DeclarationType::Const {
VariableType::Const
} else {
Declaration::Mut
VariableType::Mut
};
input.variable_names.iter().for_each(|v| {
@ -71,8 +71,8 @@ impl<'a> StatementVisitor<'a> for TypeChecker<'a> {
let var_type = if let Some(var) = self.symbol_table.borrow_mut().lookup_variable(&var_name.name) {
// TODO: Check where this check is moved to in `improved-flattening`.
match &var.declaration {
Declaration::Const => self.emit_err(TypeCheckerError::cannot_assign_to_const_var(var_name, var.span)),
Declaration::Input(ParamMode::Const) => {
VariableType::Const => self.emit_err(TypeCheckerError::cannot_assign_to_const_var(var_name, var.span)),
VariableType::Input(ParamMode::Const) => {
self.emit_err(TypeCheckerError::cannot_assign_to_const_input(var_name, var.span))
}
_ => {}
@ -117,7 +117,7 @@ impl<'a> StatementVisitor<'a> for TypeChecker<'a> {
VariableSymbol {
type_: input.type_.clone(),
span: input.span(),
declaration: Declaration::Const,
declaration: VariableType::Const,
},
) {
self.handler.emit_err(err);

View File

@ -69,7 +69,7 @@ create_messages!(
non_const_loop_bounds {
args: (pos: impl Display),
msg: format!(
"The loop has an `{pos}` bound that is non_const.",
"The loop has a `{pos}` bound that is non_const.",
),
help: None,
}

View File

@ -2,4 +2,4 @@
namespace: Compile
expectation: Fail
outputs:
- "Error [EFLA0373004]: The loop has an `stop` bound that is non_const.\n --> compiler-test:7:25\n |\n 7 | for i: u32 in 0u32..COUNT {\n | ^^^^^\n"
- "Error [EFLA0373004]: The loop has a `stop` bound that is non_const.\n --> compiler-test:7:25\n |\n 7 | for i: u32 in 0u32..COUNT {\n | ^^^^^\n"