More feedback/cleanup

This commit is contained in:
Pranav Gaddamadugu 2022-07-18 11:54:33 -07:00
parent 1b872576b2
commit 8199a1e9d3
9 changed files with 58 additions and 68 deletions

View File

@ -61,30 +61,17 @@ impl<I: LoopBound> Iterator for RangeIterator<I> {
fn next(&mut self) -> Option<Self::Item> {
match self.current {
None => None,
Some(value) => match self.clusivity {
Clusivity::Exclusive => {
if value < self.end {
self.current = Some(value.add(I::one()));
Some(value)
} else if value == self.end {
self.current = None;
None
} else {
None
}
Some(value) if value < self.end => {
self.current = Some(value.add(I::one()));
Some(value)
}
Some(value) => {
self.current = None;
match self.clusivity {
Clusivity::Exclusive => None,
Clusivity::Inclusive => Some(value),
}
Clusivity::Inclusive => {
if value < self.end {
self.current = Some(value.add(I::one()));
Some(value)
} else if value == self.end {
self.current = None;
Some(value)
} else {
None
}
}
},
}
}
}
}

View File

@ -22,12 +22,12 @@ use crate::Unroller;
impl ProgramReconstructor for Unroller<'_> {
fn reconstruct_function(&mut self, function: Function) -> Function {
let function_name = &function.name();
let function_name = function.name();
// Grab our function scope.
let prev_st = std::mem::take(&mut self.symbol_table);
self.symbol_table
.swap(prev_st.borrow().get_fn_scope(function_name).unwrap());
.swap(prev_st.borrow().lookup_fn_scope(function_name).unwrap());
self.symbol_table.borrow_mut().parent = Some(Box::new(prev_st.into_inner()));
// Set our current block scope index to 0
self.block_index = 0;
@ -44,7 +44,7 @@ impl ProgramReconstructor for Unroller<'_> {
// Pop back to parent scope.
let prev_st = *self.symbol_table.borrow_mut().parent.take().unwrap();
self.symbol_table.swap(prev_st.get_fn_scope(function_name).unwrap());
self.symbol_table.swap(prev_st.lookup_fn_scope(function_name).unwrap());
self.symbol_table = RefCell::new(prev_st);
reconstructed_function

View File

@ -71,7 +71,7 @@ impl StatementReconstructor for Unroller<'_> {
}
fn reconstruct_block(&mut self, input: Block) -> Block {
let scope_index = self.get_current_scope_index();
let scope_index = self.current_scope_index();
// Enter the block scope.
self.enter_block_scope(scope_index);

View File

@ -47,7 +47,7 @@ impl<'a> Unroller<'a> {
/// Returns the index of the current scope.
/// Note that if we are in the midst of unrolling an IterationStatement, a new scope is created.
pub(crate) fn get_current_scope_index(&mut self) -> usize {
pub(crate) fn current_scope_index(&mut self) -> usize {
if self.is_unrolling {
self.symbol_table.borrow_mut().insert_block()
} else {
@ -59,14 +59,14 @@ impl<'a> Unroller<'a> {
pub(crate) fn enter_block_scope(&mut self, index: usize) {
let previous_symbol_table = std::mem::take(&mut self.symbol_table);
self.symbol_table
.swap(previous_symbol_table.borrow().get_block_scope(index).unwrap());
.swap(previous_symbol_table.borrow().lookup_scope_by_index(index).unwrap());
self.symbol_table.borrow_mut().parent = Some(Box::new(previous_symbol_table.into_inner()));
}
/// Exits the current block scope.
pub(crate) fn exit_block_scope(&mut self, index: usize) {
let prev_st = *self.symbol_table.borrow_mut().parent.take().unwrap();
self.symbol_table.swap(prev_st.get_block_scope(index).unwrap());
self.symbol_table.swap(prev_st.lookup_scope_by_index(index).unwrap());
self.symbol_table = RefCell::new(prev_st);
}
@ -101,7 +101,7 @@ impl<'a> Unroller<'a> {
};
// Get the index of the current scope.
let scope_index = self.get_current_scope_index();
let scope_index = self.current_scope_index();
// Enter the scope of the loop body.
self.enter_block_scope(scope_index);

View File

@ -102,19 +102,19 @@ impl SymbolTable {
}
/// Attempts to lookup a function in the symbol table.
pub fn lookup_fn(&self, symbol: &Symbol) -> Option<&FunctionSymbol> {
if let Some(func) = self.functions.get(symbol) {
pub fn lookup_fn_symbol(&self, symbol: Symbol) -> Option<&FunctionSymbol> {
if let Some(func) = self.functions.get(&symbol) {
Some(func)
} else if let Some(parent) = self.parent.as_ref() {
parent.lookup_fn(symbol)
parent.lookup_fn_symbol(symbol)
} else {
None
}
}
/// Attempts to lookup a circuit in the symbol table.
pub fn lookup_circuit(&self, symbol: &Symbol) -> Option<&Circuit> {
if let Some(circ) = self.circuits.get(symbol) {
pub fn lookup_circuit(&self, symbol: Symbol) -> Option<&Circuit> {
if let Some(circ) = self.circuits.get(&symbol) {
Some(circ)
} else if let Some(parent) = self.parent.as_ref() {
parent.lookup_circuit(symbol)
@ -124,8 +124,8 @@ impl SymbolTable {
}
/// Attempts to lookup a variable in the symbol table.
pub fn lookup_variable(&self, symbol: &Symbol) -> Option<&VariableSymbol> {
if let Some(var) = self.variables.get(symbol) {
pub fn lookup_variable(&self, symbol: Symbol) -> Option<&VariableSymbol> {
if let Some(var) = self.variables.get(&symbol) {
Some(var)
} else if let Some(parent) = self.parent.as_ref() {
parent.lookup_variable(symbol)
@ -135,14 +135,14 @@ impl SymbolTable {
}
/// Returns true if the variable exists in the local scope
pub fn variable_in_local_scope(&self, symbol: &Symbol) -> bool {
self.variables.contains_key(symbol)
pub fn variable_in_local_scope(&self, symbol: Symbol) -> bool {
self.variables.contains_key(&symbol)
}
/// Returns true if the variable exists in any parent scope
pub fn variable_in_parent_scope(&self, symbol: &Symbol) -> bool {
pub fn variable_in_parent_scope(&self, symbol: Symbol) -> bool {
if let Some(parent) = self.parent.as_ref() {
if parent.variables.contains_key(symbol) {
if parent.variables.contains_key(&symbol) {
true
} else {
parent.variable_in_parent_scope(symbol)
@ -153,8 +153,8 @@ impl SymbolTable {
}
/// Returns a mutable reference to the `VariableSymbol` if it exists in the symbol table.
pub fn lookup_variable_mut(&mut self, symbol: &Symbol) -> Option<&mut VariableSymbol> {
if let Some(var) = self.variables.get_mut(symbol) {
pub fn lookup_variable_mut(&mut self, symbol: Symbol) -> Option<&mut VariableSymbol> {
if let Some(var) = self.variables.get_mut(&symbol) {
Some(var)
} else if let Some(parent) = self.parent.as_mut() {
parent.lookup_variable_mut(symbol)
@ -164,18 +164,12 @@ impl SymbolTable {
}
/// Returns the scope associated with the function symbol, if it exists in the symbol table.
pub fn get_fn_scope(&self, symbol: &Symbol) -> Option<&RefCell<Self>> {
if let Some(func) = self.functions.get(symbol) {
self.scopes.get(func.id)
} else if let Some(parent) = self.parent.as_ref() {
parent.get_fn_scope(symbol)
} else {
None
}
pub fn lookup_fn_scope(&self, symbol: Symbol) -> Option<&RefCell<Self>> {
self.lookup_fn_symbol(symbol).and_then(|func| self.scopes.get(func.id))
}
/// Returns the scope associated with `index`, if it exists in the symbol table.
pub fn get_block_scope(&self, index: usize) -> Option<&RefCell<Self>> {
pub fn lookup_scope_by_index(&self, index: usize) -> Option<&RefCell<Self>> {
self.scopes.get(index)
}
}

View File

@ -126,7 +126,7 @@ impl<'a> ExpressionVisitor<'a> for TypeChecker<'a> {
match self.visit_expression(&access.inner, &None) {
Some(Type::Identifier(identifier)) => {
// Retrieve the circuit definition associated with `identifier`.
let circ = self.symbol_table.borrow().lookup_circuit(&identifier.name).cloned();
let circ = self.symbol_table.borrow().lookup_circuit(identifier.name).cloned();
if let Some(circ) = circ {
// Check that `access.name` is a member of the circuit.
match circ
@ -166,7 +166,7 @@ impl<'a> ExpressionVisitor<'a> for TypeChecker<'a> {
}
fn visit_circuit_init(&mut self, input: &'a CircuitExpression, additional: &Self::AdditionalInput) -> Self::Output {
let circ = self.symbol_table.borrow().lookup_circuit(&input.name.name).cloned();
let circ = self.symbol_table.borrow().lookup_circuit(input.name.name).cloned();
if let Some(circ) = circ {
// Check circuit type name.
let ret = self.check_expected_circuit(circ.identifier, additional, input.name.span());
@ -210,9 +210,9 @@ impl<'a> ExpressionVisitor<'a> for TypeChecker<'a> {
}
fn visit_identifier(&mut self, var: &'a Identifier, expected: &Self::AdditionalInput) -> Self::Output {
if let Some(circuit) = self.symbol_table.borrow().lookup_circuit(&var.name) {
if let Some(circuit) = self.symbol_table.borrow().lookup_circuit(var.name) {
Some(self.assert_and_return_type(Type::Identifier(circuit.identifier), expected, var.span))
} else if let Some(var) = self.symbol_table.borrow().lookup_variable(&var.name) {
} else if let Some(var) = self.symbol_table.borrow().lookup_variable(var.name) {
Some(self.assert_and_return_type(var.type_.clone(), expected, var.span))
} else {
self.emit_err(TypeCheckerError::unknown_sym("variable", var.name, var.span()));
@ -516,7 +516,7 @@ impl<'a> ExpressionVisitor<'a> for TypeChecker<'a> {
Expression::Identifier(ident) => {
// Note: The function symbol lookup is performed outside of the `if let Some(func) ...` block to avoid a RefCell lifetime bug in Rust.
// Do not move it into the `if let Some(func) ...` block or it will keep `self.symbol_table` alive for the entire block and will be very memory inefficient!
let func = self.symbol_table.borrow().lookup_fn(&ident.name).cloned();
let func = self.symbol_table.borrow().lookup_fn_symbol(ident.name).cloned();
if let Some(func) = func {
let ret = self.assert_and_return_type(func.output, expected, func.span);

View File

@ -28,7 +28,7 @@ impl<'a> ProgramVisitor<'a> for TypeChecker<'a> {
fn visit_function(&mut self, input: &'a Function) {
let prev_st = std::mem::take(&mut self.symbol_table);
self.symbol_table
.swap(prev_st.borrow().get_fn_scope(&input.name()).unwrap());
.swap(prev_st.borrow().lookup_fn_scope(input.name()).unwrap());
self.symbol_table.borrow_mut().parent = Some(Box::new(prev_st.into_inner()));
self.has_return = false;
@ -56,7 +56,7 @@ impl<'a> ProgramVisitor<'a> for TypeChecker<'a> {
}
let prev_st = *self.symbol_table.borrow_mut().parent.take().unwrap();
self.symbol_table.swap(prev_st.get_fn_scope(&input.name()).unwrap());
self.symbol_table.swap(prev_st.lookup_fn_scope(input.name()).unwrap());
self.symbol_table = RefCell::new(prev_st);
}

View File

@ -26,7 +26,11 @@ impl<'a> StatementVisitor<'a> for TypeChecker<'a> {
// we can safely unwrap all self.parent instances because
// statements should always have some parent block
let parent = self.parent.unwrap();
let return_type = &self.symbol_table.borrow().lookup_fn(&parent).map(|f| f.output.clone());
let return_type = &self
.symbol_table
.borrow()
.lookup_fn_symbol(parent)
.map(|f| f.output.clone());
self.check_core_type_conflict(return_type);
self.has_return = true;
@ -68,7 +72,7 @@ impl<'a> StatementVisitor<'a> for TypeChecker<'a> {
}
};
let var_type = if let Some(var) = self.symbol_table.borrow_mut().lookup_variable(&var_name.name) {
let var_type = if let Some(var) = self.symbol_table.borrow_mut().lookup_variable(var_name.name) {
match &var.declaration {
VariableType::Const => self.emit_err(TypeCheckerError::cannot_assign_to_const_var(var_name, var.span)),
VariableType::Input(ParamMode::Const) => {
@ -107,7 +111,7 @@ impl<'a> StatementVisitor<'a> for TypeChecker<'a> {
let scope_index = self.symbol_table.borrow_mut().insert_block();
let prev_st = std::mem::take(&mut self.symbol_table);
self.symbol_table
.swap(prev_st.borrow().get_block_scope(scope_index).unwrap());
.swap(prev_st.borrow().lookup_scope_by_index(scope_index).unwrap());
self.symbol_table.borrow_mut().parent = Some(Box::new(prev_st.into_inner()));
// Add the loop variable to the scope of the loop body.
@ -126,7 +130,8 @@ impl<'a> StatementVisitor<'a> for TypeChecker<'a> {
// Restore the previous scope.
let prev_st = *self.symbol_table.borrow_mut().parent.take().unwrap();
self.symbol_table.swap(prev_st.get_block_scope(scope_index).unwrap());
self.symbol_table
.swap(prev_st.lookup_scope_by_index(scope_index).unwrap());
self.symbol_table = RefCell::new(prev_st);
self.visit_expression(&input.start, iter_type);
@ -159,15 +164,19 @@ impl<'a> StatementVisitor<'a> for TypeChecker<'a> {
// Creates a new sub-scope since we are in a block.
let scope_index = self.symbol_table.borrow_mut().insert_block();
let previous_symbol_table = std::mem::take(&mut self.symbol_table);
self.symbol_table
.swap(previous_symbol_table.borrow().get_block_scope(scope_index).unwrap());
self.symbol_table.swap(
previous_symbol_table
.borrow()
.lookup_scope_by_index(scope_index)
.unwrap(),
);
self.symbol_table.borrow_mut().parent = Some(Box::new(previous_symbol_table.into_inner()));
input.statements.iter().for_each(|stmt| self.visit_statement(stmt));
let previous_symbol_table = *self.symbol_table.borrow_mut().parent.take().unwrap();
self.symbol_table
.swap(previous_symbol_table.get_block_scope(scope_index).unwrap());
.swap(previous_symbol_table.lookup_scope_by_index(scope_index).unwrap());
self.symbol_table = RefCell::new(previous_symbol_table);
}
}

View File

@ -173,7 +173,7 @@ impl Package {
InputsDirectory::create(path)?;
// Create the Leo build/ directory
BuildDirectory::create(&path)?;
BuildDirectory::create(path)?;
// Create the input file in the inputs directory.
InputFile::new(package_name).write_to(path)?;