From d86af21c7f78685bfe71bb394fca6d5c4fe43f06 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sat, 7 Dec 2019 00:26:07 -0500 Subject: [PATCH] Remove ExposedImport --- src/can/expr.rs | 6 --- src/can/module.rs | 93 ++++++++++++++++++++++++--------------------- src/can/operator.rs | 2 - src/fmt/expr.rs | 4 -- src/load/mod.rs | 18 +++++++-- src/parse/ast.rs | 3 -- src/parse/mod.rs | 3 -- src/solve.rs | 27 +++++++------ tests/test_load.rs | 8 +--- 9 files changed, 80 insertions(+), 84 deletions(-) diff --git a/src/can/expr.rs b/src/can/expr.rs index a46389970f..a7e30dfe4c 100644 --- a/src/can/expr.rs +++ b/src/can/expr.rs @@ -308,12 +308,6 @@ pub fn canonicalize_expr( (expr, output, constraint) } - ast::Expr::ExposedImport(name) => { - let symbol = scope.symbol(name); - let ident = Ident::new(&[], name); - - canonicalize_lookup(env, scope, ident, symbol, region, expected, var_store) - } ast::Expr::Var(module_parts, name) => { let symbol = if module_parts.is_empty() { scope.symbol(name) diff --git a/src/can/module.rs b/src/can/module.rs index 6b61af403c..b0cb2ddddb 100644 --- a/src/can/module.rs +++ b/src/can/module.rs @@ -3,17 +3,21 @@ use crate::can::env::Env; use crate::can::expr::Output; use crate::can::operator::desugar_def; use crate::can::scope::Scope; -use crate::collections::ImMap; +use crate::can::symbol::Symbol; +use crate::collections::{ImMap, SendMap}; use crate::parse::ast::{self, ExposesEntry}; use crate::region::Located; -use crate::subs::VarStore; +use crate::subs::{VarStore, Variable}; use crate::types::Constraint::{self, *}; +use crate::types::Expected::*; +use crate::types::Type; use bumpalo::Bump; #[derive(Clone, Debug, PartialEq)] pub struct Module { pub name: Option>, pub defs: Vec, + pub exposed_imports: SendMap, pub constraint: Constraint, } @@ -24,10 +28,12 @@ pub fn canonicalize_module_defs<'a, I>( _exposes: I, scope: &mut Scope, var_store: &VarStore, -) -> (Vec, Constraint) +) -> (Vec, SendMap, Constraint) where I: Iterator>>, { + let mut exposed_imports = SendMap::default(); + // Desugar operators (convert them to Apply calls, taking into account // operator precedence and associativity rules), before doing other canonicalization. // @@ -38,46 +44,6 @@ where let mut desugared = bumpalo::collections::Vec::with_capacity_in(loc_defs.len() + scope.idents.len(), arena); - // Exposed values are desugared as defs that appear before any others, e.g. - // - // imports [ Foo.{ bar, baz } ] - // - // ...desugars to these extra defs at the start of the module: - // - // bar = Foo.bar - // baz = Foo.baz - // - // This is the part where we add those defs to the beginning of the module. - for (ident, (symbol, region)) in scope.idents.iter() { - if ident.first_char().is_lowercase() { - // TODO eventually, we should get rid of Expr::ExposedImport and instead: - // - // 1. Move this logic to right before the call to canonicalize_defs() - // 2. Change it to canonicalize the imports directly (since we can skip a bunch of - // unnecessary scope-checking work that canonicalize_def has to do) - // 3. If there's an error, give a message about exposed imports as opposed to - // something confusing referring to a (generaetd) def the author never wrote. - let pattern = ast::Pattern::Identifier(arena.alloc(ident.clone().name())); - let expr = ast::Expr::ExposedImport(arena.alloc(symbol.clone().into_boxed_str())); - let loc_pattern = Located { - value: pattern, - region: region.clone(), - }; - let loc_expr = Located { - value: expr, - region: region.clone(), - }; - let value = ast::Def::Body(arena.alloc(loc_pattern), arena.alloc(loc_expr)); - - desugared.push(&*arena.alloc(Located { - value, - region: region.clone(), - })); - } else { - // TODO add type aliases to type alias dictionary, based on exposed types - } - } - for loc_def in loc_defs { desugared.push(&*arena.alloc(Located { value: desugar_def(arena, arena.alloc(loc_def.value)), @@ -88,6 +54,45 @@ where let mut env = Env::new(home); let rigids = ImMap::default(); let mut flex_info = Info::default(); + + // Exposed values are treated like defs that appear before any others, e.g. + // + // imports [ Foo.{ bar, baz } ] + // + // ...is basically the same as if we'd added these extra defs at the start of the module: + // + // bar = Foo.bar + // baz = Foo.baz + // + // Here we essentially add those "defs" to "the beginning of the module" + // by canonicalizing them right before we canonicalize the actual ast::Def nodes. + for (ident, (symbol, region)) in scope.idents.iter() { + if ident.first_char().is_lowercase() { + let expr_var = var_store.fresh(); + + // Add an entry to exposed_imports using the current module's name + // as the key; e.g. if this is the Foo module and we have + // exposes [ Bar.{ baz } ] then insert Foo.baz as the key, so when + // anything references `baz` in this Foo module, it will resolve to Bar.baz. + exposed_imports.insert(scope.symbol(&*ident.clone().name()), expr_var); + + // Add the usual constraint info as if this were a normal def. + let expr_type = Type::Variable(expr_var); + let expected = NoExpectation(expr_type.clone()); + let loc_type = Located { + region: region.clone(), + value: expr_type, + }; + + flex_info.def_types.insert(symbol.clone(), loc_type); + flex_info + .constraints + .push(Lookup(symbol.clone(), expected, region.clone())); + } else { + // TODO add type aliases to type alias dictionary, based on exposed types + } + } + let defs = canonicalize_defs( &rigids, &mut env, @@ -108,5 +113,5 @@ where // TODO incorporate rigids into here (possibly by making this be a Let instead // of an And) - (defs, And(flex_info.constraints)) + (defs, exposed_imports, And(flex_info.constraints)) } diff --git a/src/can/operator.rs b/src/can/operator.rs index cb4163088c..0ad38404cc 100644 --- a/src/can/operator.rs +++ b/src/can/operator.rs @@ -70,8 +70,6 @@ pub fn desugar_expr<'a>(arena: &'a Bump, loc_expr: &'a Located>) -> &'a | Nested(AccessorFunction(_)) | Var(_, _) | Nested(Var(_, _)) - | ExposedImport(_) - | Nested(ExposedImport(_)) | MalformedIdent(_) | Nested(MalformedIdent(_)) | MalformedClosure diff --git a/src/fmt/expr.rs b/src/fmt/expr.rs index 2b2c279559..082224a3fd 100644 --- a/src/fmt/expr.rs +++ b/src/fmt/expr.rs @@ -40,9 +40,6 @@ pub fn fmt_expr<'a>( buf.push_str(name); } - ExposedImport(name) => { - buf.push_str(name); - } Apply(loc_expr, loc_args, _) => { if apply_needs_parens { buf.push('('); @@ -257,7 +254,6 @@ pub fn is_multiline_expr<'a>(expr: &'a Expr<'a>) -> bool { | Access(_, _) | AccessorFunction(_) | Var(_, _) - | ExposedImport(_) | MalformedIdent(_) | MalformedClosure | GlobalTag(_) diff --git a/src/load/mod.rs b/src/load/mod.rs index d9d32b794b..b98774daa6 100644 --- a/src/load/mod.rs +++ b/src/load/mod.rs @@ -2,7 +2,7 @@ use crate::can::def::Def; use crate::can::module::{canonicalize_module_defs, Module}; use crate::can::scope::Scope; use crate::can::symbol::Symbol; -use crate::collections::{ImMap, SendSet}; +use crate::collections::{ImMap, SendSet, SendMap}; use crate::ident::Ident; use crate::module::ModuleName; use crate::parse::ast::{self, Attempting, ExposesEntry, ImportsEntry}; @@ -213,7 +213,7 @@ async fn load_filename( let mut scope = Scope::new(format!("{}.", declared_name).into(), scope_from_imports); - let (defs, constraint) = parse_and_canonicalize_defs( + let (defs, exposed_imports, constraint) = parse_and_canonicalize_defs( &arena, state, declared_name.clone(), @@ -224,6 +224,7 @@ async fn load_filename( let module = Module { name: Some(declared_name), defs, + exposed_imports, constraint, }; @@ -253,7 +254,7 @@ async fn load_filename( let mut scope = Scope::new(".".into(), scope_from_imports); // The app module has no declared name. Pass it as "". - let (defs, constraint) = parse_and_canonicalize_defs( + let (defs, exposed_imports, constraint) = parse_and_canonicalize_defs( &arena, state, "".into(), @@ -264,6 +265,7 @@ async fn load_filename( let module = Module { name: None, defs, + exposed_imports, constraint, }; @@ -288,7 +290,7 @@ fn parse_and_canonicalize_defs<'a, I>( exposes: I, scope: &mut Scope, var_store: &VarStore, -) -> (Vec, Constraint) +) -> (Vec, SendMap, Constraint) where I: Iterator>>, { @@ -352,6 +354,10 @@ pub fn solve_loaded(module: &Module, subs: &mut Subs, loaded_deps: LoadedDeps) { let mut env: ImMap = ImMap::default(); let mut constraints = Vec::with_capacity(loaded_deps.len() + 1); + for (symbol, var) in module.exposed_imports.iter() { + env.insert(symbol.clone(), var.clone()); + } + // Add each loaded module's top-level defs to the Env, so that when we go // to solve, looking up qualified idents gets the correct answer. // @@ -360,6 +366,10 @@ pub fn solve_loaded(module: &Module, subs: &mut Subs, loaded_deps: LoadedDeps) { for loaded_dep in loaded_deps { match loaded_dep { Valid(valid_dep) => { + for (symbol, var) in valid_dep.exposed_imports { + env.insert(symbol, var); + } + constraints.push(valid_dep.constraint); for def in valid_dep.defs { diff --git a/src/parse/ast.rs b/src/parse/ast.rs index a1425cf7cb..8ed09db6f0 100644 --- a/src/parse/ast.rs +++ b/src/parse/ast.rs @@ -133,9 +133,6 @@ pub enum Expr<'a> { // Lookups Var(&'a [&'a str], &'a str), - /// A lookup added to the top of a module's AST because of an exposed import - /// (e.g. `bar` in `imports [ Foo.{ bar } ]`) - ExposedImport(&'a str), // Tags GlobalTag(&'a str), diff --git a/src/parse/mod.rs b/src/parse/mod.rs index 5f86acb700..a8a24cf630 100644 --- a/src/parse/mod.rs +++ b/src/parse/mod.rs @@ -255,9 +255,6 @@ fn expr_to_pattern<'a>(arena: &'a Bump, expr: &Expr<'a>) -> Result, })) } } - Expr::ExposedImport(_) => { - panic!("epxr_to_pattern was called on a ExposedImport. This should never happen!"); - } Expr::GlobalTag(value) => Ok(Pattern::GlobalTag(value)), Expr::PrivateTag(value) => Ok(Pattern::PrivateTag(value)), Expr::Apply(loc_val, loc_args, _) => { diff --git a/src/solve.rs b/src/solve.rs index 803b795b0d..e752160843 100644 --- a/src/solve.rs +++ b/src/solve.rs @@ -7,7 +7,7 @@ use crate::unify::unify; type Env = ImMap; -pub fn solve(env: &Env, subs: &mut Subs, constraint: &Constraint) { +pub fn solve(vars_by_symbol: &Env, subs: &mut Subs, constraint: &Constraint) { match constraint { True => (), Eq(typ, expected_type, _region) => { @@ -19,10 +19,13 @@ pub fn solve(env: &Env, subs: &mut Subs, constraint: &Constraint) { } Lookup(symbol, expected_type, _region) => { // TODO use region? - let actual = subs.copy_var(*env.get(&symbol).unwrap_or_else(|| { + let actual = subs.copy_var(*vars_by_symbol.get(&symbol).unwrap_or_else(|| { // TODO Instead of panicking, solve this as True and record // a Problem ("module Foo does not expose `bar`") for later. - panic!("Could not find symbol {:?} in env {:?}", symbol, env) + panic!( + "Could not find symbol {:?} in vars_by_symbol {:?}", + symbol, vars_by_symbol + ) })); let expected = type_to_var(subs, expected_type.clone().get_type()); @@ -30,7 +33,7 @@ pub fn solve(env: &Env, subs: &mut Subs, constraint: &Constraint) { } And(sub_constraints) => { for sub_constraint in sub_constraints.iter() { - solve(env, subs, sub_constraint); + solve(vars_by_symbol, subs, sub_constraint); } } Pattern(_region, _category, typ, expected) => { @@ -45,30 +48,30 @@ pub fn solve(env: &Env, subs: &mut Subs, constraint: &Constraint) { True => { // If the return expression is guaranteed to solve, // solve the assignments themselves and move on. - solve(env, subs, &let_con.defs_constraint) + solve(vars_by_symbol, subs, &let_con.defs_constraint) } ret_con => { // Solve the assignments' constraints first. - solve(env, subs, &let_con.defs_constraint); + solve(vars_by_symbol, subs, &let_con.defs_constraint); - // Add a variable for each assignment to the env. - let mut new_env = env.clone(); + // Add a variable for each assignment to the vars_by_symbol. + let mut new_vars_by_symbol = vars_by_symbol.clone(); for (symbol, loc_type) in let_con.def_types.iter() { // We must not overwrite existing symbols! If we do, // we will overwrite procedure entries, which were // inserted earlier in solving. (If we allowed // shadowing, we'd need to do something fancier here.) - if !new_env.contains_key(&symbol) { + if !new_vars_by_symbol.contains_key(&symbol) { let var = type_to_var(subs, loc_type.value.clone()); - new_env.insert(symbol.clone(), var); + new_vars_by_symbol.insert(symbol.clone(), var); } } - // Now solve the body, using the new env which includes + // Now solve the body, using the new vars_by_symbol which includes // the assignments' name-to-variable mappings. - solve(&new_env, subs, &ret_con); + solve(&new_vars_by_symbol, subs, &ret_con); // TODO do an occurs check for each of the assignments! } diff --git a/tests/test_load.rs b/tests/test_load.rs index fabe3bd294..277af9458e 100644 --- a/tests/test_load.rs +++ b/tests/test_load.rs @@ -61,9 +61,7 @@ mod test_load { let module = expect_module(load(src_dir, filename, &mut deps, 0).await); assert_eq!(module.name, Some("Primary".into())); - - // This should be the sum of explicit defs vs desugared ones from exposed imports - assert_eq!(module.defs.len(), 8); + assert_eq!(module.defs.len(), 6); let module_names: Vec>> = deps .into_iter() @@ -121,9 +119,7 @@ mod test_load { let module = expect_module(load(src_dir, filename, &mut deps, vars_created).await); assert_eq!(module.name, Some("Primary".into())); - - // This should be the sum of explicit defs vs desugared ones from exposed imports - assert_eq!(module.defs.len(), 8); + assert_eq!(module.defs.len(), 6); let module_names: Vec>> = deps .into_iter()