From 73ddd644c1d3e877378f90e13d33360d4cdca949 Mon Sep 17 00:00:00 2001 From: Michael Schimmel Date: Wed, 18 Feb 2026 13:23:08 +0100 Subject: [PATCH] Add redefinition checks for variables Ensure that variables are not redefined within the same scope, both for global and function-local variables. This commit modifies the `define` method in `CompilerScope` to return a `Result` and includes checks for existing definitions. The `Binder`'s logic for handling global variables and function parameters is also updated to incorporate these new checks and return appropriate errors. --- src/ast/compiler/binder.rs | 37 ++++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/src/ast/compiler/binder.rs b/src/ast/compiler/binder.rs index ee16f29..8a4cf8b 100644 --- a/src/ast/compiler/binder.rs +++ b/src/ast/compiler/binder.rs @@ -25,11 +25,14 @@ impl CompilerScope { } } - fn define(&mut self, name: &str, ty: StaticType) -> u32 { + fn define(&mut self, name: &str, ty: StaticType) -> Result { + if self.locals.contains_key(name) { + return Err(format!("Variable '{}' is already defined in this scope level.", name)); + } let slot = self.slot_count; self.locals.insert(name.to_string(), LocalInfo { slot, ty }); self.slot_count += 1; - slot + Ok(slot) } fn resolve(&self, name: &str) -> Option { @@ -135,16 +138,15 @@ impl Binder { // 1. Pre-declare name to support recursion let slot_or_idx = if self.functions.len() == 1 { let mut globals = self.globals.borrow_mut(); - if let Some((idx, _)) = globals.get(name.as_ref()) { - *idx - } else { - let idx = globals.len() as u32; - globals.insert(name.to_string(), (idx, StaticType::Any)); - idx + if globals.contains_key(name.as_ref()) { + return Err(format!("Global variable '{}' is already defined.", name)); } + let idx = globals.len() as u32; + globals.insert(name.to_string(), (idx, StaticType::Any)); + idx } else { let current_fn = self.functions.last_mut().unwrap(); - current_fn.scope.define(name, StaticType::Any) + current_fn.scope.define(name, StaticType::Any)? }; // 2. Bind Value (now 'name' is visible) @@ -206,8 +208,8 @@ impl Binder { { let current_fn = self.functions.last_mut().unwrap(); for param in params { - // For now, lambda params are Any unless we have a way to specify them - current_fn.scope.define(param, StaticType::Any); + // Lambda params must also be unique in their scope + current_fn.scope.define(param, StaticType::Any)?; param_types.push(StaticType::Any); } } @@ -411,4 +413,17 @@ mod tests { panic!("Root should be a Lambda"); } } + + #[test] + fn test_redefinition_error() { + let source = "(do (def x 1) (def x 2))"; + let mut parser = Parser::new(source).unwrap(); + let untyped = parser.parse_expression().unwrap(); + + let globals = Rc::new(RefCell::new(HashMap::new())); + let result = Binder::bind_root(globals, &untyped); + + assert!(result.is_err()); + assert!(result.unwrap_err().contains("already defined")); + } }