Skip to content

Commit

Permalink
Merge pull request #28433 from ProvableHQ/shadowing
Browse files Browse the repository at this point in the history
Disallow local variables from shadowing functions, structs, mappings,…
  • Loading branch information
d0cd authored Nov 7, 2024
2 parents 333d3eb + 0033837 commit 1e31a90
Show file tree
Hide file tree
Showing 14 changed files with 140 additions and 63 deletions.
41 changes: 34 additions & 7 deletions compiler/passes/src/common/symbol_table/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,24 @@ pub struct SymbolTable {
impl SymbolTable {
/// Recursively checks if the symbol table contains an entry for the given symbol.
/// Leo does not allow any variable shadowing or overlap between different symbols.
pub fn check_shadowing(&self, location: &Location, is_struct: bool, span: Span) -> Result<()> {
pub fn check_shadowing(
&self,
location: &Location,
program: Option<Symbol>,
is_struct: bool,
span: Span,
) -> Result<()> {
self.check_shadowing_impl(location, is_struct, span)?;
// Even if the current item is not scoped by program, we want a collision
// if there are program-scoped items with the same name, so check that too.
if program.is_some() && location.program.is_none() {
let location2 = Location::new(program, location.name);
self.check_shadowing_impl(&location2, is_struct, span)?;
}
Ok(())
}

fn check_shadowing_impl(&self, location: &Location, is_struct: bool, span: Span) -> Result<()> {
if self.functions.contains_key(location) {
return Err(AstError::shadowed_function(location.name, span).into());
} else if self.structs.get(location).is_some() && !(location.program.is_none() && is_struct) {
Expand All @@ -68,7 +85,11 @@ impl SymbolTable {
} else if self.variables.contains_key(location) {
return Err(AstError::shadowed_variable(location.name, span).into());
}
if let Some(parent) = self.parent.as_ref() { parent.check_shadowing(location, is_struct, span) } else { Ok(()) }
if let Some(parent) = self.parent.as_ref() {
parent.check_shadowing_impl(location, is_struct, span)
} else {
Ok(())
}
}

/// Returns the current scope index.
Expand All @@ -82,7 +103,7 @@ impl SymbolTable {
/// Inserts a function into the symbol table.
pub fn insert_fn(&mut self, location: Location, insert: &Function) -> Result<()> {
let id = self.scope_index();
self.check_shadowing(&location, false, insert.span)?;
self.check_shadowing(&location, None, false, insert.span)?;
self.functions.insert(location, Self::new_function_symbol(id, insert));
self.scopes.push(Default::default());
Ok(())
Expand All @@ -91,7 +112,7 @@ impl SymbolTable {
/// Inserts a struct into the symbol table.
pub fn insert_struct(&mut self, location: Location, insert: &Composite) -> Result<()> {
// Check shadowing.
self.check_shadowing(&location, !insert.is_record, insert.span)?;
self.check_shadowing(&location, None, !insert.is_record, insert.span)?;

if insert.is_record {
// Insert the record into the symbol table.
Expand Down Expand Up @@ -139,8 +160,13 @@ impl SymbolTable {
}

/// Inserts a variable into the symbol table.
pub fn insert_variable(&mut self, location: Location, insert: VariableSymbol) -> Result<()> {
self.check_shadowing(&location, false, insert.span)?;
pub fn insert_variable(
&mut self,
location: Location,
program: Option<Symbol>,
insert: VariableSymbol,
) -> Result<()> {
self.check_shadowing(&location, program, false, insert.span)?;
self.variables.insert(location, insert);
Ok(())
}
Expand Down Expand Up @@ -294,6 +320,7 @@ mod tests {
symbol_table
.insert_variable(
Location::new(Some(Symbol::intern("credits")), Symbol::intern("accounts")),
None,
VariableSymbol { type_: Type::Address, span: Default::default(), declaration: VariableType::Const },
)
.unwrap();
Expand All @@ -308,7 +335,7 @@ mod tests {
})
.unwrap();
symbol_table
.insert_variable(Location::new(None, Symbol::intern("foo")), VariableSymbol {
.insert_variable(Location::new(None, Symbol::intern("foo")), None, VariableSymbol {
type_: Type::Address,
span: Default::default(),
declaration: VariableType::Const,
Expand Down
12 changes: 5 additions & 7 deletions compiler/passes/src/loop_unrolling/unroll_statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,11 @@ impl StatementReconstructor for Unroller<'_> {
fn reconstruct_definition(&mut self, input: DefinitionStatement) -> (Statement, Self::AdditionalOutput) {
// Helper function to add variables to symbol table
let insert_variable = |symbol: Symbol, type_: Type, span: Span| {
if let Err(err) =
self.symbol_table.borrow_mut().insert_variable(Location::new(None, symbol), VariableSymbol {
type_,
span,
declaration: VariableType::Mut,
})
{
if let Err(err) = self.symbol_table.borrow_mut().insert_variable(
Location::new(None, symbol),
self.current_program,
VariableSymbol { type_, span, declaration: VariableType::Mut },
) {
self.handler.emit_err(err);
}
};
Expand Down
2 changes: 1 addition & 1 deletion compiler/passes/src/symbol_table_creation/creator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ impl<'a> ProgramVisitor<'a> for SymbolTableCreator<'a> {
};
// Add the variable associated with the mapping to the symbol table.
if let Err(err) =
self.symbol_table.insert_variable(Location::new(program, input.identifier.name), VariableSymbol {
self.symbol_table.insert_variable(Location::new(program, input.identifier.name), None, VariableSymbol {
type_: Type::Mapping(MappingType {
key: Box::new(input.key_type.clone()),
value: Box::new(input.value_type.clone()),
Expand Down
24 changes: 10 additions & 14 deletions compiler/passes/src/type_checking/check_statements.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,13 +224,11 @@ impl<'a, N: Network> StatementVisitor<'a> for TypeChecker<'a, N> {
self.visit_expression(&input.value, &Some(input.type_.clone()));

// Add constants to symbol table so that any references to them in later statements will pass TC
if let Err(err) =
self.symbol_table.borrow_mut().insert_variable(Location::new(None, input.place.name), VariableSymbol {
type_: input.type_.clone(),
span: input.place.span,
declaration: VariableType::Const,
})
{
if let Err(err) = self.symbol_table.borrow_mut().insert_variable(
Location::new(None, input.place.name),
self.scope_state.program_name,
VariableSymbol { type_: input.type_.clone(), span: input.place.span, declaration: VariableType::Const },
) {
self.handler.emit_err(err);
}
}
Expand Down Expand Up @@ -322,13 +320,11 @@ impl<'a, N: Network> StatementVisitor<'a> for TypeChecker<'a, N> {
let scope_index = self.create_child_scope();

// Add the loop variable to the scope of the loop body.
if let Err(err) =
self.symbol_table.borrow_mut().insert_variable(Location::new(None, input.variable.name), VariableSymbol {
type_: input.type_.clone(),
span: input.span(),
declaration: VariableType::Const,
})
{
if let Err(err) = self.symbol_table.borrow_mut().insert_variable(
Location::new(None, input.variable.name),
self.scope_state.program_name,
VariableSymbol { type_: input.type_.clone(), span: input.span(), declaration: VariableType::Const },
) {
self.handler.emit_err(err);
}

Expand Down
14 changes: 7 additions & 7 deletions compiler/passes/src/type_checking/checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1159,6 +1159,7 @@ impl<'a, N: Network> TypeChecker<'a, N> {
// Insert to symbol table
if let Err(err) = self.symbol_table.borrow_mut().insert_variable(
Location::new(None, t1.identifier().name),
self.scope_state.program_name,
VariableSymbol {
type_: t2.clone(),
span: t1.identifier.span(),
Expand Down Expand Up @@ -1230,6 +1231,7 @@ impl<'a, N: Network> TypeChecker<'a, N> {
if !matches!(&input_var.type_(), &Type::Future(_)) {
if let Err(err) = self.symbol_table.borrow_mut().insert_variable(
Location::new(None, input_var.identifier().name),
self.scope_state.program_name,
VariableSymbol {
type_: input_var.type_().clone(),
span: input_var.identifier().span(),
Expand Down Expand Up @@ -1359,13 +1361,11 @@ impl<'a, N: Network> TypeChecker<'a, N> {
type_
};
// Insert the variable into the symbol table.
if let Err(err) =
self.symbol_table.borrow_mut().insert_variable(Location::new(None, name.name), VariableSymbol {
type_: ty,
span,
declaration: VariableType::Mut,
})
{
if let Err(err) = self.symbol_table.borrow_mut().insert_variable(
Location::new(None, name.name),
self.scope_state.program_name,
VariableSymbol { type_: ty, span, declaration: VariableType::Mut },
) {
self.handler.emit_err(err);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ outputs = ["""
Error [ETYC0372110]: A `transition` cannot return a future.
--> compiler-test:5:35
|
5 | transition a(a: u64, b: u64) -> Future {
5 | transition f(a: u64, b: u64) -> Future {
| ^^^^^^
|
= Use an `async transition` instead.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
namespace = "Compile"
expectation = "Fail"
outputs = ["""
Error [EAST0372006]: function `f1` shadowed by
--> compiler-test:9:13
|
9 | let f1: u8 = 1u8;
| ^^
Error [ETYC0372005]: Unknown variable `f1`
--> compiler-test:10:16
|
10 | return f1;
| ^^
"""]
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
namespace = "Compile"
expectation = "Fail"
outputs = ["""
Error [EAST0372009]: variable `m` shadowed by
--> compiler-test:7:13
|
7 | let m: u8 = 1u8;
| ^
Error [ETYC0372003]: Expected type `u8` but type `(u8 => u8)` was found
--> compiler-test:8:16
|
8 | return m;
| ^
"""]
Loading

0 comments on commit 1e31a90

Please sign in to comment.