From c64f53a050b422d450b782087631cf7bb750a4dc Mon Sep 17 00:00:00 2001 From: greg Date: Sun, 10 Mar 2019 17:02:01 -0700 Subject: [PATCH] Detect duplicate variable declarations correctly Later I'll probably want to make it so that you can explicitly override the value of a declared variable --- schala-lang/language/src/eval.rs | 8 ++- schala-lang/language/src/symbol_table.rs | 82 ++++++++++++++++++++---- 2 files changed, 76 insertions(+), 14 deletions(-) diff --git a/schala-lang/language/src/eval.rs b/schala-lang/language/src/eval.rs index 3be52cd..750dcd4 100644 --- a/schala-lang/language/src/eval.rs +++ b/schala-lang/language/src/eval.rs @@ -483,8 +483,14 @@ impl<'a> State<'a> { _ => unreachable!(), }, SymbolSpec::RecordConstructor { .. } => return Err(format!("This shouldn't be a record!")), + SymbolSpec::Binding => match self.values.lookup(&name) { + Some(Binding { val, .. }) => val.clone(), + None => return Err(format!("Symbol {} exists in symbol table but not in evaluator table", name)) + } }, - /* see if it's an ordinary variable TODO make variables go in symbol table */ + //TODO ideally this should be returning a runtime error if this is ever None, but it's not + //handling all bindings correctly yet + //None => return Err(format!("Couldn't find value {}", name)), None => match self.values.lookup(&name) { Some(Binding { val, .. }) => val.clone(), None => return Err(format!("Couldn't find value {}", name)), diff --git a/schala-lang/language/src/symbol_table.rs b/schala-lang/language/src/symbol_table.rs index 2eb881d..63a1560 100644 --- a/schala-lang/language/src/symbol_table.rs +++ b/schala-lang/language/src/symbol_table.rs @@ -76,7 +76,8 @@ pub enum SymbolSpec { }, RecordConstructor { fields: HashMap, Rc> - } + }, + Binding } impl fmt::Display for SymbolSpec { @@ -86,6 +87,7 @@ impl fmt::Display for SymbolSpec { Func(type_names) => write!(f, "Func({:?})", type_names), DataConstructor { index, type_name, type_args } => write!(f, "DataConstructor(idx: {})({:?} -> {})", index, type_args, type_name), RecordConstructor { fields: _fields } => write!(f, "RecordConstructor( )"), + Binding => write!(f, "Binding"), } } } @@ -95,18 +97,14 @@ impl SymbolTable { * later */ pub fn add_top_level_symbols(&mut self, ast: &ast::AST) -> Result<(), String> { - let mut seen_identifiers = HashMap::new(); let mut scope_name_stack = Vec::new(); - self.add_symbols_from_scope(&ast.0, &mut scope_name_stack, &mut seen_identifiers) + self.add_symbols_from_scope(&ast.0, &mut scope_name_stack) } - fn add_symbols_from_scope<'a>(&'a mut self, - statements: &Vec>, - scope_name_stack: &mut Vec>, - seen_identifiers: &mut SymbolTrackTable) -> Result<(), String> { + fn add_symbols_from_scope<'a>(&'a mut self, statements: &Vec>, scope_name_stack: &mut Vec>) -> Result<(), String> { use self::ast::Declaration::*; - fn check_symbol(table: &mut SymbolTrackTable, name: &Rc) -> Result<(), String> { + fn insert_and_check_duplicate_symbol(table: &mut SymbolTrackTable, name: &Rc) -> Result<(), String> { match table.entry(name.clone()) { Entry::Occupied(o) => { let line_number = o.get(); //TODO make this actually work @@ -120,24 +118,33 @@ impl SymbolTable { } } + let mut seen_identifiers: SymbolTrackTable = HashMap::new(); + for meta in statements.iter() { let statement = meta.node(); if let Statement::Declaration(decl) = statement { match decl { FuncSig(ref signature) => { - check_symbol(seen_identifiers, &signature.name)?; + insert_and_check_duplicate_symbol(&mut seen_identifiers, &signature.name)?; self.add_function_signature(signature, scope_name_stack)? } FuncDecl(ref signature, ref body) => { - check_symbol(seen_identifiers, &signature.name)?; + insert_and_check_duplicate_symbol(&mut seen_identifiers, &signature.name)?; self.add_function_signature(signature, scope_name_stack)?; - let mut subscope_seen_identifiers = HashMap::new(); scope_name_stack.push(signature.name.clone()); - let output = self.add_symbols_from_scope(body, scope_name_stack, &mut subscope_seen_identifiers); + let output = self.add_symbols_from_scope(body, scope_name_stack); let _ = scope_name_stack.pop(); output? }, - TypeDecl { name, body, mutable } => self.add_type_decl(name, body, mutable, scope_name_stack)?, + TypeDecl { name, body, mutable } => { + insert_and_check_duplicate_symbol(&mut seen_identifiers, &name.name)?; + self.add_type_decl(name, body, mutable, scope_name_stack)? + }, + Binding { name, .. } => { + insert_and_check_duplicate_symbol(&mut seen_identifiers, name)?; + let symbol = Symbol { name: name.clone(), spec: SymbolSpec::Binding }; + self.add_new_symbol(name, scope_name_stack, symbol); + } _ => () } } @@ -263,6 +270,55 @@ mod symbol_table_tests { assert!(output.contains("Duplicate")) } + #[test] + fn no_duplicates_2() { + let source = r#" + let a = 20; + let q = 39; + let a = 30; + "#; + let mut symbol_table = SymbolTable::new(); + let ast = quick_ast(source); + let output = symbol_table.add_top_level_symbols(&ast).unwrap_err(); + assert!(output.contains("Duplicate")) + } + + #[test] + fn no_duplicates_3() { + let source = r#" + fn a() { + let a = 20 + let b = 40 + a + b + } + + fn q() { + let x = 30 + let x = 33 + } + "#; + let mut symbol_table = SymbolTable::new(); + let ast = quick_ast(source); + let output = symbol_table.add_top_level_symbols(&ast).unwrap_err(); + assert!(output.contains("Duplicate")) + } + + fn dont_falsely_detect_duplicates() { + let source = r#" + let a = 20; + fn some_func() { + let a = 40; + 77 + } + let q = 39; + "#; + let mut symbol_table = SymbolTable::new(); + let ast = quick_ast(source); + symbol_table.add_top_level_symbols(&ast).unwrap(); + assert!(symbol_table.lookup_by_path(&rc!(a), &vec![]).is_some()); + assert!(symbol_table.lookup_by_path(&rc!(a), &vec![rc!(some_func)]).is_some()); + } + #[test] fn enclosing_scopes() { let source = r#"