From 15a08aa8f7121f8d961a3d31645695b3cb907ba0 Mon Sep 17 00:00:00 2001 From: Greg Shuflin Date: Tue, 19 Oct 2021 19:19:21 -0700 Subject: [PATCH] SymbolTable error refactoring --- Cargo.lock | 7 ++ schala-lang/language/Cargo.toml | 1 + schala-lang/language/src/error.rs | 14 ++++ schala-lang/language/src/schala.rs | 2 +- schala-lang/language/src/symbol_table/mod.rs | 74 ++++++++++--------- .../language/src/symbol_table/resolver.rs | 3 +- schala-lang/language/src/symbol_table/test.rs | 39 +++++++--- 7 files changed, 90 insertions(+), 50 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8ae5c95..2a43692 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -41,6 +41,12 @@ dependencies = [ "nodrop", ] +[[package]] +name = "assert_matches" +version = "1.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9b34d609dfbaf33d6889b2b7106d3ca345eacad44200913df5ba02bfd31d2ba9" + [[package]] name = "autocfg" version = "0.1.6" @@ -831,6 +837,7 @@ dependencies = [ name = "schala-lang" version = "0.1.0" dependencies = [ + "assert_matches", "colored", "derivative", "ena", diff --git a/schala-lang/language/Cargo.toml b/schala-lang/language/Cargo.toml index a7fcefa..994a486 100644 --- a/schala-lang/language/Cargo.toml +++ b/schala-lang/language/Cargo.toml @@ -14,6 +14,7 @@ stopwatch = "0.0.7" derivative = "1.0.3" colored = "1.8" radix_trie = "0.1.5" +assert_matches = "1.5" schala-lang-codegen = { path = "../codegen" } schala-repl = { path = "../../schala-repl" } diff --git a/schala-lang/language/src/error.rs b/schala-lang/language/src/error.rs index a3f3ee4..cf24816 100644 --- a/schala-lang/language/src/error.rs +++ b/schala-lang/language/src/error.rs @@ -1,6 +1,7 @@ use crate::parsing::ParseError; use crate::schala::{SourceReference, Stage}; use crate::tokenizing::{Location, Token, TokenKind}; +use crate::symbol_table::SymbolError; use crate::typechecking::TypeError; pub struct SchalaError { @@ -29,6 +30,19 @@ impl SchalaError { } } + pub(crate) fn from_symbol_table(symbol_errs: Vec) -> Self { + //TODO this could be better + let errors = symbol_errs.into_iter().map(|_symbol_err| Error { + location: None, + text: Some("symbol table error".to_string()), + stage: Stage::Symbols + }).collect(); + Self { + errors, + formatted_parse_error: None + } + } + pub(crate) fn from_string(text: String, stage: Stage) -> Self { Self { formatted_parse_error: None, diff --git a/schala-lang/language/src/schala.rs b/schala-lang/language/src/schala.rs index 83ef697..b19f424 100644 --- a/schala-lang/language/src/schala.rs +++ b/schala-lang/language/src/schala.rs @@ -81,7 +81,7 @@ impl Schala { //Perform all symbol table work self.symbol_table.borrow_mut().process_ast(&ast) - .map_err(|err| SchalaError::from_string(err, Stage::Symbols))?; + .map_err(|err| SchalaError::from_symbol_table(err))?; // Typechecking // TODO typechecking not working diff --git a/schala-lang/language/src/symbol_table/mod.rs b/schala-lang/language/src/symbol_table/mod.rs index 8c77f6a..0310f01 100644 --- a/schala-lang/language/src/symbol_table/mod.rs +++ b/schala-lang/language/src/symbol_table/mod.rs @@ -54,9 +54,11 @@ enum Scope { #[allow(dead_code)] #[derive(Debug, Clone)] -struct DuplicateName { - prev_name: FQSN, - location: Location +pub enum SymbolError { + DuplicateName { + prev_name: FQSN, + location: Location + } } #[allow(dead_code)] @@ -86,10 +88,10 @@ impl NameTable { Self { table: HashMap::new() } } - fn register(&mut self, name: FQSN, spec: NameSpec) -> Result<(), DuplicateName> { + fn register(&mut self, name: FQSN, spec: NameSpec) -> Result<(), SymbolError> { match self.table.entry(name.clone()) { Entry::Occupied(o) => { - Err(DuplicateName { prev_name: name, location: o.get().location }) + Err(SymbolError::DuplicateName { prev_name: name, location: o.get().location }) }, Entry::Vacant(v) => { v.insert(spec); @@ -132,13 +134,13 @@ impl SymbolTable { /// The main entry point into the symbol table. This will traverse the AST in several /// different ways and populate subtables with information that will be used further in the /// compilation process. - pub fn process_ast(&mut self, ast: &ast::AST) -> Result<(), String> { + pub fn process_ast(&mut self, ast: &ast::AST) -> Result<(), Vec> { let errs = self.populate_name_tables(ast); if !errs.is_empty() { - return Err(format!("{:?}", errs)); + return Err(errs); } - self.resolve_symbol_ids(ast)?; + self.resolve_symbol_ids(ast); Ok(()) } @@ -204,22 +206,21 @@ impl SymbolTable { /// Walks the AST, matching the ID of an identifier used in some expression to /// the corresponding Symbol. - fn resolve_symbol_ids(&mut self, ast: &ast::AST) -> Result<(), String> { + fn resolve_symbol_ids(&mut self, ast: &ast::AST) { let mut resolver = resolver::Resolver::new(self); - resolver.resolve(ast)?; - Ok(()) + resolver.resolve(ast); } /// This function traverses the AST and adds symbol table entries for /// constants, functions, types, and modules defined within. This simultaneously /// checks for dupicate definitions (and returns errors if discovered), and sets /// up name tables that will be used by further parts of the compiler - fn populate_name_tables(&mut self, ast: &ast::AST) -> Vec { + fn populate_name_tables(&mut self, ast: &ast::AST) -> Vec { let mut scope_stack = vec![]; self.add_from_scope(ast.statements.as_ref(), &mut scope_stack) } - fn add_from_scope<'a>(&'a mut self, statements: &[Statement], scope_stack: &mut Vec) -> Vec { + fn add_from_scope<'a>(&'a mut self, statements: &[Statement], scope_stack: &mut Vec) -> Vec { let mut errors = vec![]; for statement in statements { @@ -227,34 +228,35 @@ impl SymbolTable { let location = *location; if let Err(err) = self.add_single_statement(kind, location, &scope_stack) { errors.push(err); + } else { // If there's an error with a name, don't recurse into subscopes of that name + let recursive_errs = match kind { + StatementKind::Declaration(Declaration::FuncDecl(signature, body)) => { + let new_scope = Scope::Name(signature.name.clone()); + scope_stack.push(new_scope); + let output = self.add_from_scope(body.as_ref(), scope_stack); + scope_stack.pop(); + output + } + StatementKind::Module(ModuleSpecifier { name, contents }) => { + let new_scope = Scope::Name(name.clone()); + scope_stack.push(new_scope); + let output = self.add_from_scope(contents.as_ref(), scope_stack); + scope_stack.pop(); + output + } + StatementKind::Declaration(Declaration::TypeDecl { name, body, mutable }) => { + self.add_type_members(name, body, mutable, location, scope_stack) + } + _ => vec![] + }; + errors.extend(recursive_errs.into_iter()); } - let recursive_errs = match kind { - StatementKind::Declaration(Declaration::FuncDecl(signature, body)) => { - let new_scope = Scope::Name(signature.name.clone()); - scope_stack.push(new_scope); - let output = self.add_from_scope(body.as_ref(), scope_stack); - scope_stack.pop(); - output - } - StatementKind::Module(ModuleSpecifier { name, contents }) => { - let new_scope = Scope::Name(name.clone()); - scope_stack.push(new_scope); - let output = self.add_from_scope(contents.as_ref(), scope_stack); - scope_stack.pop(); - output - } - StatementKind::Declaration(Declaration::TypeDecl { name, body, mutable }) => { - self.add_type_members(name, body, mutable, location, scope_stack) - } - _ => vec![] - }; - errors.extend(recursive_errs.into_iter()); } errors } - fn add_single_statement(&mut self, kind: &StatementKind, location: Location, scope_stack: &Vec) -> Result<(), DuplicateName> { + fn add_single_statement(&mut self, kind: &StatementKind, location: Location, scope_stack: &Vec) -> Result<(), SymbolError> { match kind { StatementKind::Declaration(Declaration::FuncSig(signature)) => { let fq_function = FQSN::from_scope_stack(scope_stack.as_ref(), signature.name.clone()); @@ -298,7 +300,7 @@ impl SymbolTable { Ok(()) } - fn add_type_members(&mut self, type_name: &TypeSingletonName, type_body: &TypeBody, _mutable: &bool, location: Location, scope_stack: &mut Vec) -> Vec { + fn add_type_members(&mut self, type_name: &TypeSingletonName, type_body: &TypeBody, _mutable: &bool, location: Location, scope_stack: &mut Vec) -> Vec { let mut errors = vec![]; let mut register = |fqsn: FQSN, spec: SymbolSpec| { diff --git a/schala-lang/language/src/symbol_table/resolver.rs b/schala-lang/language/src/symbol_table/resolver.rs index 1b784da..f224dcc 100644 --- a/schala-lang/language/src/symbol_table/resolver.rs +++ b/schala-lang/language/src/symbol_table/resolver.rs @@ -16,9 +16,8 @@ impl<'a> Resolver<'a> { let name_scope_stack: ScopeStack<'a, Rc, FQSNPrefix> = ScopeStack::new(None); Self { symbol_table, name_scope_stack } } - pub fn resolve(&mut self, ast: &AST) -> Result<(), String> { + pub fn resolve(&mut self, ast: &AST) { walk_ast(self, ast); - Ok(()) } fn lookup_name_in_scope(&self, sym_name: &QualifiedName) -> FQSN { diff --git a/schala-lang/language/src/symbol_table/test.rs b/schala-lang/language/src/symbol_table/test.rs index 963812a..28a3843 100644 --- a/schala-lang/language/src/symbol_table/test.rs +++ b/schala-lang/language/src/symbol_table/test.rs @@ -1,8 +1,9 @@ #![cfg(test)] use super::*; +use assert_matches::assert_matches; use crate::util::quick_ast; -fn add_symbols(src: &str) -> (SymbolTable, Result<(), String>) { +fn add_symbols(src: &str) -> (SymbolTable, Result<(), Vec>) { let ast = quick_ast(src); let mut symbol_table = SymbolTable::new(); let result = symbol_table.process_ast(&ast); @@ -41,8 +42,11 @@ fn no_function_definition_duplicates() { fn a() { 3 } "#; let (_, output) = add_symbols(source); - //TODO test for right type of error - output.unwrap_err(); + let errs = output.unwrap_err(); + assert_matches!(&errs[..], [ + SymbolError::DuplicateName { prev_name, ..} + ] if prev_name == &FQSN::from_strs(&["a"]) + ); } #[test] @@ -52,13 +56,16 @@ fn no_variable_definition_duplicates() { let a = 20 let q = 39 let a = 30 + let x = 34 "#; let (_, output) = add_symbols(source); - let _output = output.unwrap_err(); - /* - assert!(output.contains("Duplicate variable definition: a")); - assert!(output.contains("already defined at 2")); - */ + let errs = output.unwrap_err(); + + assert_matches!(&errs[..], [ + SymbolError::DuplicateName { prev_name: pn1, ..}, + SymbolError::DuplicateName { prev_name: pn2, ..} + ] if pn1 == &FQSN::from_strs(&["a"]) && pn2 == &FQSN::from_strs(&["x"]) + ); } #[test] @@ -77,8 +84,11 @@ fn no_variable_definition_duplicates_in_function() { } "#; let (_, output) = add_symbols(source); - let _err = output.unwrap_err(); - //assert!(output.unwrap_err().contains("Duplicate variable definition: x")) + let errs = output.unwrap_err(); + assert_matches!(&errs[..], [ + SymbolError::DuplicateName { prev_name: pn1, ..}, + ] if pn1 == &FQSN::from_strs(&["q", "x"]) + ); } #[test] @@ -183,9 +193,16 @@ fn duplicate_modules() { } module a { + fn sarat() { 39 } fn foo() { 256.1 } } "#; let (_, output) = add_symbols(source); - let _output = output.unwrap_err(); + let errs = output.unwrap_err(); + + assert_matches!(&errs[..], [ + SymbolError::DuplicateName { prev_name: pn1, ..}, + ] if pn1 == &FQSN::from_strs(&["a"]) + ); + }