Skip to content

Commit

Permalink
Improve module system (#114)
Browse files Browse the repository at this point in the history
* Fix linker bug where input and output are the same file

* wip

* fix

* remove namespace

* Fix parser

* Fix impl_scope

* Add implicit import

* Add String.an

* address comment

* Add back import shadowing error

* Address comments
  • Loading branch information
0b01 authored Jun 29, 2022
1 parent 7210c63 commit d76e09b
Show file tree
Hide file tree
Showing 17 changed files with 154 additions and 55 deletions.
9 changes: 9 additions & 0 deletions examples/codegen/implicit_import.an
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
v = mut Vec.empty ()

Vec.push v 3i32
Vec.push v 4

print v

// args: --delete-binary
// expected stdout: [3, 4]
2 changes: 1 addition & 1 deletion examples/codegen/vec_basic.an
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import Vec
import Vec.empty push

v = mut empty ()

Expand Down
4 changes: 2 additions & 2 deletions examples/nameresolution/errors.an
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ a = 3 // already declared
// examples/nameresolution/errors.an:14:1 note: a previously defined here
// a = 2
//
// examples/nameresolution/errors.an:4:14 error: No declaration for is_an_error was found in scope
// examples/nameresolution/errors.an:4:14 error: No declaration for `is_an_error` was found in scope
// not_an_error is_an_error
//
// examples/nameresolution/errors.an:6:15 error: No declaration for c was found in scope
// examples/nameresolution/errors.an:6:15 error: No declaration for `c` was found in scope
// fn a b -> a + c + b
//
// examples/nameresolution/errors.an:9:9 warning: c is unused (prefix name with _ to silence this warning)
Expand Down
4 changes: 1 addition & 3 deletions examples/nameresolution/import.an
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ import Library
foo = fn _ b -> b

foo 4 library_int

foo 5 (library_fn ())

// args: --check
// expected stdout:
// expected stdout:
2 changes: 1 addition & 1 deletion examples/nameresolution/unused_warning.an
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ id x = error
// examples/nameresolution/unused_warning.an:2:1 note: id previously defined here
// id x = x
//
// examples/nameresolution/unused_warning.an:4:8 error: No declaration for error was found in scope
// examples/nameresolution/unused_warning.an:4:8 error: No declaration for `error` was found in scope
// id x = error
//
// examples/nameresolution/unused_warning.an:4:4 warning: x is unused (prefix name with _ to silence this warning)
Expand Down
2 changes: 1 addition & 1 deletion src/cache/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ pub struct MutualRecursionSet {
unsafe impl<'c> Send for ModuleCache<'c> {}

/// The key for accessing parse trees or `NameResolver`s
#[derive(Debug, Copy, Clone, Eq, PartialEq)]
#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)]
pub struct ModuleId(pub usize);

#[derive(Copy, Clone, Eq, PartialEq, PartialOrd, Ord, Hash)]
Expand Down
3 changes: 2 additions & 1 deletion src/nameresolution/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use crate::types::{
Field, FunctionType, GeneralizedType, LetBindingLevel, PrimitiveType, Type, TypeInfoBody, PAIR_TYPE, STRING_TYPE,
};

use std::collections::HashSet;
use std::path::PathBuf;

/// The DefinitionInfoId of the `builtin` symbol is defined to be
Expand Down Expand Up @@ -73,7 +74,7 @@ pub fn import_prelude<'a>(resolver: &mut NameResolver, cache: &mut ModuleCache<'
let prelude_dir = prelude_path();
if let Some(id) = declare_module(&prelude_dir, cache, Location::builtin()) {
let exports = define_module(id, cache, Location::builtin()).unwrap();
resolver.current_scope().import(exports, cache, Location::builtin());
resolver.current_scope().import(exports, cache, Location::builtin(), &HashSet::new());
}
}

Expand Down
61 changes: 53 additions & 8 deletions src/nameresolution/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ use crate::util::{fmap, timing, trustme};
use colored::Colorize;

use std::borrow::Cow;
use std::collections::HashMap;
use std::fs::File;
use std::io::{BufReader, Read};
use std::path::{Path, PathBuf};
Expand Down Expand Up @@ -107,6 +108,9 @@ pub struct NameResolver {
/// be defined until the 'define' pass later however.
pub exports: Scope,

/// module scopes to look up definitions and types from
pub module_scopes: HashMap<ModuleId, Scope>,

/// Type variable scopes are separate from other scopes since in general
/// type variables do not follow normal scoping rules. For example, in the trait:
///
Expand Down Expand Up @@ -488,6 +492,19 @@ impl NameResolver {
cache.impl_scopes[self.current_scope().impl_scope.0].push(id);
id
}

fn add_module_scope_and_import_impls<'c>(&mut self, relative_path: &str, location: Location<'c>, cache: &mut ModuleCache<'c>) -> Option<ModuleId> {
if let Some(module_id) = declare_module(Path::new(&relative_path), cache, location) {
self.current_scope().modules.insert(relative_path.to_owned(), module_id);
if let Some(exports) = define_module(module_id, cache, location) {
self.current_scope().import_impls(exports, cache, );
self.module_scopes.insert(module_id, exports.to_owned());
return Some(module_id);
}
}

None
}
}

impl<'c> NameResolver {
Expand Down Expand Up @@ -522,6 +539,7 @@ impl<'c> NameResolver {
cache.modules.insert(filepath.to_owned(), module_id);

let mut resolver = NameResolver {
module_scopes: HashMap::new(),
filepath: filepath.to_owned(),
scopes: vec![FunctionScopes::new()],
exports: Scope::new(cache),
Expand Down Expand Up @@ -754,7 +772,7 @@ impl<'c> Resolvable<'c> for Ast<'c> {
}

impl<'c> Resolvable<'c> for ast::Literal<'c> {
/// Purpose of the declare pass is to collect all the names of publically exported symbols
/// Purpose of the declare pass is to collect all the names of publicly exported symbols
/// so the define pass can work in the presense of mutually recursive modules.
fn declare(&mut self, _: &mut NameResolver, _: &mut ModuleCache) {}

Expand All @@ -781,6 +799,7 @@ impl<'c> Resolvable<'c> for ast::Variable<'c> {
let id = resolver.push_definition(&name, cache, self.location);
resolver.definitions_collected.push(id);
self.definition = Some(id);

} else {
self.definition = resolver.reference_definition(&name, self.location, cache);
}
Expand All @@ -802,14 +821,34 @@ impl<'c> Resolvable<'c> for ast::Variable<'c> {
TypeConstructor(name) => Cow::Borrowed(name),
};

self.impl_scope = Some(resolver.current_scope().impl_scope);
self.definition = resolver.reference_definition(&name, self.location, cache);
self.id = Some(cache.push_variable(name.into_owned(), self.location));
if self.module_prefix.is_empty() {
self.impl_scope = Some(resolver.current_scope().impl_scope);
self.definition = resolver.reference_definition(&name, self.location, cache);
self.id = Some(cache.push_variable(name.into_owned(), self.location));
}
else {
// resolve module
let relative_path = self.module_prefix.join("/");

let mut module_id = resolver.current_scope().modules.get(&relative_path).copied();
if module_id.is_none() {
module_id = resolver.add_module_scope_and_import_impls(&relative_path, self.location, cache);
}

if let Some(module_id) = module_id {
self.definition = resolver.module_scopes[&module_id].definitions.get(name.as_ref()).copied();
self.impl_scope = Some(resolver.current_scope().impl_scope);
self.id = Some(cache.push_variable(name.into_owned(), self.location));
}
else {
error!(self.location, "Could not find module `{}`", relative_path);
}
}
}

// If it is still not declared, print an error
if self.definition.is_none() {
error!(self.location, "No declaration for {} was found in scope", self);
error!(self.location, "No declaration for `{}` was found in scope", self);
}
}
}
Expand Down Expand Up @@ -1107,7 +1146,7 @@ pub fn declare_module<'a>(path: &Path, cache: &mut ModuleCache<'a>, error_locati
}

pub fn define_module<'a>(
module_id: ModuleId, cache: &mut ModuleCache<'a>, error_location: Location,
module_id: ModuleId, cache: &mut ModuleCache<'a>, error_location: Location
) -> Option<&'a Scope> {
let import = cache.name_resolvers.get_mut(module_id.0).unwrap();
match import.state {
Expand All @@ -1126,15 +1165,21 @@ pub fn define_module<'a>(
}

impl<'c> Resolvable<'c> for ast::Import<'c> {
fn declare(&mut self, _resolver: &mut NameResolver, cache: &mut ModuleCache<'c>) {
fn declare(&mut self, resolver: &mut NameResolver, cache: &mut ModuleCache<'c>) {
let relative_path = self.path.clone().join("/");
self.module_id = declare_module(Path::new(&relative_path), cache, self.location);
if let Some(module_id) = self.module_id {
resolver.current_scope().modules.insert(relative_path, module_id);
}
}

fn define(&mut self, resolver: &mut NameResolver, cache: &mut ModuleCache<'c>) {
if let Some(module_id) = self.module_id {
if let Some(exports) = define_module(module_id, cache, self.location) {
resolver.current_scope().import(exports, cache, self.location);
// import only the imported symbols
resolver.current_scope().import(exports, cache, self.location, &self.symbols);
// add the module scope itself
resolver.module_scopes.insert(module_id, exports.to_owned());
}
}
}
Expand Down
25 changes: 17 additions & 8 deletions src/nameresolution/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@
//! is significant because a type variable's scope is different
//! than the general Scope for other symbols. See the TypeVariableScope
//! struct for more details on this.
use crate::cache::{DefinitionInfoId, ImplInfoId, ImplScopeId, ModuleCache, TraitInfoId};
use crate::error::location::{Locatable, Location};
use crate::cache::{DefinitionInfoId, ImplInfoId, ImplScopeId, ModuleCache, TraitInfoId, ModuleId};
use crate::error::location::{Location, Locatable};
use crate::parser::ast;
use crate::types::{TypeInfoId, TypeVariableId};
use std::collections::HashMap;
use std::collections::{HashMap, HashSet};
use std::rc::Rc;

/// A scope represents all symbols defined in a given scope.
Expand All @@ -28,13 +28,14 @@ use std::rc::Rc;
/// DefinitionInfoId afterward. The main exception are the ImplScopeId
/// keys which can be used to retrieve which impls were in scope for
/// a given variable later during type inference.
#[derive(Debug)]
#[derive(Debug, Clone)]
pub struct Scope {
pub definitions: HashMap<String, DefinitionInfoId>,
pub types: HashMap<String, TypeInfoId>,
pub traits: HashMap<String, TraitInfoId>,
pub impls: HashMap<TraitInfoId, Vec<ImplInfoId>>,
pub impl_scope: ImplScopeId,
pub modules: HashMap<String, ModuleId>,
}

impl Scope {
Expand All @@ -45,6 +46,7 @@ impl Scope {
types: HashMap::new(),
traits: HashMap::new(),
impls: HashMap::new(),
modules: HashMap::new(),
}
}

Expand All @@ -54,9 +56,13 @@ impl Scope {
/// symbols are exported are determined in the "declare" pass. This is because since
/// the other Scope's symbols are mutably added to self, they cannot be easily distinguished
/// from definitions originating in this scope.
pub fn import(&mut self, other: &Scope, cache: &mut ModuleCache, location: Location) {
self.import_definitions_types_and_traits(other, cache, location);
pub fn import(&mut self, other: &Scope, cache: &mut ModuleCache, location: Location, symbols: &HashSet<String>) {
self.import_definitions_types_and_traits(other, cache, location, symbols);
self.import_impls(other, cache);
}

/// Helper for `import` which imports all non-impl symbols.
pub fn import_impls(&mut self, other: &Scope, cache: &mut ModuleCache) {
for (k, v) in other.impls.iter() {
if let Some(existing) = self.impls.get_mut(k) {
existing.append(&mut v.clone());
Expand All @@ -69,11 +75,14 @@ impl Scope {
}
}

/// Helper for `import` which imports all non-impl symbols.
fn import_definitions_types_and_traits(&mut self, other: &Scope, cache: &mut ModuleCache, location: Location) {
fn import_definitions_types_and_traits(&mut self, other: &Scope, cache: &mut ModuleCache, location: Location, symbols: &HashSet<String>) {
macro_rules! merge_table {
( $field:tt , $cache_field:tt , $errors:tt ) => {{
for (k, v) in other.$field.iter() {
if !symbols.is_empty() && !symbols.contains(k) {
continue;
}

if let Some(existing) = self.$field.get(k) {
let prev_loc = cache.$cache_field[existing.0].locate();
let error = make_error!(location, "import shadows previous definition of {}", k);
Expand Down
17 changes: 12 additions & 5 deletions src/parser/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use crate::types::pattern::DecisionTree;
use crate::types::traits::RequiredTrait;
use crate::types::typechecker::TypeBindings;
use crate::types::{self, LetBindingLevel, TypeInfoId};
use std::collections::{BTreeMap, HashMap};
use std::collections::{BTreeMap, HashMap, HashSet};
use std::rc::Rc;

#[derive(Clone, Debug, Eq, PartialOrd, Ord)]
Expand Down Expand Up @@ -64,6 +64,9 @@ pub struct Variable<'a> {
pub kind: VariableKind,
pub location: Location<'a>,

/// module prefix path
pub module_prefix: Vec<String>,

/// A variable's definition is initially undefined.
/// During name resolution, every definition is filled
/// out - becoming Some(id)
Expand Down Expand Up @@ -251,6 +254,7 @@ pub struct Import<'a> {
pub location: Location<'a>,
pub typ: Option<types::Type>,
pub module_id: Option<ModuleId>,
pub symbols: HashSet<String>,
}

/// trait Name arg1 arg2 ... argN -> fundep1 fundep2 ... fundepN
Expand Down Expand Up @@ -446,9 +450,10 @@ impl<'a> Ast<'a> {
Ast::Literal(Literal { kind: LiteralKind::Unit, location, typ: None })
}

pub fn variable(name: String, location: Location<'a>) -> Ast<'a> {
pub fn variable(module_prefix: Vec<String>, name: String, location: Location<'a>) -> Ast<'a> {
Ast::Variable(Variable {
kind: VariableKind::Identifier(name),
module_prefix,
location,
definition: None,
id: None,
Expand All @@ -461,6 +466,7 @@ impl<'a> Ast<'a> {
pub fn operator(operator: Token, location: Location<'a>) -> Ast<'a> {
Ast::Variable(Variable {
kind: VariableKind::Operator(operator),
module_prefix: vec![],
location,
definition: None,
id: None,
Expand All @@ -470,10 +476,11 @@ impl<'a> Ast<'a> {
})
}

pub fn type_constructor(name: String, location: Location<'a>) -> Ast<'a> {
pub fn type_constructor(module_prefix: Vec<String>, name: String, location: Location<'a>) -> Ast<'a> {
Ast::Variable(Variable {
kind: VariableKind::TypeConstructor(name),
location,
module_prefix,
definition: None,
id: None,
impl_scope: None,
Expand Down Expand Up @@ -550,9 +557,9 @@ impl<'a> Ast<'a> {
Ast::TypeAnnotation(TypeAnnotation { lhs: Box::new(lhs), rhs, location, typ: None })
}

pub fn import(path: Vec<String>, location: Location<'a>) -> Ast<'a> {
pub fn import(path: Vec<String>, location: Location<'a>, symbols: HashSet<String>) -> Ast<'a> {
assert!(!path.is_empty());
Ast::Import(Import { path, location, typ: None, module_id: None })
Ast::Import(Import { path, location, typ: None, module_id: None, symbols })
}

pub fn trait_definition(
Expand Down
Loading

0 comments on commit d76e09b

Please sign in to comment.