Skip to content

Commit

Permalink
Each call to new_scope() should be matched with leave_scope() (#1562)
Browse files Browse the repository at this point in the history
The try catch blocks had an additional new_scope() which is incorrect. I
think the extra scope is harmless but let's clean it up.

Rename `new_scope()` to `enter_scope()` as this matches `leave_scope()`
better.

Signed-off-by: Sean Young <[email protected]>
  • Loading branch information
seanyoung authored Oct 18, 2023
1 parent 3b55390 commit 6b04c23
Show file tree
Hide file tree
Showing 9 changed files with 38 additions and 40 deletions.
10 changes: 5 additions & 5 deletions src/codegen/statements/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ pub(crate) fn statement(
cfg.set_basic_block(body);

vartab.new_dirty_tracker();
loops.new_scope(end, cond);
loops.enter_scope(end, cond);

let mut body_reachable = true;

Expand Down Expand Up @@ -335,7 +335,7 @@ pub(crate) fn statement(
cfg.set_basic_block(body);

vartab.new_dirty_tracker();
loops.new_scope(end, cond);
loops.enter_scope(end, cond);

let mut body_reachable = true;

Expand Down Expand Up @@ -397,7 +397,7 @@ pub(crate) fn statement(

cfg.set_basic_block(body_block);

loops.new_scope(
loops.enter_scope(
end_block,
if next.is_none() {
body_block
Expand Down Expand Up @@ -500,7 +500,7 @@ pub(crate) fn statement(
cfg.set_basic_block(body_block);

// continue goes to next
loops.new_scope(end_block, next_block);
loops.enter_scope(end_block, next_block);

vartab.new_dirty_tracker();

Expand Down Expand Up @@ -1024,7 +1024,7 @@ impl LoopScopes {
LoopScopes(Vec::new())
}

pub(crate) fn new_scope(&mut self, break_bb: usize, continue_bb: usize) {
pub(crate) fn enter_scope(&mut self, break_bb: usize, continue_bb: usize) {
self.0.push(LoopScope {
break_bb,
continue_bb,
Expand Down
2 changes: 1 addition & 1 deletion src/codegen/yul/statements.rs
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ fn process_for_block(
);

cfg.set_basic_block(body_block);
loops.new_scope(end_block, next_block);
loops.enter_scope(end_block, next_block);
vartab.new_dirty_tracker();

for stmt in &execution_block.statements {
Expand Down
30 changes: 14 additions & 16 deletions src/sema/statements.rs
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ fn statement(
unchecked,
loc,
} => {
symtable.new_scope();
symtable.enter_scope();
let mut reachable = true;

let mut context = context.clone();
Expand Down Expand Up @@ -495,9 +495,9 @@ fn statement(
used_variable(ns, &expr, symtable);
let cond = expr.cast(&expr.loc(), &Type::Bool, true, ns, diagnostics)?;

symtable.new_scope();
symtable.enter_scope();
let mut body_stmts = Vec::new();
loops.new_scope();
loops.enter_scope();
statement(
body,
&mut body_stmts,
Expand Down Expand Up @@ -527,9 +527,9 @@ fn statement(
used_variable(ns, &expr, symtable);
let cond = expr.cast(&expr.loc(), &Type::Bool, true, ns, diagnostics)?;

symtable.new_scope();
symtable.enter_scope();
let mut body_stmts = Vec::new();
loops.new_scope();
loops.enter_scope();
statement(
body,
&mut body_stmts,
Expand Down Expand Up @@ -559,7 +559,7 @@ fn statement(

let cond = expr.cast(&expr.loc(), &Type::Bool, true, ns, diagnostics)?;

symtable.new_scope();
symtable.enter_scope();
let mut then_stmts = Vec::new();
let mut reachable = statement(
then,
Expand All @@ -574,7 +574,7 @@ fn statement(

let mut else_stmts = Vec::new();
if let Some(stmts) = else_ {
symtable.new_scope();
symtable.enter_scope();
reachable |= statement(
stmts,
&mut else_stmts,
Expand Down Expand Up @@ -602,7 +602,7 @@ fn statement(
Err(())
}
pt::Statement::For(loc, init_stmt, None, next_expr, body_stmt) => {
symtable.new_scope();
symtable.enter_scope();

let mut init = Vec::new();

Expand All @@ -618,7 +618,7 @@ fn statement(
)?;
}

loops.new_scope();
loops.enter_scope();
context.enter_loop();

let mut body = Vec::new();
Expand Down Expand Up @@ -664,7 +664,7 @@ fn statement(
Ok(reachable)
}
pt::Statement::For(loc, init_stmt, Some(cond_expr), next_expr, body_stmt) => {
symtable.new_scope();
symtable.enter_scope();

let mut init = Vec::new();
let mut body = Vec::new();
Expand Down Expand Up @@ -695,7 +695,7 @@ fn statement(
let cond = cond.cast(&cond_expr.loc(), &Type::Bool, true, ns, diagnostics)?;

// continue goes to next, and if that does exist, cond
loops.new_scope();
loops.enter_scope();

let mut body_reachable = match body_stmt {
Some(body_stmt) => statement(
Expand Down Expand Up @@ -2310,8 +2310,6 @@ fn try_catch(
}
};

symtable.new_scope();

let mut args = match &fcall {
Expression::ExternalFunctionCall {
returns: func_returns,
Expand Down Expand Up @@ -2360,7 +2358,7 @@ fn try_catch(
}
};

symtable.new_scope();
symtable.enter_scope();

let mut params = Vec::new();
let mut broken = false;
Expand Down Expand Up @@ -2487,7 +2485,7 @@ fn try_catch(

match clause_stmt {
CatchClause::Simple(_, param, stmt) => {
symtable.new_scope();
symtable.enter_scope();

let mut catch_param = None;
let mut catch_param_pos = None;
Expand Down Expand Up @@ -2598,7 +2596,7 @@ fn try_catch(
));
}

symtable.new_scope();
symtable.enter_scope();

let mut error_pos = None;
let mut error_stmt_resolved = Vec::new();
Expand Down
16 changes: 8 additions & 8 deletions src/sema/symtable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ struct VarScope(HashMap<String, usize>, Option<HashSet<usize>>);
#[derive(Default, Debug, Clone)]
pub struct Symtable {
pub vars: IndexMap<usize, Variable>,
names: Vec<VarScope>,
scopes: Vec<VarScope>,
pub arguments: Vec<Option<usize>>,
pub returns: Vec<usize>,
}
Expand All @@ -92,7 +92,7 @@ impl Symtable {
pub fn new() -> Self {
Symtable {
vars: IndexMap::new(),
names: vec![VarScope(HashMap::new(), None)],
scopes: vec![VarScope(HashMap::new(), None)],
arguments: Vec::new(),
returns: Vec::new(),
}
Expand Down Expand Up @@ -137,7 +137,7 @@ impl Symtable {
return None;
}

self.names
self.scopes
.last_mut()
.unwrap()
.0
Expand Down Expand Up @@ -174,7 +174,7 @@ impl Symtable {
}

pub fn find(&self, name: &str) -> Option<&Variable> {
for scope in self.names.iter().rev() {
for scope in self.scopes.iter().rev() {
if let Some(n) = scope.0.get(name) {
return self.vars.get(n);
}
Expand All @@ -183,12 +183,12 @@ impl Symtable {
None
}

pub fn new_scope(&mut self) {
self.names.push(VarScope(HashMap::new(), None));
pub fn enter_scope(&mut self) {
self.scopes.push(VarScope(HashMap::new(), None));
}

pub fn leave_scope(&mut self) {
self.names.pop();
self.scopes.pop();
}

pub fn get_name(&self, pos: usize) -> &str {
Expand All @@ -214,7 +214,7 @@ impl LoopScopes {
LoopScopes(Vec::new())
}

pub fn new_scope(&mut self) {
pub fn enter_scope(&mut self) {
self.0.push(LoopScope {
no_breaks: 0,
no_continues: 0,
Expand Down
4 changes: 2 additions & 2 deletions src/sema/yul/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ pub fn resolve_yul_block(
symtable: &mut Symtable,
ns: &mut Namespace,
) -> (YulBlock, bool) {
function_table.new_scope();
symtable.new_scope();
function_table.enter_scope();
symtable.enter_scope();

let (body, mut next_reachable) = process_statements(
statements,
Expand Down
6 changes: 3 additions & 3 deletions src/sema/yul/for_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ pub(crate) fn resolve_for_loop(
function_table: &mut FunctionsTable,
ns: &mut Namespace,
) -> Result<(YulStatement, bool), ()> {
symtable.new_scope();
function_table.new_scope();
symtable.enter_scope();
function_table.enter_scope();
let mut next_reachable = reachable;
let resolved_init_block = resolve_for_init_block(
&yul_for.init_block,
Expand All @@ -40,7 +40,7 @@ pub(crate) fn resolve_for_loop(
let resolved_cond =
resolve_condition(&yul_for.condition, context, symtable, function_table, ns)?;

loop_scope.new_scope();
loop_scope.enter_scope();

let resolved_exec_block = resolve_yul_block(
&yul_for.execution_block.loc,
Expand Down
2 changes: 1 addition & 1 deletion src/sema/yul/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ impl FunctionsTable {
}
}

pub fn new_scope(&mut self) {
pub fn enter_scope(&mut self) {
self.scopes.push(HashMap::new());
}

Expand Down
4 changes: 2 additions & 2 deletions src/sema/yul/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ pub fn resolve_inline_assembly(
) -> (InlineAssembly, bool) {
let start = ns.yul_functions.len();
let mut functions_table = FunctionsTable::new(start);
functions_table.new_scope();
symtable.new_scope();
functions_table.enter_scope();
symtable.enter_scope();
let mut loop_scope = LoopScopes::new();

let (body, reachable) = process_statements(
Expand Down
4 changes: 2 additions & 2 deletions src/sema/yul/tests/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ fn function_call() {
let context = ExprContext::default();
let mut symtable = Symtable::new();
let mut function_table = FunctionsTable::new(0);
function_table.new_scope();
function_table.enter_scope();
let mut ns = Namespace::new(Target::EVM);
let loc = Loc::File(0, 2, 3);

Expand Down Expand Up @@ -639,7 +639,7 @@ fn check_arguments() {
let context = ExprContext::default();
let mut symtable = Symtable::new();
let mut function_table = FunctionsTable::new(0);
function_table.new_scope();
function_table.enter_scope();
let mut ns = Namespace::new(Target::EVM);
let loc = Loc::File(0, 2, 3);

Expand Down

0 comments on commit 6b04c23

Please sign in to comment.