From 06bc187f8b68b234bcbbd5060a4a710d11fcff32 Mon Sep 17 00:00:00 2001 From: Folkert Date: Mon, 12 Oct 2020 01:42:03 +0200 Subject: [PATCH] all tests passing + clippy satisfied --- compiler/gen/src/llvm/build.rs | 10 -- compiler/load/src/file.rs | 25 +-- compiler/load/tests/test_load.rs | 15 +- compiler/load/tests/test_uniq_load.rs | 15 +- compiler/mono/src/ir.rs | 233 ++++++++++++++++---------- compiler/mono/src/layout.rs | 5 +- compiler/types/src/solved_types.rs | 11 +- compiler/types/src/types.rs | 2 +- 8 files changed, 177 insertions(+), 139 deletions(-) diff --git a/compiler/gen/src/llvm/build.rs b/compiler/gen/src/llvm/build.rs index c339cf65b2..55790291b3 100644 --- a/compiler/gen/src/llvm/build.rs +++ b/compiler/gen/src/llvm/build.rs @@ -1511,16 +1511,6 @@ pub fn build_exp_stmt<'a, 'ctx, 'env>( increment_refcount_layout(env, parent, layout_ids, value, &layout); } - /* - match layout { - Layout::Builtin(Builtin::List(MemoryMode::Refcounted, _)) => { - increment_refcount_list(env, parent, value.into_struct_value()); - build_exp_stmt(env, layout_ids, scope, parent, cont) - } - _ => build_exp_stmt(env, layout_ids, scope, parent, cont), - } - */ - build_exp_stmt(env, layout_ids, scope, parent, cont) } Dec(symbol, cont) => { diff --git a/compiler/load/src/file.rs b/compiler/load/src/file.rs index 30d72e6f86..8f69b4ebe7 100644 --- a/compiler/load/src/file.rs +++ b/compiler/load/src/file.rs @@ -24,7 +24,6 @@ use roc_region::all::{Located, Region}; use roc_solve::module::SolvedModule; use roc_solve::solve; use roc_types::solved_types::Solved; -use roc_types::solved_types::SolvedType; use roc_types::subs::{Subs, VarStore, Variable}; use roc_types::types::Alias; use std::collections::hash_map::Entry::{Occupied, Vacant}; @@ -308,7 +307,7 @@ fn start_phase<'a>(module_id: ModuleId, phase: Phase, state: &mut State<'a>) -> decls, finished_info, ident_ids, - pending_specializations: state.all_pending_specializations.clone(), + exposed_to_host: state.exposed_to_host.clone(), } } Phase::MakeSpecializations => { @@ -647,8 +646,7 @@ enum BuildTask<'a> { ident_ids: IdentIds, decls: Vec, finished_info: FinishedInfo<'a>, - // TODO remove? - pending_specializations: MutMap, PendingSpecialization>>, + exposed_to_host: MutSet, }, MakeSpecializations { module_id: ModuleId, @@ -717,8 +715,6 @@ pub fn load_and_typecheck( ) -> Result { use LoadResult::*; - // Reserve one CPU for the main thread, and let all the others be eligible - match load( arena, filename, @@ -1980,7 +1976,7 @@ fn make_specializations<'a>( ); let external_specializations_requested = procs.externals_we_need.clone(); - let (procedures, _param_map) = procs.get_specialized_procs_help(mono_env.arena); + let procedures = procs.get_specialized_procs_without_rc(mono_env.arena); Msg::MadeSpecializations { module_id: home, @@ -2005,7 +2001,7 @@ fn build_pending_specializations<'a>( _module_timing: ModuleTiming, mut layout_cache: LayoutCache<'a>, // TODO remove - _pending_specializations: MutMap, PendingSpecialization>>, + exposed_to_host: MutSet, finished_info: FinishedInfo<'a>, ) -> Msg<'a> { let mut procs = Procs::default(); @@ -2029,13 +2025,7 @@ fn build_pending_specializations<'a>( match decl { Declare(def) | Builtin(def) => match def.loc_pattern.value { Identifier(symbol) => { - let is_exposed = finished_info - .exposed_vars_by_symbol - .iter() - .find(|(k, _)| *k == symbol) - .is_some(); - - let is_exposed = is_exposed && !(format!("{:?}", home) == "Utils"); + let is_exposed = exposed_to_host.contains(&symbol); match def.loc_expr.value { Closure { @@ -2094,7 +2084,6 @@ fn build_pending_specializations<'a>( // get specialized! if is_exposed { let annotation = def.expr_var; - let ret_var = def.expr_var; let layout = layout_cache.from_var(mono_env.arena, annotation, mono_env.subs).unwrap_or_else(|err| todo!("TODO gracefully handle the situation where we expose a function to the host which doesn't have a valid layout (e.g. maybe the function wasn't monomorphic): {:?}", err) ); @@ -2195,7 +2184,7 @@ fn run_task<'a>( layout_cache, solved_subs, finished_info, - pending_specializations, + exposed_to_host, } => Ok(build_pending_specializations( arena, solved_subs, @@ -2204,7 +2193,7 @@ fn run_task<'a>( decls, module_timing, layout_cache, - pending_specializations, + exposed_to_host, finished_info, )), MakeSpecializations { diff --git a/compiler/load/tests/test_load.rs b/compiler/load/tests/test_load.rs index 214fe2655c..9899af2a84 100644 --- a/compiler/load/tests/test_load.rs +++ b/compiler/load/tests/test_load.rs @@ -14,19 +14,18 @@ mod helpers; #[cfg(test)] mod test_load { use crate::helpers::fixtures_dir; + use bumpalo::Bump; use inlinable_string::InlinableString; use roc_can::def::Declaration::*; use roc_can::def::Def; use roc_collections::all::MutMap; use roc_constrain::module::SubsByModule; - use roc_load::file::{load, LoadedModule, Phase}; + use roc_load::file::LoadedModule; use roc_module::symbol::{Interns, ModuleId}; use roc_types::pretty_print::{content_to_string, name_all_type_vars}; use roc_types::subs::Subs; use std::collections::HashMap; - const GOAL_PHASE: Phase = Phase::SolveTypes; - // HELPERS fn load_fixture( @@ -36,12 +35,13 @@ mod test_load { ) -> LoadedModule { let src_dir = fixtures_dir().join(dir_name); let filename = src_dir.join(format!("{}.roc", module_name)); - let loaded = load( + let arena = Bump::new(); + let loaded = roc_load::file::load_and_typecheck( + &arena, filename, roc_builtins::std::standard_stdlib(), src_dir.as_path(), subs_by_module, - GOAL_PHASE, ); let loaded_module = loaded.expect("Test module failed to load"); @@ -132,12 +132,13 @@ mod test_load { let subs_by_module = MutMap::default(); let src_dir = fixtures_dir().join("interface_with_deps"); let filename = src_dir.join("Primary.roc"); - let loaded = load( + let arena = Bump::new(); + let loaded = roc_load::file::load_and_typecheck( + &arena, filename, roc_builtins::std::standard_stdlib(), src_dir.as_path(), subs_by_module, - GOAL_PHASE, ); let mut loaded_module = loaded.expect("Test module failed to load"); diff --git a/compiler/load/tests/test_uniq_load.rs b/compiler/load/tests/test_uniq_load.rs index cbb5c5a2cf..aaede3ddc6 100644 --- a/compiler/load/tests/test_uniq_load.rs +++ b/compiler/load/tests/test_uniq_load.rs @@ -14,20 +14,19 @@ mod helpers; #[cfg(test)] mod test_uniq_load { use crate::helpers::fixtures_dir; + use bumpalo::Bump; use inlinable_string::InlinableString; use roc_builtins::unique; use roc_can::def::Declaration::*; use roc_can::def::Def; use roc_collections::all::MutMap; use roc_constrain::module::SubsByModule; - use roc_load::file::{load, LoadedModule, Phase}; + use roc_load::file::LoadedModule; use roc_module::symbol::{Interns, ModuleId}; use roc_types::pretty_print::{content_to_string, name_all_type_vars}; use roc_types::subs::Subs; use std::collections::HashMap; - const GOAL_PHASE: Phase = Phase::SolveTypes; - // HELPERS fn load_fixture( @@ -35,14 +34,15 @@ mod test_uniq_load { module_name: &str, subs_by_module: SubsByModule, ) -> LoadedModule { + let arena = Bump::new(); let src_dir = fixtures_dir().join(dir_name); let filename = src_dir.join(format!("{}.roc", module_name)); - let loaded = load( + let loaded = roc_load::file::load_and_typecheck( + &arena, filename, unique::uniq_stdlib(), src_dir.as_path(), subs_by_module, - GOAL_PHASE, ); let loaded_module = loaded.expect("Test module failed to load"); @@ -129,15 +129,16 @@ mod test_uniq_load { #[test] fn interface_with_deps() { + let arena = Bump::new(); let subs_by_module = MutMap::default(); let src_dir = fixtures_dir().join("interface_with_deps"); let filename = src_dir.join("Primary.roc"); - let loaded = load( + let loaded = roc_load::file::load_and_typecheck( + &arena, filename, roc_builtins::std::standard_stdlib(), src_dir.as_path(), subs_by_module, - GOAL_PHASE, ); let mut loaded_module = loaded.expect("Test module failed to load"); diff --git a/compiler/mono/src/ir.rs b/compiler/mono/src/ir.rs index 481751fa4e..746f9649ba 100644 --- a/compiler/mono/src/ir.rs +++ b/compiler/mono/src/ir.rs @@ -194,6 +194,37 @@ impl<'a> Procs<'a> { } } + pub fn get_specialized_procs_without_rc( + self, + arena: &'a Bump, + ) -> MutMap<(Symbol, Layout<'a>), Proc<'a>> { + let mut result = MutMap::with_capacity_and_hasher(self.specialized.len(), default_hasher()); + + for (key, in_prog_proc) in self.specialized.into_iter() { + match in_prog_proc { + InProgress => unreachable!("The procedure {:?} should have be done by now", key), + Done(proc) => { + result.insert(key, proc); + } + } + } + + for (_, proc) in result.iter_mut() { + use self::SelfRecursive::*; + if let SelfRecursive(id) = proc.is_self_recursive { + proc.body = crate::tail_recursion::make_tail_recursive( + arena, + id, + proc.name, + proc.body.clone(), + proc.args, + ); + } + } + + result + } + // TODO investigate make this an iterator? pub fn get_specialized_procs(self, arena: &'a Bump) -> MutMap<(Symbol, Layout<'a>), Proc<'a>> { let mut result = MutMap::with_capacity_and_hasher(self.specialized.len(), default_hasher()); @@ -223,7 +254,7 @@ impl<'a> Procs<'a> { let borrow_params = arena.alloc(crate::borrow::infer_borrow(arena, &result)); for (_, proc) in result.iter_mut() { - // crate::inc_dec::visit_proc(arena, borrow_params, proc); + crate::inc_dec::visit_proc(arena, borrow_params, proc); } result @@ -263,7 +294,7 @@ impl<'a> Procs<'a> { let borrow_params = arena.alloc(crate::borrow::infer_borrow(arena, &result)); for (_, proc) in result.iter_mut() { - // crate::inc_dec::visit_proc(arena, borrow_params, proc); + crate::inc_dec::visit_proc(arena, borrow_params, proc); } (result, borrow_params) @@ -332,7 +363,7 @@ impl<'a> Procs<'a> { let is_self_recursive = false; match patterns_to_when(env, layout_cache, loc_args, ret_var, loc_body) { - Ok((pattern_vars, pattern_symbols, body)) => { + Ok((_, pattern_symbols, body)) => { // an anonymous closure. These will always be specialized already // by the surrounding context, so we can add pending specializations // for them immediately. @@ -386,9 +417,12 @@ impl<'a> Procs<'a> { self.specialized .insert((symbol, layout.clone()), InProgress); + let outside_layout = layout.clone(); + match specialize(env, self, symbol, layout_cache, pending, partial_proc) { Ok((proc, layout)) => { + debug_assert_eq!(outside_layout, layout); self.specialized.insert((symbol, layout), Done(proc)); } Err(error) => { @@ -398,6 +432,7 @@ impl<'a> Procs<'a> { ); self.runtime_errors .insert(symbol, env.arena.alloc(error_msg)); + panic!(); } } } @@ -459,69 +494,44 @@ impl<'a> Procs<'a> { // We're done with that tuple, so move layout back out to avoid cloning it. let (name, layout) = tuple; - // now we have to pull some tricks to extract the return var and pattern vars from Subs - match get_args_ret_var(env.subs, fn_var) { - Some((pattern_vars, ret_var)) => { - let pattern_vars = - Vec::from_iter_in(pattern_vars.into_iter(), env.arena).into_bump_slice(); - let pending = PendingSpecialization::from_var(env.subs, fn_var); + let pending = PendingSpecialization::from_var(env.subs, fn_var); - // This should only be called when pending_specializations is Some. - // Otherwise, it's being called in the wrong pass! - match &mut self.pending_specializations { - Some(pending_specializations) => { - // register the pending specialization, so this gets code genned later - add_pending(pending_specializations, name, layout, pending) + // This should only be called when pending_specializations is Some. + // Otherwise, it's being called in the wrong pass! + match &mut self.pending_specializations { + Some(pending_specializations) => { + // register the pending specialization, so this gets code genned later + add_pending(pending_specializations, name, layout, pending) + } + None => { + let symbol = name; + + // TODO should pending_procs hold a Rc? + let partial_proc = self.partial_procs.get(&symbol).unwrap().clone(); + + // Mark this proc as in-progress, so if we're dealing with + // mutually recursive functions, we don't loop forever. + // (We had a bug around this before this system existed!) + self.specialized + .insert((symbol, layout.clone()), InProgress); + + match specialize(env, self, symbol, layout_cache, pending, partial_proc) { + Ok((proc, layout)) => { + self.specialized.insert((symbol, layout), Done(proc)); } - None => { - let symbol = name; - - // TODO should pending_procs hold a Rc? - let partial_proc = self.partial_procs.get(&symbol).unwrap().clone(); - - // Mark this proc as in-progress, so if we're dealing with - // mutually recursive functions, we don't loop forever. - // (We had a bug around this before this system existed!) - self.specialized - .insert((symbol, layout.clone()), InProgress); - - match specialize(env, self, symbol, layout_cache, pending, partial_proc) { - Ok((proc, layout)) => { - self.specialized.insert((symbol, layout), Done(proc)); - } - Err(error) => { - let error_msg = - format!("TODO generate a RuntimeError message for {:?}", error); - self.runtime_errors - .insert(symbol, env.arena.alloc(error_msg)); - } - } + Err(error) => { + let error_msg = + format!("TODO generate a RuntimeError message for {:?}", error); + self.runtime_errors + .insert(symbol, env.arena.alloc(error_msg)); + panic!(); } } } - other => { - unreachable!( - "trying to insert a symbol that is not a function: {:?} {:?}", - name, other - ); - } } } } -fn get_args_ret_var(subs: &Subs, var: Variable) -> Option<(std::vec::Vec, Variable)> { - match subs.get_without_compacting(var).content { - Content::Structure(FlatType::Func(pattern_vars, _closure_var, ret_var)) => { - Some((pattern_vars, ret_var)) - } - Content::Structure(FlatType::Apply(Symbol::ATTR_ATTR, args)) => { - get_args_ret_var(subs, args[1]) - } - Content::Alias(_, _, actual) => get_args_ret_var(subs, actual), - _ => None, - } -} - fn add_pending<'a>( pending_specializations: &mut MutMap, PendingSpecialization>>, symbol: Symbol, @@ -1240,11 +1250,6 @@ pub fn specialize_all<'a>( mut procs: Procs<'a>, layout_cache: &mut LayoutCache<'a>, ) -> Procs<'a> { - // add the specializations that other modules require of us - use roc_constrain::module::{to_type, FreeVars}; - use roc_solve::solve::{insert_type_into_subs, unsafe_copy_var_help}; - use roc_types::subs::VarStore; - let it = procs.externals_others_need.specs.clone(); let it = it .into_iter() @@ -1308,20 +1313,36 @@ pub fn specialize_all<'a>( // Mark this proc as in-progress, so if we're dealing with // mutually recursive functions, we don't loop forever. // (We had a bug around this before this system existed!) - procs.specialized.insert((name, layout.clone()), InProgress); + let outside_layout = layout.clone(); + procs + .specialized + .insert((name, outside_layout.clone()), InProgress); - match specialize(env, &mut procs, name, layout_cache, pending, partial_proc) { + match specialize( + env, + &mut procs, + name, + layout_cache, + pending.clone(), + partial_proc, + ) { + Ok((proc, layout)) if outside_layout != layout => { + println!("Layouts don't match for function {:?}", proc.name,); + dbg!(outside_layout, layout, &pending.solved_type); + panic!(); + } Ok((proc, layout)) => { + procs.specialized.remove(&(name, outside_layout)); procs.specialized.insert((name, layout), Done(proc)); } Err(error) => { + let error_msg = env.arena.alloc(format!( + "TODO generate a RuntimeError message for {:?}", + error + )); + + procs.runtime_errors.insert(name, error_msg); panic!("failed to specialize {:?}", name); - // let error_msg = env.arena.alloc(format!( - // "TODO generate a RuntimeError message for {:?}", - // error - // )); - // - // procs.runtime_errors.insert(name, error_msg); } } } @@ -1392,7 +1413,9 @@ fn build_specialized_proc_from_var<'a>( Content::Structure(FlatType::Func(pattern_vars, _closure_var, ret_var)) => { build_specialized_proc(env, layout_cache, pattern_symbols, &pattern_vars, ret_var) } - Content::Structure(FlatType::Apply(Symbol::ATTR_ATTR, args)) => { + Content::Structure(FlatType::Apply(Symbol::ATTR_ATTR, args)) + if !pattern_symbols.is_empty() => + { build_specialized_proc_from_var(env, layout_cache, pattern_symbols, args[1]) } Content::Alias(_, _, actual) => { @@ -1473,20 +1496,22 @@ fn specialize_solved_type<'a>( let mut free_vars = FreeVars::default(); let mut var_store = VarStore::new_from_subs(env.subs); + let before = var_store.peek(); + let normal_type = to_type(&solved_type, &mut free_vars, &mut var_store); - let variables_introduced = var_store.peek() as usize - (env.subs.len() - 1); + let after = var_store.peek(); + let variables_introduced = after - before; - env.subs.extend_by(variables_introduced); + env.subs.extend_by(variables_introduced as usize); let fn_var = insert_type_into_subs(env.subs, &normal_type); - let layout = layout_cache - .from_var(&env.arena, fn_var, env.subs) - .unwrap_or_else(|err| panic!("TODO handle invalid function {:?}", err)); - match specialize_external(env, procs, proc_name, layout_cache, fn_var, partial_proc) { Ok(proc) => { + let layout = layout_cache + .from_var(&env.arena, fn_var, env.subs) + .unwrap_or_else(|err| panic!("TODO handle invalid function {:?}", err)); env.subs.rollback_to(snapshot); Ok((proc, layout)) } @@ -1497,6 +1522,30 @@ fn specialize_solved_type<'a>( } } +#[derive(Debug)] +struct FunctionLayouts<'a> { + full: Layout<'a>, + arguments: &'a [Layout<'a>], + result: Layout<'a>, +} + +impl<'a> FunctionLayouts<'a> { + pub fn from_layout(layout: Layout<'a>) -> Self { + match &layout { + Layout::FunctionPointer(arguments, result) => FunctionLayouts { + arguments, + result: (*result).clone(), + full: layout, + }, + _ => FunctionLayouts { + full: layout.clone(), + arguments: &[], + result: layout, + }, + } + } +} + pub fn with_hole<'a>( env: &mut Env<'a, '_>, can_expr: roc_can::expr::Expr, @@ -1679,14 +1728,12 @@ pub fn with_hole<'a>( if procs.module_thunks.contains(&symbol) { let partial_proc = procs.partial_procs.get(&symbol).unwrap(); let fn_var = partial_proc.annotation; - let ret_var = fn_var; // These are the same for a thunk. // This is a top-level declaration, which will code gen to a 0-arity thunk. let result = call_by_name( env, procs, fn_var, - ret_var, symbol, std::vec::Vec::new(), layout_cache, @@ -2386,7 +2433,6 @@ pub fn with_hole<'a>( env, procs, fn_var, - ret_var, proc_name, loc_args, layout_cache, @@ -2567,7 +2613,6 @@ pub fn from_can<'a>( .from_var(env.arena, cond_var, env.subs) .expect("invalid cond_layout"); - dbg!("in an if"); let mut stmt = from_can(env, final_else.value, procs, layout_cache); for (loc_cond, loc_then) in branches.into_iter().rev() { @@ -3561,7 +3606,6 @@ fn call_by_name<'a>( env: &mut Env<'a, '_>, procs: &mut Procs<'a>, fn_var: Variable, - ret_var: Variable, proc_name: Symbol, loc_args: std::vec::Vec<(Variable, Located)>, layout_cache: &mut LayoutCache<'a>, @@ -3685,16 +3729,22 @@ fn call_by_name<'a>( pending, partial_proc, ) { - Ok((proc, _layout)) => { - procs - .specialized - .insert((proc_name, full_layout.clone()), Done(proc)); + Ok((proc, layout)) => { + debug_assert_eq!(full_layout, layout); + let function_layout = FunctionLayouts::from_layout(layout); + + procs.specialized.remove(&(proc_name, full_layout)); + + procs.specialized.insert( + (proc_name, function_layout.full.clone()), + Done(proc), + ); let call = Expr::FunctionCall { call_type: CallType::ByName(proc_name), - ret_layout: ret_layout.clone(), - full_layout: full_layout.clone(), - arg_layouts, + ret_layout: function_layout.result.clone(), + full_layout: function_layout.full, + arg_layouts: function_layout.arguments, args: field_symbols, }; @@ -3704,7 +3754,7 @@ fn call_by_name<'a>( .zip(field_symbols.iter().rev()); let result = - Stmt::Let(assigned, call, ret_layout.clone(), hole); + Stmt::Let(assigned, call, function_layout.result, hole); assign_to_symbols(env, procs, layout_cache, iter, result) } @@ -3716,7 +3766,8 @@ fn call_by_name<'a>( procs.runtime_errors.insert(proc_name, error_msg); - Stmt::RuntimeError(error_msg) + panic!(); + // Stmt::RuntimeError(error_msg) } } } diff --git a/compiler/mono/src/layout.rs b/compiler/mono/src/layout.rs index e54fbadcca..2208121a9e 100644 --- a/compiler/mono/src/layout.rs +++ b/compiler/mono/src/layout.rs @@ -250,10 +250,13 @@ impl<'a> LayoutCache<'a> { seen: MutSet::default(), }; + /* self.layouts .entry(var) .or_insert_with(|| Layout::from_var(&mut env, var)) .clone() + */ + Layout::from_var(&mut env, var) } } @@ -370,7 +373,7 @@ fn layout_from_flat_type<'a>( // correct the memory mode of unique lists match Layout::from_var(env, wrapped_var)? { - Layout::Builtin(Builtin::List(_, elem_layout)) => { + Layout::Builtin(Builtin::List(_ignored, elem_layout)) => { let uniqueness_var = args[0]; let uniqueness_content = subs.get_without_compacting(uniqueness_var).content; diff --git a/compiler/types/src/solved_types.rs b/compiler/types/src/solved_types.rs index 84440e01a8..8ff0ce5e32 100644 --- a/compiler/types/src/solved_types.rs +++ b/compiler/types/src/solved_types.rs @@ -72,10 +72,13 @@ impl SolvedBool { match boolean { Bool::Shared => SolvedBool::SolvedShared, Bool::Container(cvar, mvars) => { - debug_assert!(matches!( - subs.get_without_compacting(*cvar).content, - crate::subs::Content::FlexVar(_) - )); + match subs.get_without_compacting(*cvar).content { + crate::subs::Content::FlexVar(_) => {} + crate::subs::Content::Structure(FlatType::Boolean(Bool::Shared)) => { + return SolvedBool::SolvedShared; + } + other => panic!("Container var is not flex but {:?}", other), + } SolvedBool::SolvedContainer( VarId::from_var(*cvar, subs), diff --git a/compiler/types/src/types.rs b/compiler/types/src/types.rs index 40b10e8a98..d7f9058726 100644 --- a/compiler/types/src/types.rs +++ b/compiler/types/src/types.rs @@ -203,7 +203,7 @@ impl fmt::Debug for Type { } // Sometimes it's useful to see the expansion of the alias - write!(f, "[ but actually {:?} ]", _actual)?; + // write!(f, "[ but actually {:?} ]", _actual)?; Ok(()) }