SymbolTable error refactoring

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",

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" }

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<SymbolError>) -> 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,
@@ -85,7 +99,7 @@ fn format_parse_error(error: ParseError, source_reference: &SourceReference) ->
     let line_num = error.token.location.line_num;
     let ch = error.token.location.char_num;
     let line_from_program = source_reference.get_line(line_num as usize);
-    let location_pointer = format!("{}^", " ".repeat(ch));
+    let location_pointer = format!("{}^", " ".repeat(ch.into()));
 
     let line_num_digits = format!("{}", line_num).chars().count();
     let space_padding = " ".repeat(line_num_digits);

schala-lang/language/src/reduced_ast.rs

@@ -190,11 +190,11 @@ impl<'a> Reducer<'a> {
 
     match spec {
       SymbolSpec::RecordConstructor { .. } => Expr::ReductionError(format!("AST reducer doesn't expect a RecordConstructor here")),
-      SymbolSpec::DataConstructor { index, type_args, type_name } => Expr::Constructor {
+      SymbolSpec::DataConstructor { index, arity, type_name } => Expr::Constructor {
         type_name: type_name.clone(),
         name: local_name.clone(),
         tag: index.clone(),
-        arity: type_args.len(),
+        arity: *arity,
       },
       SymbolSpec::Func(_) => Expr::Sym(local_name.clone()),
       SymbolSpec::Binding => Expr::Sym(local_name.clone()), //TODO not sure if this is right, probably needs to eventually be fqsn

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

schala-lang/language/src/scope_resolution.rs

@@ -1,119 +0,0 @@
-use std::rc::Rc;
-
-use crate::schala::SymbolTableHandle;
-use crate::symbol_table::{ScopeSegment, FullyQualifiedSymbolName};
-use crate::ast::*;
-use crate::util::ScopeStack;
-
-type FQSNPrefix = Vec<ScopeSegment>;
-
-pub struct ScopeResolver<'a> {
-  symbol_table_handle: SymbolTableHandle,
-  name_scope_stack: ScopeStack<'a, Rc<String>, FQSNPrefix>,
-}
-
-impl<'a> ASTVisitor for ScopeResolver<'a> {
-  //TODO need to un-insert these - maybe need to rethink visitor
-  fn import(&mut self, import_spec: &ImportSpecifier) {
-    let ref symbol_table = self.symbol_table_handle.borrow();
-    let ImportSpecifier { ref path_components, ref imported_names, .. } = &import_spec;
-    match imported_names {
-      ImportedNames::All => {
-        let prefix = FullyQualifiedSymbolName(path_components.iter().map(|c| ScopeSegment {
-          name: c.clone(),
-        }).collect());
-        let members = symbol_table.lookup_children_of_fqsn(&prefix);
-        for member in members.into_iter() {
-          let local_name = member.0.last().unwrap().name.clone();
-          self.name_scope_stack.insert(local_name.clone(), member.0);
-        }
-      },
-      ImportedNames::LastOfPath => {
-        let name = path_components.last().unwrap(); //TODO handle better
-        let fqsn_prefix = path_components.iter().map(|c| ScopeSegment {
-          name: c.clone(),
-        }).collect();
-        self.name_scope_stack.insert(name.clone(), fqsn_prefix);
-      }
-      ImportedNames::List(ref names) => {
-        let fqsn_prefix: FQSNPrefix = path_components.iter().map(|c| ScopeSegment {
-          name: c.clone(),
-        }).collect();
-        for name in names.iter() {
-          self.name_scope_stack.insert(name.clone(), fqsn_prefix.clone());
-        }
-      }
-    };
-  }
-
-  fn qualified_name(&mut self, qualified_name: &QualifiedName) {
-    let ref mut symbol_table = self.symbol_table_handle.borrow_mut();
-    let fqsn = self.lookup_name_in_scope(&qualified_name);
-    let ref id = qualified_name.id;
-    symbol_table.map_id_to_fqsn(id, fqsn);
-  }
-
-  fn named_struct(&mut self, name: &QualifiedName, _fields: &Vec<(Rc<String>, Expression)>) {
-    let ref mut symbol_table = self.symbol_table_handle.borrow_mut();
-    let ref id = name.id;
-    let fqsn = self.lookup_name_in_scope(&name);
-    symbol_table.map_id_to_fqsn(id, fqsn);
-  }
-
-  fn pattern(&mut self, pat: &Pattern) {
-    use Pattern::*;
-    match pat {
-      Ignored => (),
-      TuplePattern(_) => (),
-      Literal(_) => (),
-      TupleStruct(name, _) => self.qualified_name_in_pattern(name),
-      Record(name, _) => self.qualified_name_in_pattern(name),
-      VarOrName(name) => self.qualified_name_in_pattern(name),
-    };
-  }
-}
-
-impl<'a> ScopeResolver<'a> {
-  pub fn new(symbol_table_handle: SymbolTableHandle) -> ScopeResolver<'static> {
-    let name_scope_stack = ScopeStack::new(None);
-    ScopeResolver { symbol_table_handle, name_scope_stack }
-  }
-  pub fn resolve(&mut self, ast: &mut AST) -> Result<(), String> {
-    walk_ast(self, ast);
-    Ok(())
-  }
-
-  fn lookup_name_in_scope(&self, sym_name: &QualifiedName) -> FullyQualifiedSymbolName {
-    let QualifiedName { components, .. } = sym_name;
-    let first_component = &components[0];
-    match self.name_scope_stack.lookup(first_component) {
-      None => {
-        FullyQualifiedSymbolName(components.iter().map(|name| ScopeSegment { name: name.clone() }).collect())
-      },
-      Some(fqsn_prefix) => {
-        let mut full_name = fqsn_prefix.clone();
-        let rest_of_name: FQSNPrefix = components[1..].iter().map(|name| ScopeSegment { name: name.clone() }).collect();
-        full_name.extend_from_slice(&rest_of_name);
-        FullyQualifiedSymbolName(full_name)
-      }
-    }
-  }
-
-  /// this might be a variable or a pattern. if a variable, set to none
-  fn qualified_name_in_pattern(&mut self, qualified_name: &QualifiedName) {
-    let ref mut symbol_table = self.symbol_table_handle.borrow_mut();
-    let ref id = qualified_name.id;
-    let fqsn = self.lookup_name_in_scope(qualified_name);
-    if symbol_table.lookup_by_fqsn(&fqsn).is_some() {
-      symbol_table.map_id_to_fqsn(&id, fqsn);
-    }
-  }
-}
-
-#[cfg(test)]
-mod tests {
-  #[test]
-  fn basic_scope() {
-
-  }
-}

schala-lang/language/src/symbol_table/mod.rs

@@ -2,7 +2,6 @@ use std::collections::HashMap;
 use std::collections::hash_map::Entry;
 use std::rc::Rc;
 use std::fmt;
-use std::fmt::Write;
 
 use crate::tokenizing::Location;
 use crate::ast;
@@ -23,7 +22,7 @@ pub struct FQSN {
 }
 
 impl FQSN {
-  fn from_scope_stack(scopes: &[Scope], new_name: String) -> Self {
+  fn from_scope_stack(scopes: &[Scope], new_name: Rc<String>) -> Self {
     let mut v = Vec::new();
     for s in scopes {
       v.push(s.clone());
@@ -31,20 +30,35 @@ impl FQSN {
     v.push(Scope::Name(new_name));
     FQSN { scopes: v }
   }
+
+  #[cfg(test)]
+  fn from_strs(strs: &[&str]) -> FQSN {
+    let mut scopes = vec![];
+    for s in strs {
+      scopes.push(Scope::Name(Rc::new(s.to_string())));
+    }
+    FQSN {
+      scopes
+    }
+  }
 }
 
+
+
 //TODO eventually this should use ItemId's to avoid String-cloning
 /// One segment within a scope.
 #[derive(Debug, Clone, Eq, PartialEq, Hash, PartialOrd, Ord)]
 enum Scope {
-  Name(String)
+  Name(Rc<String>)
 }
 
 #[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)]
@@ -74,10 +88,10 @@ impl<K> NameTable<K> {
     Self { table: HashMap::new() }
   }
 
-  fn register(&mut self, name: FQSN, spec: NameSpec<K>) -> Result<(), DuplicateName> {
+  fn register(&mut self, name: FQSN, spec: NameSpec<K>) -> 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);
@@ -87,43 +101,8 @@ impl<K> NameTable<K> {
   }
 }
 
-#[derive(PartialEq, Eq, Hash, Debug, Clone, PartialOrd, Ord)]
-pub struct FullyQualifiedSymbolName(pub Vec<ScopeSegment>);
-
-impl fmt::Display for FullyQualifiedSymbolName {
-  fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-    let FullyQualifiedSymbolName(v) = self;
-    for segment in v {
-      write!(f, "::{}", segment)?;
-    }
-    Ok(())
-  }
-}
-
-#[derive(Debug, Clone, Eq, PartialEq, Hash, PartialOrd, Ord)]
-pub struct ScopeSegment {
-  pub name: Rc<String>, //TODO maybe this could be a &str, for efficiency?
-}
-
-impl fmt::Display for ScopeSegment {
-  fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-    let kind = ""; //TODO implement some kind of kind-tracking here
-    write!(f, "{}{}", self.name, kind)
-  }
-}
-
-impl ScopeSegment {
-  #[allow(dead_code)]
-  pub fn new(name: Rc<String>) ->  ScopeSegment {
-    ScopeSegment { name }
-  }
-}
-
 //cf. p. 150 or so of Language Implementation Patterns
 pub struct SymbolTable {
-  symbol_path_to_symbol: HashMap<FullyQualifiedSymbolName, Symbol>,
-
-
   /// Used for import resolution.
   symbol_trie: SymbolTrie,
 
@@ -144,9 +123,7 @@ pub struct SymbolTable {
 impl SymbolTable {
   pub fn new() -> SymbolTable {
     SymbolTable {
-      symbol_path_to_symbol: HashMap::new(),
       symbol_trie: SymbolTrie::new(),
-
       fq_names: NameTable::new(),
       types: NameTable::new(),
       id_to_fqsn: HashMap::new(),
@@ -157,10 +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<SymbolError>> {
 
-    self.populate_name_tables(ast)?;
-    self.resolve_symbol_ids(ast)?;
+    let errs = self.populate_name_tables(ast);
+    if !errs.is_empty() {
+      return Err(errs);
+    }
+    self.resolve_symbol_ids(ast);
     Ok(())
   }
 
@@ -189,8 +169,8 @@ pub enum SymbolSpec {
   Func(Vec<TypeName>),
   DataConstructor {
     index: usize,
+    arity: usize,
     type_name: TypeName, //TODO this eventually needs to be some kind of ID
-    type_args: Vec<Rc<String>>, //TODO this should be a lookup table into type information, it's not the concern of the symbol table
   },
   RecordConstructor {
     index: usize,
@@ -205,7 +185,7 @@ impl fmt::Display for SymbolSpec {
     use self::SymbolSpec::*;
     match self {
       Func(type_names) => write!(f, "Func({:?})", type_names),
-      DataConstructor { index, type_name, type_args } => write!(f, "DataConstructor(idx: {})({:?} -> {})", index, type_args, type_name),
+      DataConstructor { index, type_name, arity } => write!(f, "DataConstructor(idx: {}, arity: {}, type: {})", index, arity, type_name),
       RecordConstructor { type_name, index, ..} => write!(f, "RecordConstructor(idx: {})(<members> -> {})", index, type_name),
       Binding => write!(f, "Binding"),
     }
@@ -217,41 +197,69 @@ impl SymbolTable {
   /* note: this adds names for *forward reference* but doesn't actually create any types. solve that problem
    * later */
 
+  /// Register a new mapping of a fully-qualified symbol name (e.g. `Option::Some`)
+  /// to a Symbol, a descriptor of what that name refers to.
+  fn add_symbol(&mut self, fqsn: FQSN, symbol: Symbol) {
+    self.symbol_trie.insert(&fqsn);
+    self.fqsn_to_symbol.insert(fqsn, symbol);
+  }
+
   /// 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) -> Result<(), String> {
+  fn populate_name_tables(&mut self, ast: &ast::AST) -> Vec<SymbolError> {
     let mut scope_stack = vec![];
     self.add_from_scope(ast.statements.as_ref(), &mut scope_stack)
-      .map_err(|err| format!("{:?}", err))?;
-    Ok(())
   }
 
-  fn add_symbol(&mut self, fqsn: FQSN, symbol: Symbol) {
-    self.symbol_trie.insert(&fqsn);
-    self.fqsn_to_symbol.insert(fqsn, symbol);
+  fn add_from_scope<'a>(&'a mut self, statements: &[Statement], scope_stack: &mut Vec<Scope>) -> Vec<SymbolError> {
+    let mut errors = vec![];
 
-  }
-
-  //TODO this should probably return a vector of duplicate name errors
-  fn add_from_scope<'a>(&'a mut self, statements: &[Statement], scope_stack: &mut Vec<Scope>) -> Result<(), DuplicateName> {
     for statement in statements {
-      //TODO I'm not sure if I need to do anything with this ID
-      let Statement { id: _, kind, location } = statement;
+      let Statement { id: _, kind, location } = statement;  //TODO I'm not sure if I need to do anything with this ID
       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());
+      }
+    }
+
+    errors
+  }
+
+  fn add_single_statement(&mut self, kind: &StatementKind, location: Location, scope_stack: &Vec<Scope>) -> Result<(), SymbolError> {
       match kind {
         StatementKind::Declaration(Declaration::FuncSig(signature)) => {
-          let fn_name: String = signature.name.as_str().to_owned();
-          let fq_function = FQSN::from_scope_stack(scope_stack.as_ref(), fn_name);
+          let fq_function = FQSN::from_scope_stack(scope_stack.as_ref(), signature.name.clone());
           self.fq_names.register(fq_function.clone(), NameSpec { location, kind: NameKind::Function })?;
           self.types.register(fq_function.clone(), NameSpec { location, kind: TypeKind } )?;
 
@@ -260,10 +268,9 @@ impl SymbolTable {
             spec: SymbolSpec::Func(vec![]), //TODO does this inner vec need to exist at all?
           });
         }
-        StatementKind::Declaration(Declaration::FuncDecl(signature, body)) => {
-          let fn_name: String = signature.name.as_str().to_owned();
-          let new_scope = Scope::Name(fn_name.clone());
-          let fq_function = FQSN::from_scope_stack(scope_stack.as_ref(), fn_name);
+        StatementKind::Declaration(Declaration::FuncDecl(signature, ..)) => {
+          let fn_name = &signature.name;
+          let fq_function = FQSN::from_scope_stack(scope_stack.as_ref(), fn_name.clone());
           self.fq_names.register(fq_function.clone(), NameSpec { location, kind: NameKind::Function })?;
           self.types.register(fq_function.clone(), NameSpec { location, kind: TypeKind } )?;
 
@@ -271,44 +278,29 @@ impl SymbolTable {
             local_name: signature.name.clone(),
             spec: SymbolSpec::Func(vec![]), //TODO does this inner vec need to exist at all?
           });
-
-          scope_stack.push(new_scope);
-          let output = self.add_from_scope(body.as_ref(), scope_stack);
-          scope_stack.pop();
-          output?
         },
-        StatementKind::Declaration(Declaration::TypeDecl { name, body, mutable }) => {
-          let fq_type = FQSN::from_scope_stack(scope_stack.as_ref(), name.name.as_ref().to_owned());
+        StatementKind::Declaration(Declaration::TypeDecl { name, .. }) => {
+          let fq_type = FQSN::from_scope_stack(scope_stack.as_ref(), name.name.clone());
           self.types.register(fq_type, NameSpec { location, kind: TypeKind } )?;
-          if let Err(errors) = self.add_type_members(name, body, mutable, location, scope_stack) {
-            return Err(errors[0].clone());
-          }
         },
         StatementKind::Declaration(Declaration::Binding { name, .. }) => {
-          let fq_binding = FQSN::from_scope_stack(scope_stack.as_ref(), name.as_str().to_owned());
+          let fq_binding = FQSN::from_scope_stack(scope_stack.as_ref(), name.clone());
           self.fq_names.register(fq_binding.clone(), NameSpec { location, kind: NameKind::Binding })?;
           self.add_symbol(fq_binding, Symbol {
             local_name: name.clone(),
             spec: SymbolSpec::Binding,
           });
         }
-        StatementKind::Module(ModuleSpecifier { name, contents }) => {
-          let mod_name = name.as_str().to_owned();
-          let fq_module = FQSN::from_scope_stack(scope_stack.as_ref(), mod_name.clone());
-          let new_scope = Scope::Name(mod_name);
+        StatementKind::Module(ModuleSpecifier { name, .. }) => {
+          let fq_module = FQSN::from_scope_stack(scope_stack.as_ref(), name.clone());
           self.fq_names.register(fq_module, NameSpec { location, kind: NameKind::Module })?;
-          scope_stack.push(new_scope);
-          let output = self.add_from_scope(contents.as_ref(), scope_stack);
-          scope_stack.pop();
-          output?
         },
         _ => (),
       }
-    }
-    Ok(())
+      Ok(())
   }
 
-  fn add_type_members(&mut self, type_name: &TypeSingletonName, type_body: &TypeBody, _mutable: &bool, location: Location, scope_stack: &mut Vec<Scope>) -> Result<(), Vec<DuplicateName>> {
+  fn add_type_members(&mut self, type_name: &TypeSingletonName, type_body: &TypeBody, _mutable: &bool, location: Location, scope_stack: &mut Vec<Scope>) -> Vec<SymbolError> {
     let mut errors = vec![];
 
     let mut register = |fqsn: FQSN, spec: SymbolSpec| {
@@ -326,31 +318,31 @@ impl SymbolTable {
     };
 
     let TypeBody(variants) = type_body;
-    let new_scope = Scope::Name(type_name.name.as_ref().to_owned());
+    let new_scope = Scope::Name(type_name.name.clone());
     scope_stack.push(new_scope);
 
     for (index, variant) in variants.iter().enumerate() {
       match variant {
         Variant::UnitStruct(name) => {
-          let fq_name = FQSN::from_scope_stack(scope_stack.as_ref(), name.as_ref().to_owned());
+          let fq_name = FQSN::from_scope_stack(scope_stack.as_ref(), name.clone());
           let spec = SymbolSpec::DataConstructor {
             index,
+            arity: 0,
             type_name: name.clone(),
-            type_args: vec![],
           };
           register(fq_name, spec);
         },
         Variant::TupleStruct(name, items) => {
-          let fq_name = FQSN::from_scope_stack(scope_stack.as_ref(), name.as_ref().to_owned());
+          let fq_name = FQSN::from_scope_stack(scope_stack.as_ref(), name.clone());
           let spec = SymbolSpec::DataConstructor {
             index,
+            arity: items.len(),
             type_name: name.clone(),
-            type_args: items.iter().map(|_| Rc::new("DUMMY_TYPE_ID".to_string())).collect()
           };
           register(fq_name, spec);
         },
         Variant::Record { name, members } => {
-          let fq_name = FQSN::from_scope_stack(scope_stack.as_ref(), name.as_ref().to_owned());
+          let fq_name = FQSN::from_scope_stack(scope_stack.as_ref(), name.clone());
           let spec = SymbolSpec::RecordConstructor {
             index,
             type_name: name.clone(),
@@ -382,22 +374,6 @@ impl SymbolTable {
     }
 
     scope_stack.pop();
-
-    if errors.is_empty() {
-      Ok(())
-    } else {
-      Err(errors)
-    }
-  }
-
-  #[allow(dead_code)]
-  pub fn debug_symbol_table(&self) -> String {
-    let mut output = "Symbol table\n".to_string();
-    let mut sorted_symbols: Vec<(&FullyQualifiedSymbolName, &Symbol)> = self.symbol_path_to_symbol.iter().collect();
-    sorted_symbols.sort_by(|(fqsn, _), (other_fqsn, _)| fqsn.cmp(other_fqsn));
-    for (name, sym) in sorted_symbols.iter() {
-      write!(output, "{} -> {}\n", name, sym).unwrap();
-    }
-    output
+    errors
   }
 }

schala-lang/language/src/symbol_table/resolver.rs

@@ -16,9 +16,8 @@ impl<'a> Resolver<'a> {
     let name_scope_stack: ScopeStack<'a, Rc<String>, 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 {
@@ -28,13 +27,13 @@ impl<'a> Resolver<'a> {
       None => {
         FQSN {
           scopes: components.iter()
-            .map(|name| Scope::Name(name.as_ref().to_owned()))
+            .map(|name| Scope::Name(name.clone()))
             .collect()
         }
       },
       Some(fqsn_prefix) => {
         let mut full_name = fqsn_prefix.clone();
-        let rest_of_name: FQSNPrefix = components[1..].iter().map(|name| Scope::Name(name.as_ref().to_owned())).collect();
+        let rest_of_name: FQSNPrefix = components[1..].iter().map(|name| Scope::Name(name.clone())).collect();
         full_name.extend_from_slice(&rest_of_name);
 
         FQSN {
@@ -61,25 +60,25 @@ impl<'a> ASTVisitor for Resolver<'a> {
     match imported_names {
       ImportedNames::All => {
         let prefix = FQSN {
-          scopes: path_components.iter().map(|c| Scope::Name(c.as_ref().to_owned())).collect()
+          scopes: path_components.iter().map(|c| Scope::Name(c.clone())).collect()
         };
         let members = self.symbol_table.symbol_trie.get_children(&prefix);
         for member in members.into_iter() {
           let Scope::Name(n) = member.scopes.last().unwrap();
-          let local_name = Rc::new(n.clone());
+          let local_name = n.clone();
           self.name_scope_stack.insert(local_name, member.scopes);
         }
       },
       ImportedNames::LastOfPath => {
         let name = path_components.last().unwrap(); //TODO handle better
         let fqsn_prefix = path_components.iter()
-          .map(|c| Scope::Name(c.as_ref().to_owned()))
+          .map(|c| Scope::Name(c.clone()))
           .collect();
         self.name_scope_stack.insert(name.clone(), fqsn_prefix);
       }
       ImportedNames::List(ref names) => {
         let fqsn_prefix: FQSNPrefix = path_components.iter()
-          .map(|c| Scope::Name(c.as_ref().to_owned()))
+          .map(|c| Scope::Name(c.clone()))
           .collect();
         for name in names.iter() {
           self.name_scope_stack.insert(name.clone(), fqsn_prefix.clone());

schala-lang/language/src/symbol_table/symbol_trie.rs

@@ -41,16 +41,10 @@ impl SymbolTrie {
 #[cfg(test)]
 mod test {
   use super::*;
-  use crate::symbol_table::{Scope, FQSN};
+  use crate::symbol_table::FQSN;
 
   fn make_fqsn(strs: &[&str]) -> FQSN {
-    let mut scopes = vec![];
-    for s in strs {
-      scopes.push(Scope::Name(s.to_string()));
-    }
-    FQSN {
-      scopes
-    }
+    FQSN::from_strs(strs)
   }
 
 #[test]

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<SymbolError>>) {
   let ast = quick_ast(src);
   let mut symbol_table = SymbolTable::new();
   let result = symbol_table.process_ast(&ast);
@@ -10,13 +11,7 @@ fn add_symbols(src: &str) -> (SymbolTable, Result<(), String>) {
 }
 
 fn make_fqsn(strs: &[&str]) -> FQSN {
-  let mut scopes = vec![];
-  for s in strs {
-    scopes.push(Scope::Name(s.to_string()));
-  }
-  FQSN {
-    scopes
-  }
+  FQSN::from_strs(strs)
 }
 
 
@@ -25,6 +20,10 @@ fn basic_symbol_table() {
   let src = "let a = 10; fn b() { 20 }";
   let (symbols, _) = add_symbols(src);
 
+  fn make_fqsn(strs: &[&str]) -> FQSN {
+    FQSN::from_strs(strs)
+  }
+
   symbols.fq_names.table.get(&make_fqsn(&["b"])).unwrap();
 
   let src = "type Option<T> = Some(T) | None";
@@ -43,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]
@@ -54,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]
@@ -79,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]
@@ -185,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"])
+  );
+
 }

schala-lang/language/src/tokenizing.rs

@@ -2,12 +2,15 @@ use itertools::Itertools;
 use std::{iter::{Iterator, Peekable}, convert::TryFrom, rc::Rc, fmt};
 use std::convert::TryInto;
 
-pub type LineNumber = u32;
 
+/// A location in a particular source file. Note that the
+/// sizes of the internal unsigned integer types limit
+/// the size of a source file to 2^32 lines of
+/// at most 2^16 characters, which should be plenty big.
 #[derive(Debug, Clone, Copy, PartialEq, Default)]
 pub struct Location {
-  pub(crate) line_num: LineNumber,
-  pub(crate) char_num: usize,
+  pub(crate) line_num: u32,
+  pub(crate) char_num: u16,
 }
 
 impl fmt::Display for Location {
@@ -184,7 +187,7 @@ pub fn tokenize(input: &str) -> Vec<Token> {
       c if is_operator(&c) => handle_operator(c, &mut input),
       unknown => Error(format!("Unexpected character: {}", unknown)),
     };
-    let location = Location { line_num: line_num.try_into().unwrap(), char_num };
+    let location = Location { line_num: line_num.try_into().unwrap(), char_num: char_num.try_into().unwrap() };
     tokens.push(Token { kind: cur_tok_kind, location });
   }
   tokens