diff --git a/cli/src/build.rs b/cli/src/build.rs index 681608081c..1c0550025f 100644 --- a/cli/src/build.rs +++ b/cli/src/build.rs @@ -117,22 +117,24 @@ pub fn build_file<'a>( report_timing(buf, "Generate LLVM IR", code_gen_timing.code_gen); report_timing(buf, "Emit .o file", code_gen_timing.emit_o_file); - println!( - "\n\nCompilation finished! Here's how long each module took to compile:\n\n{}", - buf - ); - - println!("\nSuccess! 🎉\n\n\t➡ {}\n", app_o_file.display()); - let compilation_end = compilation_start.elapsed().unwrap(); let size = std::fs::metadata(&app_o_file).unwrap().len(); - println!( - "Finished compilation and code gen in {} ms\n\nProduced a app.o file of size {:?}\n", - compilation_end.as_millis(), - size, - ); + if emit_debug_info { + println!( + "\n\nCompilation finished! Here's how long each module took to compile:\n\n{}", + buf + ); + + println!("\nSuccess! 🎉\n\n\t➡ {}\n", app_o_file.display()); + + println!( + "Finished compilation and code gen in {} ms\n\nProduced a app.o file of size {:?}\n", + compilation_end.as_millis(), + size, + ); + } // Step 2: link the precompiled host and compiled app let mut host_input_path = PathBuf::from(cwd); @@ -144,10 +146,13 @@ pub fn build_file<'a>( let rebuild_host_start = SystemTime::now(); rebuild_host(host_input_path.as_path()); let rebuild_host_end = rebuild_host_start.elapsed().unwrap(); - println!( - "Finished rebuilding the host in {} ms\n", - rebuild_host_end.as_millis() - ); + + if emit_debug_info { + println!( + "Finished rebuilding the host in {} ms\n", + rebuild_host_end.as_millis() + ); + } // TODO try to move as much of this linking as possible to the precompiled // host, to minimize the amount of host-application linking required. @@ -168,7 +173,9 @@ pub fn build_file<'a>( }); let link_end = link_start.elapsed().unwrap(); - println!("Finished linking in {} ms\n", link_end.as_millis()); + if emit_debug_info { + println!("Finished linking in {} ms\n", link_end.as_millis()); + } // Clean up the leftover .o file from the Roc, if possible. // (If cleaning it up fails, that's fine. No need to take action.) diff --git a/cli/tests/cli_run.rs b/cli/tests/cli_run.rs index bf010a1f75..1e6ace6b9b 100644 --- a/cli/tests/cli_run.rs +++ b/cli/tests/cli_run.rs @@ -13,6 +13,7 @@ mod helpers; mod cli_run { use crate::helpers::{ example_file, extract_valgrind_errors, fixture_file, run_cmd, run_roc, run_with_valgrind, + ValgrindError, ValgrindErrorXWhat, }; use serial_test::serial; use std::path::Path; @@ -60,7 +61,24 @@ mod cli_run { }); if !memory_errors.is_empty() { - panic!("{:?}", memory_errors); + for error in memory_errors { + let ValgrindError { + kind, + what: _, + xwhat, + } = error; + println!("Valgrind Error: {}\n", kind); + + if let Some(ValgrindErrorXWhat { + text, + leakedbytes: _, + leakedblocks: _, + }) = xwhat + { + println!(" {}", text); + } + } + panic!("Valgrind reported memory errors"); } } else { let exit_code = match valgrind_out.status.code() { @@ -223,6 +241,69 @@ mod cli_run { ); } + #[ignore] + #[test] + #[serial(closure1)] + fn closure1() { + check_output( + &example_file("benchmarks", "Closure1.roc"), + "closure1", + &[], + "", + true, + ); + } + + #[ignore] + #[test] + #[serial(closure2)] + fn closure2() { + check_output( + &example_file("benchmarks", "Closure2.roc"), + "closure2", + &[], + "", + true, + ); + } + + #[ignore] + #[test] + #[serial(closure3)] + fn closure3() { + check_output( + &example_file("benchmarks", "Closure3.roc"), + "closure3", + &[], + "", + true, + ); + } + + #[ignore] + #[test] + #[serial(closure4)] + fn closure4() { + check_output( + &example_file("benchmarks", "Closure4.roc"), + "closure4", + &[], + "", + true, + ); + } + + // #[test] + // #[serial(effect)] + // fn run_effect_unoptimized() { + // check_output( + // &example_file("effect", "Main.roc"), + // &[], + // "I am Dep2.str2\n", + // true, + // ); + // } + #[test] #[serial(multi_dep_str)] fn run_multi_dep_str_unoptimized() { diff --git a/cli/tests/helpers.rs b/cli/tests/helpers.rs index 376158d678..27e8ad1044 100644 --- a/cli/tests/helpers.rs +++ b/cli/tests/helpers.rs @@ -192,20 +192,20 @@ struct ValgrindDummyStruct {} #[derive(Debug, Deserialize, Clone)] pub struct ValgrindError { - kind: String, + pub kind: String, #[serde(default)] - what: Option, + pub what: Option, #[serde(default)] - xwhat: Option, + pub xwhat: Option, } #[derive(Debug, Deserialize, Clone)] pub struct ValgrindErrorXWhat { - text: String, + pub text: String, #[serde(default)] - leakedbytes: Option, + pub leakedbytes: Option, #[serde(default)] - leakedblocks: Option, + pub leakedblocks: Option, } #[allow(dead_code)] diff --git a/compiler/gen/src/llvm/build.rs b/compiler/gen/src/llvm/build.rs index ac167fbf20..01992f84a3 100644 --- a/compiler/gen/src/llvm/build.rs +++ b/compiler/gen/src/llvm/build.rs @@ -735,10 +735,25 @@ pub fn build_exp_call<'a, 'ctx, 'env>( arg_vals.push(load_symbol(scope, arg)); } - let call = match sub_expr { - BasicValueEnum::PointerValue(ptr) => { + let call = match (full_layout, sub_expr) { + (_, BasicValueEnum::PointerValue(ptr)) => { env.builder.build_call(ptr, arg_vals.as_slice(), "tmp") } + (Layout::Closure(_, _, _), BasicValueEnum::StructValue(closure_struct)) => { + let fpointer = env + .builder + .build_extract_value(closure_struct, 0, "fpointer") + .unwrap() + .into_pointer_value(); + + let closure_data = env + .builder + .build_extract_value(closure_struct, 1, "closure_data") + .unwrap(); + + arg_vals.push(closure_data); + env.builder.build_call(fpointer, arg_vals.as_slice(), "tmp") + } non_ptr => { panic!( "Tried to call by pointer, but encountered a non-pointer: {:?} {:?} {:?}", diff --git a/compiler/gen/tests/gen_primitives.rs b/compiler/gen/tests/gen_primitives.rs index 59b5760dda..3a247fad7e 100644 --- a/compiler/gen/tests/gen_primitives.rs +++ b/compiler/gen/tests/gen_primitives.rs @@ -1952,7 +1952,7 @@ mod gen_primitives { main = x : Tree F64 x = singleton 3 - when x is + when x is Tree 3.0 _ -> True _ -> False "# @@ -2215,4 +2215,23 @@ mod gen_primitives { i64 ); } + + #[test] + fn build_then_apply_closure() { + assert_evals_to!( + indoc!( + r#" + app "test" provides [ main ] to "./platform" + + main : Str + main = + x = "long string that is malloced" + + (\_ -> x) {} + "# + ), + "long string that is malloced", + &'static str + ); + } } diff --git a/compiler/load/src/file.rs b/compiler/load/src/file.rs index a385eee464..ea760f8c28 100644 --- a/compiler/load/src/file.rs +++ b/compiler/load/src/file.rs @@ -751,6 +751,7 @@ enum Msg<'a> { layout_cache: LayoutCache<'a>, external_specializations_requested: MutMap, procedures: MutMap<(Symbol, Layout<'a>), Proc<'a>>, + passed_by_pointer: MutMap<(Symbol, Layout<'a>), Symbol>, problems: Vec, module_timing: ModuleTiming, subs: Subs, @@ -781,6 +782,7 @@ struct State<'a> { pub module_cache: ModuleCache<'a>, pub dependencies: Dependencies<'a>, pub procedures: MutMap<(Symbol, Layout<'a>), Proc<'a>>, + pub passed_by_pointer: MutMap<(Symbol, Layout<'a>), Symbol>, pub exposed_to_host: MutMap, /// This is the "final" list of IdentIds, after canonicalization and constraint gen @@ -1403,6 +1405,7 @@ where module_cache: ModuleCache::default(), dependencies: Dependencies::default(), procedures: MutMap::default(), + passed_by_pointer: MutMap::default(), exposed_to_host: MutMap::default(), exposed_types, headers_parsed, @@ -1931,6 +1934,7 @@ fn update<'a>( mut ident_ids, subs, procedures, + passed_by_pointer, external_specializations_requested, problems, module_timing, @@ -1945,12 +1949,17 @@ fn update<'a>( .notify(module_id, Phase::MakeSpecializations); state.procedures.extend(procedures); + state.passed_by_pointer.extend(passed_by_pointer); state.timings.insert(module_id, module_timing); if state.dependencies.solved_all() && state.goal_phase == Phase::MakeSpecializations { debug_assert!(work.is_empty(), "still work remaining {:?}", &work); - Proc::insert_refcount_operations(arena, &mut state.procedures); + Proc::insert_refcount_operations( + arena, + &mut state.procedures, + &state.passed_by_pointer, + ); Proc::optimize_refcount_operations( arena, @@ -3621,7 +3630,7 @@ fn make_specializations<'a>( ); let external_specializations_requested = procs.externals_we_need.clone(); - let procedures = procs.get_specialized_procs_without_rc(mono_env.arena); + let (procedures, passed_by_pointer) = procs.get_specialized_procs_without_rc(mono_env.arena); let make_specializations_end = SystemTime::now(); module_timing.make_specializations = make_specializations_end @@ -3633,6 +3642,7 @@ fn make_specializations<'a>( ident_ids, layout_cache, procedures, + passed_by_pointer, problems: mono_problems, subs, external_specializations_requested, diff --git a/compiler/mono/src/borrow.rs b/compiler/mono/src/borrow.rs index 47fd1c8c95..3e7d948ad4 100644 --- a/compiler/mono/src/borrow.rs +++ b/compiler/mono/src/borrow.rs @@ -6,14 +6,31 @@ use roc_collections::all::{MutMap, MutSet}; use roc_module::low_level::LowLevel; use roc_module::symbol::Symbol; +fn should_borrow_layout(layout: &Layout) -> bool { + match layout { + Layout::Closure(_, _, _) => false, + _ => layout.is_refcounted(), + } +} + pub fn infer_borrow<'a>( arena: &'a Bump, procs: &MutMap<(Symbol, Layout<'a>), Proc<'a>>, + passed_by_pointer: &MutMap<(Symbol, Layout<'a>), Symbol>, ) -> ParamMap<'a> { let mut param_map = ParamMap { items: MutMap::default(), }; + for (key, other) in passed_by_pointer { + if let Some(proc) = procs.get(key) { + let mut proc: Proc = proc.clone(); + proc.name = *other; + + param_map.visit_proc_always_owned(arena, &proc); + } + } + for proc in procs.values() { param_map.visit_proc(arena, proc); } @@ -116,7 +133,22 @@ impl<'a> ParamMap<'a> { fn init_borrow_args(arena: &'a Bump, ps: &'a [(Layout<'a>, Symbol)]) -> &'a [Param<'a>] { Vec::from_iter_in( ps.iter().map(|(layout, symbol)| Param { - borrow: layout.is_refcounted(), + borrow: should_borrow_layout(layout), + layout: layout.clone(), + symbol: *symbol, + }), + arena, + ) + .into_bump_slice() + } + + fn init_borrow_args_always_owned( + arena: &'a Bump, + ps: &'a [(Layout<'a>, Symbol)], + ) -> &'a [Param<'a>] { + Vec::from_iter_in( + ps.iter().map(|(layout, symbol)| Param { + borrow: false, layout: layout.clone(), symbol: *symbol, }), @@ -134,6 +166,15 @@ impl<'a> ParamMap<'a> { self.visit_stmt(arena, proc.name, &proc.body); } + fn visit_proc_always_owned(&mut self, arena: &'a Bump, proc: &Proc<'a>) { + self.items.insert( + Key::Declaration(proc.name), + Self::init_borrow_args_always_owned(arena, proc.args), + ); + + self.visit_stmt(arena, proc.name, &proc.body); + } + fn visit_stmt(&mut self, arena: &'a Bump, _fnid: Symbol, stmt: &Stmt<'a>) { use Stmt::*; diff --git a/compiler/mono/src/expand_rc.rs b/compiler/mono/src/expand_rc.rs index c05f232eb7..4cc335cf35 100644 --- a/compiler/mono/src/expand_rc.rs +++ b/compiler/mono/src/expand_rc.rs @@ -157,6 +157,24 @@ impl<'a, 'i> Env<'a, 'i> { } } + fn try_insert_struct_info(&mut self, symbol: Symbol, layout: &Layout<'a>) { + use Layout::*; + + match layout { + Struct(fields) => { + self.constructor_map.insert(symbol, 0); + self.layout_map.insert(symbol, Layout::Struct(fields)); + } + Closure(arguments, closure_layout, result) => { + let fpointer = Layout::FunctionPointer(arguments, result); + let fields = self.arena.alloc([fpointer, closure_layout.layout.clone()]); + self.constructor_map.insert(symbol, 0); + self.layout_map.insert(symbol, Layout::Struct(fields)); + } + _ => {} + } + } + fn insert_struct_info(&mut self, symbol: Symbol, fields: &'a [Layout<'a>]) { self.constructor_map.insert(symbol, 0); self.layout_map.insert(symbol, Layout::Struct(fields)); @@ -185,7 +203,7 @@ impl<'a, 'i> Env<'a, 'i> { } fn layout_for_constructor<'a>( - _arena: &'a Bump, + arena: &'a Bump, layout: &Layout<'a>, constructor: u64, ) -> ConstructorLayout<&'a [Layout<'a>]> { @@ -227,7 +245,12 @@ fn layout_for_constructor<'a>( debug_assert_eq!(constructor, 0); HasFields(fields) } - _ => unreachable!(), + Closure(arguments, closure_layout, result) => { + let fpointer = Layout::FunctionPointer(arguments, result); + let fields = arena.alloc([fpointer, closure_layout.layout.clone()]); + HasFields(fields) + } + other => unreachable!("weird layout {:?}", other), } } @@ -253,11 +276,11 @@ fn work_for_constructor<'a>( match layout_for_constructor(env.arena, full_layout, constructor) { Unknown => Unknown, IsNull => IsNull, - HasFields(cons_layout) => { + HasFields(constructor_layout) => { // figure out if there is at least one aliased refcounted field. Only then // does it make sense to inline the decrement let at_least_one_aliased = (|| { - for (i, field_layout) in cons_layout.iter().enumerate() { + for (i, field_layout) in constructor_layout.iter().enumerate() { if field_layout.contains_refcounted() && field_aliases.and_then(|map| map.get(&(i as u64))).is_some() { @@ -269,7 +292,7 @@ fn work_for_constructor<'a>( // for each field, if it has refcounted content, check if it has an alias // if so, use the alias, otherwise load the field. - for (i, field_layout) in cons_layout.iter().enumerate() { + for (i, field_layout) in constructor_layout.iter().enumerate() { if field_layout.contains_refcounted() { match field_aliases.and_then(|map| map.get(&(i as u64))) { Some(alias_symbol) => { @@ -283,7 +306,7 @@ fn work_for_constructor<'a>( let expr = Expr::AccessAtIndex { index: i as u64, - field_layouts: cons_layout, + field_layouts: constructor_layout, structure: *symbol, wrapped: Wrapped::MultiTagUnion, }; @@ -350,10 +373,11 @@ pub fn expand_and_cancel_proc<'a>( introduced.push(*symbol); } - Layout::Closure(_, closure_layout, _) => { - if let Layout::Struct(fields) = closure_layout.layout { - env.insert_struct_info(*symbol, fields); - } + Layout::Closure(arguments, closure_layout, result) => { + let fpointer = Layout::FunctionPointer(arguments, result); + let fields = env.arena.alloc([fpointer, closure_layout.layout.clone()]); + env.insert_struct_info(*symbol, fields); + introduced.push(*symbol); } _ => {} } @@ -414,7 +438,10 @@ fn expand_and_cancel<'a>(env: &mut Env<'a, '_>, stmt: &'a Stmt<'a>) -> &'a Stmt< match &expr { Expr::AccessAtIndex { - structure, index, .. + structure, + index, + field_layouts, + .. } => { let entry = env .alias_map @@ -423,8 +450,14 @@ fn expand_and_cancel<'a>(env: &mut Env<'a, '_>, stmt: &'a Stmt<'a>) -> &'a Stmt< entry.insert(*index, symbol); + // if the field is a struct, we know its constructor too! + let field_layout = &field_layouts[*index as usize]; + env.try_insert_struct_info(symbol, field_layout); + new_cont = expand_and_cancel(env, cont); + env.remove_struct_info(symbol); + // make sure to remove the alias, so other branches don't use it by accident env.alias_map .get_mut(structure) diff --git a/compiler/mono/src/inc_dec.rs b/compiler/mono/src/inc_dec.rs index a7413a9c60..964be08878 100644 --- a/compiler/mono/src/inc_dec.rs +++ b/compiler/mono/src/inc_dec.rs @@ -602,7 +602,10 @@ impl<'a> Context<'a> { consume: bool, ) -> Self { // can this type be reference-counted at runtime? - let reference = layout.contains_refcounted(); + let reference = match layout { + Layout::Closure(_, closure, _) => closure.layout.contains_refcounted(), + _ => layout.contains_refcounted(), + }; let info = VarInfo { reference, diff --git a/compiler/mono/src/ir.rs b/compiler/mono/src/ir.rs index d4dff32ecc..f865440442 100644 --- a/compiler/mono/src/ir.rs +++ b/compiler/mono/src/ir.rs @@ -136,8 +136,15 @@ pub enum SelfRecursive { SelfRecursive(JoinPointId), } +#[derive(Clone, Copy, Debug, PartialEq)] +pub enum Parens { + NotNeeded, + InTypeParam, + InFunction, +} + impl<'a> Proc<'a> { - pub fn to_doc<'b, D, A>(&'b self, alloc: &'b D, _parens: bool) -> DocBuilder<'b, D, A> + pub fn to_doc<'b, D, A>(&'b self, alloc: &'b D, _parens: Parens) -> DocBuilder<'b, D, A> where D: DocAllocator<'b, A>, D::Doc: Clone, @@ -148,20 +155,36 @@ impl<'a> Proc<'a> { .iter() .map(|(_, symbol)| symbol_to_doc(alloc, *symbol)); - alloc - .text("procedure ") - .append(symbol_to_doc(alloc, self.name)) - .append(" (") - .append(alloc.intersperse(args_doc, ", ")) - .append("):") - .append(alloc.hardline()) - .append(self.body.to_doc(alloc).indent(4)) + if PRETTY_PRINT_IR_SYMBOLS { + alloc + .text("procedure : ") + .append(symbol_to_doc(alloc, self.name)) + .append(" ") + .append(self.ret_layout.to_doc(alloc, Parens::NotNeeded)) + .append(alloc.hardline()) + .append(alloc.text("procedure = ")) + .append(symbol_to_doc(alloc, self.name)) + .append(" (") + .append(alloc.intersperse(args_doc, ", ")) + .append("):") + .append(alloc.hardline()) + .append(self.body.to_doc(alloc).indent(4)) + } else { + alloc + .text("procedure ") + .append(symbol_to_doc(alloc, self.name)) + .append(" (") + .append(alloc.intersperse(args_doc, ", ")) + .append("):") + .append(alloc.hardline()) + .append(self.body.to_doc(alloc).indent(4)) + } } pub fn to_pretty(&self, width: usize) -> String { let allocator = BoxAllocator; let mut w = std::vec::Vec::new(); - self.to_doc::<_, ()>(&allocator, false) + self.to_doc::<_, ()>(&allocator, Parens::NotNeeded) .1 .render(width, &mut w) .unwrap(); @@ -172,8 +195,28 @@ impl<'a> Proc<'a> { pub fn insert_refcount_operations( arena: &'a Bump, procs: &mut MutMap<(Symbol, Layout<'a>), Proc<'a>>, + _passed_by_pointer: &MutMap<(Symbol, Layout<'a>), Symbol>, ) { - let borrow_params = arena.alloc(crate::borrow::infer_borrow(arena, procs)); + // currently we ignore the passed-by-pointerness + let passed_by_pointer = &Default::default(); + + let borrow_params = + arena.alloc(crate::borrow::infer_borrow(arena, procs, passed_by_pointer)); + + for (key, other) in passed_by_pointer { + if let Some(proc) = procs.get(key) { + let mut proc: Proc = proc.clone(); + proc.name = *other; + + let layout = key.1.clone(); + procs.insert((*other, layout), proc); + } else { + unreachable!( + "we need a by-pointer version of {:?}, but its by-name version does not exist", + key.0 + ) + } + } for (_, proc) in procs.iter_mut() { crate::inc_dec::visit_proc(arena, borrow_params, proc); @@ -255,6 +298,7 @@ pub struct Procs<'a> { pub runtime_errors: MutMap, pub externals_others_need: ExternalSpecializations, pub externals_we_need: MutMap, + pub passed_by_pointer: MutMap<(Symbol, Layout<'a>), Symbol>, } impl<'a> Default for Procs<'a> { @@ -267,6 +311,7 @@ impl<'a> Default for Procs<'a> { runtime_errors: MutMap::default(), externals_we_need: MutMap::default(), externals_others_need: ExternalSpecializations::default(), + passed_by_pointer: MutMap::default(), } } } @@ -311,10 +356,14 @@ impl<'a> Procs<'a> { } } + #[allow(clippy::type_complexity)] pub fn get_specialized_procs_without_rc( self, arena: &'a Bump, - ) -> MutMap<(Symbol, Layout<'a>), Proc<'a>> { + ) -> ( + MutMap<(Symbol, Layout<'a>), Proc<'a>>, + MutMap<(Symbol, Layout<'a>), Symbol>, + ) { let mut result = MutMap::with_capacity_and_hasher(self.specialized.len(), default_hasher()); for (key, in_prog_proc) in self.specialized.into_iter() { @@ -337,7 +386,7 @@ impl<'a> Procs<'a> { } } - result + (result, self.passed_by_pointer) } // TODO investigate make this an iterator? @@ -366,7 +415,11 @@ impl<'a> Procs<'a> { } } - let borrow_params = arena.alloc(crate::borrow::infer_borrow(arena, &result)); + let borrow_params = arena.alloc(crate::borrow::infer_borrow( + arena, + &result, + &self.passed_by_pointer, + )); for (_, proc) in result.iter_mut() { crate::inc_dec::visit_proc(arena, borrow_params, proc); @@ -406,7 +459,11 @@ impl<'a> Procs<'a> { } } - let borrow_params = arena.alloc(crate::borrow::infer_borrow(arena, &result)); + let borrow_params = arena.alloc(crate::borrow::infer_borrow( + arena, + &result, + &self.passed_by_pointer, + )); for (_, proc) in result.iter_mut() { crate::inc_dec::visit_proc(arena, borrow_params, proc); @@ -2435,7 +2492,7 @@ fn specialize_naked_symbol<'a>( match hole { Stmt::Jump(_, _) => todo!("not sure what to do in this case yet"), _ => { - let expr = Expr::FunctionPointer(symbol, layout.clone()); + let expr = call_by_pointer(env, procs, symbol, layout.clone()); let new_symbol = env.unique_symbol(); return Stmt::Let( new_symbol, @@ -3523,7 +3580,7 @@ pub fn with_hole<'a>( // TODO should the let have layout Pointer? Stmt::Let( assigned, - Expr::FunctionPointer(name, layout.clone()), + call_by_pointer(env, procs, name, layout.clone()), layout, hole, ) @@ -3720,7 +3777,7 @@ pub fn with_hole<'a>( } } - let expr = Expr::FunctionPointer(name, function_ptr_layout.clone()); + let expr = call_by_pointer(env, procs, name, function_ptr_layout.clone()); stmt = Stmt::Let( function_pointer, @@ -3746,7 +3803,7 @@ pub fn with_hole<'a>( // TODO should the let have layout Pointer? Stmt::Let( assigned, - Expr::FunctionPointer(name, layout.clone()), + call_by_pointer(env, procs, name, layout.clone()), layout, hole, ) @@ -5327,7 +5384,7 @@ fn handle_variable_aliasing<'a>( .from_var(env.arena, variable, env.subs) .unwrap(); - let expr = Expr::FunctionPointer(right, layout.clone()); + let expr = call_by_pointer(env, procs, right, layout.clone()); Stmt::Let(left, expr, layout, env.arena.alloc(result)) } else { substitute_in_exprs(env.arena, &mut result, left, right); @@ -5375,7 +5432,7 @@ fn reuse_function_symbol<'a>( // an imported symbol is always a function pointer: // either it's a function, or a top-level 0-argument thunk - let expr = Expr::FunctionPointer(original, layout.clone()); + let expr = call_by_pointer(env, procs, original, layout.clone()); return Stmt::Let(symbol, expr, layout, env.arena.alloc(result)); } _ => { @@ -5473,7 +5530,7 @@ fn reuse_function_symbol<'a>( } } - let expr = Expr::FunctionPointer(original, function_ptr_layout.clone()); + let expr = call_by_pointer(env, procs, original, function_ptr_layout.clone()); stmt = Stmt::Let( function_pointer, @@ -5495,7 +5552,7 @@ fn reuse_function_symbol<'a>( Stmt::Let( symbol, - Expr::FunctionPointer(original, layout.clone()), + call_by_pointer(env, procs, original, layout.clone()), layout, env.arena.alloc(result), ) @@ -5572,6 +5629,22 @@ where result } +fn call_by_pointer<'a>( + _env: &mut Env<'a, '_>, + procs: &mut Procs<'a>, + symbol: Symbol, + layout: Layout<'a>, +) -> Expr<'a> { + // let other = env.unique_symbol(); + let other = symbol; + + procs + .passed_by_pointer + .insert((symbol, layout.clone()), other); + + Expr::FunctionPointer(other, layout) +} + fn add_needed_external<'a>( procs: &mut Procs<'a>, env: &mut Env<'a, '_>, diff --git a/compiler/mono/src/layout.rs b/compiler/mono/src/layout.rs index ddb401a0a1..94883e2152 100644 --- a/compiler/mono/src/layout.rs +++ b/compiler/mono/src/layout.rs @@ -1,3 +1,4 @@ +use crate::ir::Parens; use bumpalo::collections::Vec; use bumpalo::Bump; use roc_collections::all::{default_hasher, MutMap, MutSet}; @@ -6,6 +7,7 @@ use roc_module::symbol::{Interns, Symbol}; use roc_types::subs::{Content, FlatType, Subs, Variable}; use roc_types::types::RecordField; use std::collections::HashMap; +use ven_pretty::{DocAllocator, DocBuilder}; pub const MAX_ENUM_SIZE: usize = (std::mem::size_of::() * 8) as usize; const GENERATE_NULLABLE: bool = true; @@ -63,6 +65,34 @@ pub enum UnionLayout<'a> { }, } +impl<'a> UnionLayout<'a> { + pub fn to_doc<'b, D, A>(&'b self, alloc: &'b D, _parens: Parens) -> DocBuilder<'b, D, A> + where + D: DocAllocator<'b, A>, + D::Doc: Clone, + A: Clone, + { + use UnionLayout::*; + + match self { + NonRecursive(tags) => { + let tags_doc = tags.iter().map(|fields| { + alloc.text("C ").append(alloc.intersperse( + fields.iter().map(|x| x.to_doc(alloc, Parens::InTypeParam)), + ", ", + )) + }); + + alloc + .text("[") + .append(alloc.intersperse(tags_doc, ", ")) + .append(alloc.text("]")) + } + _ => alloc.text("TODO"), + } + } +} + #[derive(Clone, Debug, PartialEq, Eq, Hash)] pub struct ClosureLayout<'a> { /// the layout that this specific closure captures @@ -654,6 +684,51 @@ impl<'a> Layout<'a> { FunctionPointer(_, _) | Pointer(_) => false, } } + + pub fn to_doc<'b, D, A>(&'b self, alloc: &'b D, parens: Parens) -> DocBuilder<'b, D, A> + where + D: DocAllocator<'b, A>, + D::Doc: Clone, + A: Clone, + { + use Layout::*; + + match self { + Builtin(builtin) => builtin.to_doc(alloc, parens), + PhantomEmptyStruct => alloc.text("{}"), + Struct(fields) => { + let fields_doc = fields.iter().map(|x| x.to_doc(alloc, parens)); + + alloc + .text("{") + .append(alloc.intersperse(fields_doc, ", ")) + .append(alloc.text("}")) + } + Union(union_layout) => union_layout.to_doc(alloc, parens), + RecursivePointer => alloc.text("*self"), + FunctionPointer(args, result) => { + let args_doc = args.iter().map(|x| x.to_doc(alloc, Parens::InFunction)); + + alloc + .intersperse(args_doc, ", ") + .append(alloc.text(" -> ")) + .append(result.to_doc(alloc, Parens::InFunction)) + } + Closure(args, closure_layout, result) => { + let args_doc = args.iter().map(|x| x.to_doc(alloc, Parens::InFunction)); + + let bom = closure_layout.layout.to_doc(alloc, Parens::NotNeeded); + + alloc + .intersperse(args_doc, ", ") + .append(alloc.text(" {| ")) + .append(bom) + .append(" |} -> ") + .append(result.to_doc(alloc, Parens::InFunction)) + } + Pointer(_) => todo!(), + } + } } /// Avoid recomputing Layout from Variable multiple times. @@ -878,6 +953,47 @@ impl<'a> Builtin<'a> { Str | Dict(_, _) | Set(_) => true, } } + + pub fn to_doc<'b, D, A>(&'b self, alloc: &'b D, _parens: Parens) -> DocBuilder<'b, D, A> + where + D: DocAllocator<'b, A>, + D::Doc: Clone, + A: Clone, + { + use Builtin::*; + + match self { + Int128 => alloc.text("Int128"), + Int64 => alloc.text("Int64"), + Int32 => alloc.text("Int32"), + Int16 => alloc.text("Int16"), + Int8 => alloc.text("Int8"), + Int1 => alloc.text("Int1"), + Usize => alloc.text("Usize"), + Float128 => alloc.text("Float128"), + Float64 => alloc.text("Float64"), + Float32 => alloc.text("Float32"), + Float16 => alloc.text("Float16"), + + EmptyStr => alloc.text("EmptyStr"), + EmptyList => alloc.text("EmptyList"), + EmptyDict => alloc.text("EmptyDict"), + EmptySet => alloc.text("EmptySet"), + + Str => alloc.text("Str"), + List(_, layout) => alloc + .text("List ") + .append(layout.to_doc(alloc, Parens::InTypeParam)), + Set(layout) => alloc + .text("Set ") + .append(layout.to_doc(alloc, Parens::InTypeParam)), + Dict(key_layout, value_layout) => alloc + .text("Dict ") + .append(key_layout.to_doc(alloc, Parens::InTypeParam)) + .append(" ") + .append(value_layout.to_doc(alloc, Parens::InTypeParam)), + } + } } fn layout_from_flat_type<'a>( diff --git a/compiler/reporting/tests/test_reporting.rs b/compiler/reporting/tests/test_reporting.rs index f4f9c0f8b2..0feda26ada 100644 --- a/compiler/reporting/tests/test_reporting.rs +++ b/compiler/reporting/tests/test_reporting.rs @@ -1132,7 +1132,7 @@ mod test_reporting { But the type annotation on `x` says it should be: - Int b + Int a Tip: You can convert between Int and Float using functions like `Num.toFloat` and `Num.round`. @@ -1171,7 +1171,7 @@ mod test_reporting { But the type annotation on `x` says it should be: - Int b + Int a Tip: You can convert between Int and Float using functions like `Num.toFloat` and `Num.round`. @@ -1207,7 +1207,7 @@ mod test_reporting { But the type annotation on `x` says it should be: - Int b + Int a Tip: You can convert between Int and Float using functions like `Num.toFloat` and `Num.round`. @@ -1541,7 +1541,7 @@ mod test_reporting { But the type annotation says it should be: - { x : Int b } + { x : Int a } Tip: You can convert between Int and Float using functions like `Num.toFloat` and `Num.round`. @@ -4041,9 +4041,9 @@ mod test_reporting { indoc!( r#" ── PARSE PROBLEM ─────────────────────────────────────────────────────────────── - + Unexpected token : - + 1│ f :: I64 ^ "# @@ -4061,7 +4061,7 @@ mod test_reporting { indoc!( r#" x = 3 - y = + y = x == 5 Num.add 1 2 @@ -4071,12 +4071,12 @@ mod test_reporting { indoc!( r#" ── TOO MANY ARGS ─────────────────────────────────────────────────────────────── - + The `add` function expects 2 arguments, but it got 4 instead: - + 4│ Num.add 1 2 ^^^^^^^ - + Are there any missing commas? Or missing parentheses? "# ), @@ -4096,9 +4096,9 @@ mod test_reporting { indoc!( r#" ── PARSE PROBLEM ─────────────────────────────────────────────────────────────── - + Unexpected token : - + 2│ 5 ** 3 ^ "# @@ -4117,12 +4117,12 @@ mod test_reporting { indoc!( r#" ── UNFINISHED TAG UNION TYPE ─────────────────────────────────────────────────── - + I just started parsing a tag union type, but I got stuck here: - + 1│ f : [ ^ - + Tag unions look like [ Many I64, None ], so I was expecting to see a tag name next. "# @@ -4135,18 +4135,18 @@ mod test_reporting { report_problem_as( indoc!( r#" - f : [ Yes, + f : [ Yes, "# ), indoc!( r#" ── UNFINISHED TAG UNION TYPE ─────────────────────────────────────────────────── - + I am partway through parsing a tag union type, but I got stuck here: - - 1│ f : [ Yes, + + 1│ f : [ Yes, ^ - + I was expecting to see a closing square bracket before this, so try adding a ] and see if that helps? "# @@ -4159,20 +4159,20 @@ mod test_reporting { report_problem_as( indoc!( r#" - f : [ lowercase ] + f : [ lowercase ] "# ), indoc!( r#" ── WEIRD TAG NAME ────────────────────────────────────────────────────────────── - + I am partway through parsing a tag union type, but I got stuck here: - - 1│ f : [ lowercase ] + + 1│ f : [ lowercase ] ^ - + I was expecting to see a tag name. - + Hint: Tag names start with an uppercase letter, like Err or Green. "# ), @@ -4184,20 +4184,20 @@ mod test_reporting { report_problem_as( indoc!( r#" - f : [ Good, bad ] + f : [ Good, bad ] "# ), indoc!( r#" ── WEIRD TAG NAME ────────────────────────────────────────────────────────────── - + I am partway through parsing a tag union type, but I got stuck here: - - 1│ f : [ Good, bad ] + + 1│ f : [ Good, bad ] ^ - + I was expecting to see a tag name. - + Hint: Tag names start with an uppercase letter, like Err or Green. "# ), @@ -4215,12 +4215,12 @@ mod test_reporting { indoc!( r#" ── UNFINISHED RECORD TYPE ────────────────────────────────────────────────────── - + I just started parsing a record type, but I got stuck here: - + 1│ f : { ^ - + Record types look like { name : String, age : Int }, so I was expecting to see a field name next. "# @@ -4240,15 +4240,15 @@ mod test_reporting { indoc!( r#" ── UNFINISHED RECORD TYPE ────────────────────────────────────────────────────── - + I am partway through parsing a record type, but I got stuck here: - + 1│ f : { ^ - + I was expecting to see a closing curly brace before this, so try adding a } and see if that helps? - + Note: I may be confused by indentation "# ), @@ -4260,18 +4260,18 @@ mod test_reporting { report_problem_as( indoc!( r#" - f : { a: Int, + f : { a: Int, "# ), indoc!( r#" ── UNFINISHED RECORD TYPE ────────────────────────────────────────────────────── - + I am partway through parsing a record type, but I got stuck here: - - 1│ f : { a: Int, + + 1│ f : { a: Int, ^ - + I was expecting to see a closing curly brace before this, so try adding a } and see if that helps? "# @@ -4291,13 +4291,13 @@ mod test_reporting { indoc!( r#" ── NEED MORE INDENTATION ─────────────────────────────────────────────────────── - + I am partway through parsing a record type, but I got stuck here: - + 1│ f : { a: Int 2│ } ^ - + I need this curly brace to be indented more. Try adding more spaces before it! "# @@ -4310,19 +4310,19 @@ mod test_reporting { report_problem_as( indoc!( r#" - f : { if : I64 } + f : { if : I64 } "# ), indoc!( r#" ── UNFINISHED RECORD TYPE ────────────────────────────────────────────────────── - + I just started parsing a record type, but I got stuck on this field name: - - 1│ f : { if : I64 } + + 1│ f : { if : I64 } ^^ - + Looks like you are trying to use `if` as a field name, but that is a reserved word. Try using a different name! "# @@ -4342,12 +4342,12 @@ mod test_reporting { indoc!( r#" ── UNFINISHED RECORD TYPE ────────────────────────────────────────────────────── - + I am partway through parsing a record type, but I got stuck here: - + 1│ f : { foo bar } ^ - + I was expecting to see a colon, question mark, comma or closing curly brace. "# @@ -4363,12 +4363,12 @@ mod test_reporting { indoc!( r#" ── TAB CHARACTER ─────────────────────────────────────────────────────────────── - + I encountered a tab character - + 1│ f : { foo } ^ - + Tab characters are not allowed. "# ), @@ -4383,12 +4383,12 @@ mod test_reporting { indoc!( r#" ── TAB CHARACTER ─────────────────────────────────────────────────────────────── - + I encountered a tab character - + 1│ f : { foo } ^ - + Tab characters are not allowed. "# ), @@ -4407,12 +4407,12 @@ mod test_reporting { indoc!( r#" ── UNFINISHED RECORD TYPE ────────────────────────────────────────────────────── - + I am partway through parsing a record type, but I got stuck here: - - 1│ f : { a: Int, + + 1│ f : { a: Int, ^ - + I was expecting to see a closing curly brace before this, so try adding a } and see if that helps? "# @@ -4431,13 +4431,13 @@ mod test_reporting { indoc!( r#" ── UNFINISHED PARENTHESES ────────────────────────────────────────────────────── - + I am partway through parsing a type in parentheses, but I got stuck here: - + 1│ f : ( I64 ^ - + I was expecting to see a closing parenthesis before this, so try adding a ) and see if that helps? "# @@ -4450,18 +4450,18 @@ mod test_reporting { report_problem_as( indoc!( r#" - f : Foo..Bar + f : Foo..Bar "# ), indoc!( r#" ── DOUBLE DOT ────────────────────────────────────────────────────────────────── - + I encountered two dots in a row: - - 1│ f : Foo..Bar + + 1│ f : Foo..Bar ^ - + Try removing one of them. "# ), @@ -4479,12 +4479,12 @@ mod test_reporting { indoc!( r#" ── TRAILING DOT ──────────────────────────────────────────────────────────────── - + I encountered a dot with nothing after it: - + 1│ f : Foo.Bar. ^ - + Dots are used to refer to a type in a qualified way, like Num.I64 or List.List a. Try adding a type name next. "# @@ -4499,19 +4499,19 @@ mod test_reporting { report_problem_as( indoc!( r#" - f : . + f : . "# ), indoc!( r#" ── UNFINISHED PARENTHESES ────────────────────────────────────────────────────── - + I am partway through parsing a type in parentheses, but I got stuck here: - + 1│ f : ( I64 ^ - + I was expecting to see a closing parenthesis before this, so try adding a ) and see if that helps? "# @@ -4530,12 +4530,12 @@ mod test_reporting { indoc!( r#" ── WEIRD QUALIFIED NAME ──────────────────────────────────────────────────────── - + I encountered a number at the start of a qualified name segment: - + 1│ f : Foo.1 ^ - + All parts of a qualified type name must start with an uppercase letter, like Num.I64 or List.List a. "# @@ -4554,13 +4554,13 @@ mod test_reporting { indoc!( r#" ── WEIRD QUALIFIED NAME ──────────────────────────────────────────────────────── - + I encountered a lowercase letter at the start of a qualified name segment: - + 1│ f : Foo.foo ^ - + All parts of a qualified type name must start with an uppercase letter, like Num.I64 or List.List a. "# @@ -4573,7 +4573,7 @@ mod test_reporting { report_problem_as( indoc!( r#" - f : I64 as + f : I64 as f = 0 f @@ -4582,12 +4582,12 @@ mod test_reporting { indoc!( r#" ── UNFINISHED INLINE ALIAS ───────────────────────────────────────────────────── - + I just started parsing an inline type alias, but I got stuck here: - - 1│ f : I64 as + + 1│ f : I64 as ^ - + Note: I may be confused by indentation "# ), @@ -4599,7 +4599,7 @@ mod test_reporting { report_problem_as( indoc!( r#" - f : I64,,I64 -> I64 + f : I64,,I64 -> I64 f = 0 f @@ -4608,13 +4608,13 @@ mod test_reporting { indoc!( r#" ── DOUBLE COMMA ──────────────────────────────────────────────────────────────── - + I just started parsing a function argument type, but I encounterd two commas in a row: - - 1│ f : I64,,I64 -> I64 + + 1│ f : I64,,I64 -> I64 ^ - + Try removing one of them. "# ), @@ -4626,7 +4626,7 @@ mod test_reporting { report_problem_as( indoc!( r#" - f : I64, I64 + f : I64, I64 f = 0 f @@ -4635,12 +4635,12 @@ mod test_reporting { indoc!( r#" ── UNFINISHED TYPE ───────────────────────────────────────────────────────────── - + I just started parsing a type, but I got stuck here: - - 1│ f : I64, I64 + + 1│ f : I64, I64 ^ - + Note: I may be confused by indentation "# ), @@ -4653,7 +4653,7 @@ mod test_reporting { report_problem_as( indoc!( r#" - f : I64, I64 -> + f : I64, I64 -> f = 0 f @@ -4662,12 +4662,12 @@ mod test_reporting { indoc!( r#" ── UNFINISHED TYPE ───────────────────────────────────────────────────────────── - + I just started parsing a type, but I got stuck here: - - 1│ f : I64, I64 -> + + 1│ f : I64, I64 -> ^ - + Note: I may be confused by indentation "# ), @@ -4680,7 +4680,7 @@ mod test_reporting { report_problem_as( indoc!( r#" - f : [ @Foo Bool, @100 I64 ] + f : [ @Foo Bool, @100 I64 ] f = 0 f @@ -4689,14 +4689,14 @@ mod test_reporting { indoc!( r#" ── WEIRD TAG NAME ────────────────────────────────────────────────────────────── - + I am partway through parsing a tag union type, but I got stuck here: - - 1│ f : [ @Foo Bool, @100 I64 ] + + 1│ f : [ @Foo Bool, @100 I64 ] ^ - + I was expecting to see a private tag name. - + Hint: Private tag names start with an `@` symbol followed by an uppercase letter, like @UID or @SecretKey. "# @@ -4719,22 +4719,59 @@ mod test_reporting { indoc!( r#" ── TYPE MISMATCH ─────────────────────────────────────────────────────────────── - + Something is off with the body of the `myDict` definition: - + 1│ myDict : Dict I64 Str 2│ myDict = Dict.insert Dict.empty "foo" 42 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - + This `insert` call produces: - + Dict Str (Num a) - + But the type annotation on `myDict` says it should be: - + Dict I64 Str "# ), ) } + + #[test] + fn alias_type_diff() { + report_problem_as( + indoc!( + r#" + HSet a : Set a + + foo : Str -> HSet {} + + myDict : HSet Str + myDict = foo "bar" + + myDict + "# + ), + indoc!( + r#" + ── TYPE MISMATCH ─────────────────────────────────────────────────────────────── + + Something is off with the body of the `myDict` definition: + + 5│ myDict : HSet Str + 6│ myDict = foo "bar" + ^^^^^^^^^ + + This `foo` call produces: + + HSet {} + + But the type annotation on `myDict` says it should be: + + HSet Str + "# + ), + ) + } } diff --git a/compiler/unify/src/unify.rs b/compiler/unify/src/unify.rs index 96935b1e90..8e4689657d 100644 --- a/compiler/unify/src/unify.rs +++ b/compiler/unify/src/unify.rs @@ -190,7 +190,9 @@ fn unify_alias( problems.extend(unify_pool(subs, pool, *l_var, *r_var)); } - problems.extend(merge(subs, &ctx, other_content.clone())); + if problems.is_empty() { + problems.extend(merge(subs, &ctx, other_content.clone())); + } if problems.is_empty() { problems.extend(unify_pool(subs, pool, real_var, *other_real_var)); diff --git a/examples/benchmarks/Closure1.roc b/examples/benchmarks/Closure1.roc new file mode 100644 index 0000000000..6065effa90 --- /dev/null +++ b/examples/benchmarks/Closure1.roc @@ -0,0 +1,15 @@ +app "closure1" + packages { base: "platform" } + imports [base.Task] + provides [ main ] to base + +# see https://github.com/rtfeldman/roc/issues/985 + +main : Task.Task {} [] +main = + Task.succeed (foo toUnitBorrowed "a long string such that it's malloced") + |> Task.map (\_ -> {}) + +toUnitBorrowed = \x -> Str.countGraphemes x + +foo = \f, x -> f x diff --git a/examples/benchmarks/Closure2.roc b/examples/benchmarks/Closure2.roc new file mode 100644 index 0000000000..338735f813 --- /dev/null +++ b/examples/benchmarks/Closure2.roc @@ -0,0 +1,15 @@ +app "closure2" + packages { base: "platform" } + imports [base.Task] + provides [ main ] to base + +# see https://github.com/rtfeldman/roc/issues/985 + +main : Task.Task {} [] +main = + x : Str + x = "a long string such that it's malloced" + + Task.succeed {} + |> Task.map (\_ -> x) + |> Task.map (\_ -> {}) diff --git a/examples/benchmarks/Closure3.roc b/examples/benchmarks/Closure3.roc new file mode 100644 index 0000000000..e890c0579a --- /dev/null +++ b/examples/benchmarks/Closure3.roc @@ -0,0 +1,15 @@ +app "closure3" + packages { base: "platform" } + imports [base.Task] + provides [ main ] to base + +# see https://github.com/rtfeldman/roc/issues/985 + +main : Task.Task {} [] +main = + x : Str + x = "a long string such that it's malloced" + + Task.succeed {} + |> Task.after (\_ -> Task.succeed x |> Task.map (\_ -> {})) + diff --git a/examples/benchmarks/Closure4.roc b/examples/benchmarks/Closure4.roc new file mode 100644 index 0000000000..5945e8897c --- /dev/null +++ b/examples/benchmarks/Closure4.roc @@ -0,0 +1,16 @@ +app "closure4" + packages { base: "platform" } + imports [base.Task] + provides [ main ] to base + +# see https://github.com/rtfeldman/roc/issues/985 + +main : Task.Task {} [] +main = + x : Str + x = "a long string such that it's malloced" + + Task.succeed {} + |> Task.after (\_ -> Task.succeed x) + |> Task.map (\_ -> {}) + diff --git a/examples/benchmarks/platform/Task.roc b/examples/benchmarks/platform/Task.roc index 0a469829e8..28523e54e2 100644 --- a/examples/benchmarks/platform/Task.roc +++ b/examples/benchmarks/platform/Task.roc @@ -24,10 +24,10 @@ after = \effect, transform -> map : Task a err, (a -> b) -> Task b err map = \effect, transform -> - Effect.after effect \result -> + Effect.map effect \result -> when result is - Ok a -> Task.succeed (transform a) - Err err -> Effect.always (Err err) # Task.fail err does not work. WEIRD! + Ok a -> Ok (transform a) + Err err -> Err err putLine : Str -> Task {} * putLine = \line -> Effect.map (Effect.putLine line) (\_ -> Ok {}) diff --git a/roc-for-elm-programmers.md b/roc-for-elm-programmers.md index 45a261f3a7..9742d08bf2 100644 --- a/roc-for-elm-programmers.md +++ b/roc-for-elm-programmers.md @@ -955,7 +955,7 @@ Either way, you get `+` being able to work on both integers and floats! Separately, there's also `Int a`, which is a type alias for `Num (Integer a)`, and `Float a`, which is a type alias for `Num (Float a)`. These allow functions -that can work on any integer or any flaot. For example, +that can work on any integer or any float. For example, `Num.bitwiseAnd : Int a, Int a -> Int a`. In Roc, number literals with decimal points are `Float *` values. @@ -984,7 +984,7 @@ These don't exist in Roc. Also [like Python](https://www.python.org/dev/peps/pep-0515/) Roc permits underscores in number literals for readability purposes. Roc also supports hexadecimal (`0x01`), octal (`0o01`), and binary (`0b01`) integer literals; these -literals all have type `Int` instead of `Num *`. +literals all have type `Int *` instead of `Num *`. If you put these into a hypothetical Roc REPL, here's what you'd see: @@ -993,16 +993,16 @@ If you put these into a hypothetical Roc REPL, here's what you'd see: 2048 : Num * > 1 + 2.14 -3.14 : Float +3.14 : Float * > 1.0 + 1 -2.0 : Float +2.0 : Float * > 1.1 + 0x11 - + > 11 + 0x11 -28 : Int +28 : Int * ``` ## Phantom Types