From 6b04c234328dd90505920262dc675e43f7ab2f96 Mon Sep 17 00:00:00 2001 From: Sean Young Date: Wed, 18 Oct 2023 22:12:48 +0100 Subject: [PATCH] Each call to new_scope() should be matched with leave_scope() (#1562) 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 --- src/codegen/statements/mod.rs | 10 +++++----- src/codegen/yul/statements.rs | 2 +- src/sema/statements.rs | 30 ++++++++++++++---------------- src/sema/symtable.rs | 16 ++++++++-------- src/sema/yul/block.rs | 4 ++-- src/sema/yul/for_loop.rs | 6 +++--- src/sema/yul/functions.rs | 2 +- src/sema/yul/mod.rs | 4 ++-- src/sema/yul/tests/expression.rs | 4 ++-- 9 files changed, 38 insertions(+), 40 deletions(-) diff --git a/src/codegen/statements/mod.rs b/src/codegen/statements/mod.rs index a30607354..4855577be 100644 --- a/src/codegen/statements/mod.rs +++ b/src/codegen/statements/mod.rs @@ -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; @@ -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; @@ -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 @@ -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(); @@ -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, diff --git a/src/codegen/yul/statements.rs b/src/codegen/yul/statements.rs index ed2883c58..b384b6b5e 100644 --- a/src/codegen/yul/statements.rs +++ b/src/codegen/yul/statements.rs @@ -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 { diff --git a/src/sema/statements.rs b/src/sema/statements.rs index 205bcf707..85377664c 100644 --- a/src/sema/statements.rs +++ b/src/sema/statements.rs @@ -421,7 +421,7 @@ fn statement( unchecked, loc, } => { - symtable.new_scope(); + symtable.enter_scope(); let mut reachable = true; let mut context = context.clone(); @@ -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, @@ -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, @@ -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, @@ -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, @@ -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(); @@ -618,7 +618,7 @@ fn statement( )?; } - loops.new_scope(); + loops.enter_scope(); context.enter_loop(); let mut body = Vec::new(); @@ -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(); @@ -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( @@ -2310,8 +2310,6 @@ fn try_catch( } }; - symtable.new_scope(); - let mut args = match &fcall { Expression::ExternalFunctionCall { returns: func_returns, @@ -2360,7 +2358,7 @@ fn try_catch( } }; - symtable.new_scope(); + symtable.enter_scope(); let mut params = Vec::new(); let mut broken = false; @@ -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; @@ -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(); diff --git a/src/sema/symtable.rs b/src/sema/symtable.rs index c8b9c261c..8c8f01cb9 100644 --- a/src/sema/symtable.rs +++ b/src/sema/symtable.rs @@ -83,7 +83,7 @@ struct VarScope(HashMap, Option>); #[derive(Default, Debug, Clone)] pub struct Symtable { pub vars: IndexMap, - names: Vec, + scopes: Vec, pub arguments: Vec>, pub returns: Vec, } @@ -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(), } @@ -137,7 +137,7 @@ impl Symtable { return None; } - self.names + self.scopes .last_mut() .unwrap() .0 @@ -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); } @@ -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 { @@ -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, diff --git a/src/sema/yul/block.rs b/src/sema/yul/block.rs index d14f9e048..c086c0910 100644 --- a/src/sema/yul/block.rs +++ b/src/sema/yul/block.rs @@ -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, diff --git a/src/sema/yul/for_loop.rs b/src/sema/yul/for_loop.rs index 83d912ddc..d39ea10f6 100644 --- a/src/sema/yul/for_loop.rs +++ b/src/sema/yul/for_loop.rs @@ -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, @@ -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, diff --git a/src/sema/yul/functions.rs b/src/sema/yul/functions.rs index f2a1f8198..7b7f8dcd5 100644 --- a/src/sema/yul/functions.rs +++ b/src/sema/yul/functions.rs @@ -44,7 +44,7 @@ impl FunctionsTable { } } - pub fn new_scope(&mut self) { + pub fn enter_scope(&mut self) { self.scopes.push(HashMap::new()); } diff --git a/src/sema/yul/mod.rs b/src/sema/yul/mod.rs index ab4a64cbd..119fa3a7c 100644 --- a/src/sema/yul/mod.rs +++ b/src/sema/yul/mod.rs @@ -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( diff --git a/src/sema/yul/tests/expression.rs b/src/sema/yul/tests/expression.rs index c6141e468..fcb3cb15d 100644 --- a/src/sema/yul/tests/expression.rs +++ b/src/sema/yul/tests/expression.rs @@ -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); @@ -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);