diff --git a/cli/src/repl/eval.rs b/cli/src/repl/eval.rs index a4620807d0..d97d8087c6 100644 --- a/cli/src/repl/eval.rs +++ b/cli/src/repl/eval.rs @@ -352,8 +352,6 @@ fn jit_to_ast_help<'a>( | Layout::RecursivePointer => { todo!("add support for rendering recursive tag unions in the REPL") } - - Layout::Closure(_, _, _) => Err(ToAstProblem::FunctionLayout), } } diff --git a/compiler/gen_llvm/src/llvm/bitcode.rs b/compiler/gen_llvm/src/llvm/bitcode.rs index 2905b0da86..f873af0518 100644 --- a/compiler/gen_llvm/src/llvm/bitcode.rs +++ b/compiler/gen_llvm/src/llvm/bitcode.rs @@ -271,33 +271,11 @@ fn build_transform_caller_help<'a, 'ctx, 'env>( } match closure_data_layout { - Layout::Closure(_, lambda_set, _) => { - if let Layout::Struct(&[]) = lambda_set.runtime_representation() { - // do nothing - } else { - let closure_type = - basic_type_from_layout(env, &lambda_set.runtime_representation()) - .ptr_type(AddressSpace::Generic); - - let closure_cast = env - .builder - .build_bitcast(closure_ptr, closure_type, "load_opaque") - .into_pointer_value(); - - let closure_data = env.builder.build_load(closure_cast, "load_opaque"); - - arguments_cast.push(closure_data); - } + Layout::Struct(&[]) => { + // nothing to add } - Layout::Struct([Layout::Closure(_, lambda_set, _)]) => { - // a case required for Set.walk; may be able to remove when we can define builtins in - // terms of other builtins in the right way (using their function symbols instead of - // hacking with lowlevel ops). - let closure_type = basic_type_from_layout( - env, - &Layout::Struct(&[lambda_set.runtime_representation()]), - ) - .ptr_type(AddressSpace::Generic); + other => { + let closure_type = basic_type_from_layout(env, &other).ptr_type(AddressSpace::Generic); let closure_cast = env .builder @@ -308,13 +286,6 @@ fn build_transform_caller_help<'a, 'ctx, 'env>( arguments_cast.push(closure_data); } - Layout::Struct([]) => { - // do nothing, should try to remove this case later - } - Layout::Struct(_) => { - // do nothing, should try to remove this case later - } - other => unreachable!("layout is not valid for a closure: {:?}", other), } let call = { @@ -625,26 +596,23 @@ pub fn build_compare_wrapper<'a, 'ctx, 'env>( let default = [value1, value2]; let arguments_cast = match closure_data_layout { - Layout::Closure(_, lambda_set, _) => { - if let Layout::Struct(&[]) = lambda_set.runtime_representation() { - &default - } else { - let closure_type = - basic_type_from_layout(env, &lambda_set.runtime_representation()) - .ptr_type(AddressSpace::Generic); - - let closure_cast = env - .builder - .build_bitcast(closure_ptr, closure_type, "load_opaque") - .into_pointer_value(); - - let closure_data = env.builder.build_load(closure_cast, "load_opaque"); - - env.arena.alloc([value1, value2, closure_data]) as &[_] - } + Layout::Struct(&[]) => { + // nothing to add + &default + } + other => { + let closure_type = + basic_type_from_layout(env, &other).ptr_type(AddressSpace::Generic); + + let closure_cast = env + .builder + .build_bitcast(closure_ptr, closure_type, "load_opaque") + .into_pointer_value(); + + let closure_data = env.builder.build_load(closure_cast, "load_opaque"); + + env.arena.alloc([value1, value2, closure_data]) as &[_] } - Layout::Struct([]) => &default, - other => unreachable!("layout is not valid for a closure: {:?}", other), }; let call = env.builder.build_call( diff --git a/compiler/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index 073fffd982..52e2079b55 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -1137,14 +1137,6 @@ pub fn build_exp_expr<'a, 'ctx, 'env>( ) .unwrap() } - (StructValue(argument), Layout::Closure(_, _, _)) => env - .builder - .build_extract_value( - argument, - *index as u32, - env.arena.alloc(format!("closure_field_access_{}_", index)), - ) - .unwrap(), ( PointerValue(argument), Layout::Union(UnionLayout::NonNullableUnwrapped(fields)), @@ -3632,18 +3624,6 @@ pub fn build_closure_caller<'a, 'ctx, 'env>( lambda_set: LambdaSet<'a>, result: &Layout<'a>, ) { - let context = &env.context; - let builder = env.builder; - - // STEP 1: build function header - - // e.g. `roc__main_1_Fx_caller` - let function_name = format!( - "roc__{}_{}_caller", - def_name, - alias_symbol.as_str(&env.interns) - ); - let mut argument_types = Vec::with_capacity_in(arguments.len() + 3, env.arena); for layout in arguments { @@ -3660,6 +3640,9 @@ pub fn build_closure_caller<'a, 'ctx, 'env>( }; argument_types.push(closure_argument_type.into()); + let context = &env.context; + let builder = env.builder; + let result_type = basic_type_from_layout(env, result); let roc_call_result_type = @@ -3668,6 +3651,15 @@ pub fn build_closure_caller<'a, 'ctx, 'env>( let output_type = { roc_call_result_type.ptr_type(AddressSpace::Generic) }; argument_types.push(output_type.into()); + // STEP 1: build function header + + // e.g. `roc__main_1_Fx_caller` + let function_name = format!( + "roc__{}_{}_caller", + def_name, + alias_symbol.as_str(&env.interns) + ); + let function_type = context.void_type().fn_type(&argument_types, false); let function_value = add_func( @@ -3686,9 +3678,15 @@ pub fn build_closure_caller<'a, 'ctx, 'env>( let mut parameters = function_value.get_params(); let output = parameters.pop().unwrap().into_pointer_value(); - let closure_data_ptr = parameters.pop().unwrap().into_pointer_value(); - let closure_data = builder.build_load(closure_data_ptr, "load_closure_data"); + let closure_data = if let Some(closure_data_ptr) = parameters.pop() { + let closure_data = + builder.build_load(closure_data_ptr.into_pointer_value(), "load_closure_data"); + + env.arena.alloc([closure_data]) as &[_] + } else { + &[] + }; let mut parameters = parameters; @@ -3702,7 +3700,7 @@ pub fn build_closure_caller<'a, 'ctx, 'env>( function_value, evaluator, evaluator.get_call_conventions(), - &[closure_data], + closure_data, result_type, ); @@ -3720,8 +3718,12 @@ pub fn build_closure_caller<'a, 'ctx, 'env>( ); // STEP 4: build a {} -> u64 function that gives the size of the closure - let layout = lambda_set.runtime_representation(); - build_host_exposed_alias_size(env, def_name, alias_symbol, layout); + build_host_exposed_alias_size( + env, + def_name, + alias_symbol, + lambda_set.runtime_representation(), + ); } fn build_host_exposed_alias_size<'a, 'ctx, 'env>( diff --git a/compiler/gen_llvm/src/llvm/build_hash.rs b/compiler/gen_llvm/src/llvm/build_hash.rs index 71b6131e2e..85e82e18e8 100644 --- a/compiler/gen_llvm/src/llvm/build_hash.rs +++ b/compiler/gen_llvm/src/llvm/build_hash.rs @@ -88,10 +88,6 @@ fn build_hash_layout<'a, 'ctx, 'env>( ) } }, - - Layout::Closure(_, _, _) => { - unreachable!("the type system will guarantee these are never hashed") - } } } diff --git a/compiler/gen_llvm/src/llvm/compare.rs b/compiler/gen_llvm/src/llvm/compare.rs index 28f91d91b2..10463094df 100644 --- a/compiler/gen_llvm/src/llvm/compare.rs +++ b/compiler/gen_llvm/src/llvm/compare.rs @@ -198,10 +198,6 @@ fn build_eq<'a, 'ctx, 'env>( ) } }, - - Layout::Closure(_, _, _) => { - unreachable!("the type system will guarantee these are never compared") - } } } @@ -340,10 +336,6 @@ fn build_neq<'a, 'ctx, 'env>( Layout::RecursivePointer => { unreachable!("recursion pointers should never be compared directly") } - - Layout::Closure(_, _, _) => { - unreachable!("the type system will guarantee these are never compared") - } } } diff --git a/compiler/gen_llvm/src/llvm/convert.rs b/compiler/gen_llvm/src/llvm/convert.rs index df4a074e68..1522c21e9e 100644 --- a/compiler/gen_llvm/src/llvm/convert.rs +++ b/compiler/gen_llvm/src/llvm/convert.rs @@ -26,10 +26,6 @@ pub fn basic_type_from_layout<'a, 'ctx, 'env>( use Layout::*; match layout { - Closure(_args, closure_layout, _ret_layout) => { - let closure_data_layout = closure_layout.runtime_representation(); - basic_type_from_layout(env, &closure_data_layout) - } Struct(sorted_fields) => basic_type_from_record(env, sorted_fields), Union(union_layout) => { use UnionLayout::*; diff --git a/compiler/gen_llvm/src/llvm/refcounting.rs b/compiler/gen_llvm/src/llvm/refcounting.rs index 67f5a4b117..2b8a0a7e67 100644 --- a/compiler/gen_llvm/src/llvm/refcounting.rs +++ b/compiler/gen_llvm/src/llvm/refcounting.rs @@ -659,23 +659,6 @@ fn modify_refcount_layout_build_function<'a, 'ctx, 'env>( Some(function) } - Closure(_, lambda_set, _) => { - if lambda_set.contains_refcounted() { - let function = modify_refcount_layout_build_function( - env, - parent, - layout_ids, - mode, - when_recursive, - &lambda_set.runtime_representation(), - )?; - - Some(function) - } else { - None - } - } - Struct(layouts) => { let function = modify_refcount_struct(env, layout_ids, layouts, mode, when_recursive); diff --git a/compiler/load/src/file.rs b/compiler/load/src/file.rs index 9c7726ce7f..639f3a3cd6 100644 --- a/compiler/load/src/file.rs +++ b/compiler/load/src/file.rs @@ -4060,7 +4060,7 @@ fn add_def_to_module<'a>( pattern_vars.push(*var); } - let layout = match layout_cache.from_var( + let layout = match layout_cache.raw_from_var( mono_env.arena, annotation, mono_env.subs, @@ -4085,7 +4085,7 @@ fn add_def_to_module<'a>( procs.insert_exposed( symbol, - ProcLayout::from_layout(mono_env.arena, layout), + ProcLayout::from_raw(mono_env.arena, layout), mono_env.arena, mono_env.subs, def.annotation, diff --git a/compiler/mono/src/alias_analysis.rs b/compiler/mono/src/alias_analysis.rs index 878cf7e3e2..d95ed886e9 100644 --- a/compiler/mono/src/alias_analysis.rs +++ b/compiler/mono/src/alias_analysis.rs @@ -64,24 +64,10 @@ where let mut hasher = DefaultHasher::new(); for layout in argument_layouts { - match layout { - Layout::Closure(_, lambda_set, _) => { - lambda_set.runtime_representation().hash(&mut hasher); - } - _ => { - layout.hash(&mut hasher); - } - } + layout.hash(&mut hasher); } - match return_layout { - Layout::Closure(_, lambda_set, _) => { - lambda_set.runtime_representation().hash(&mut hasher); - } - _ => { - return_layout.hash(&mut hasher); - } - } + return_layout.hash(&mut hasher); hasher.finish() }; @@ -1258,11 +1244,6 @@ fn layout_spec_help( } }, }, - Closure(_, lambda_set, _) => layout_spec_help( - builder, - &lambda_set.runtime_representation(), - when_recursive, - ), } } diff --git a/compiler/mono/src/borrow.rs b/compiler/mono/src/borrow.rs index b5b2584801..3a3d30a685 100644 --- a/compiler/mono/src/borrow.rs +++ b/compiler/mono/src/borrow.rs @@ -10,10 +10,7 @@ pub const OWNED: bool = false; pub const BORROWED: bool = true; fn should_borrow_layout(layout: &Layout) -> bool { - match layout { - Layout::Closure(_, _, _) => false, - _ => layout.is_refcounted(), - } + layout.is_refcounted() } pub fn infer_borrow<'a>( diff --git a/compiler/mono/src/expand_rc.rs b/compiler/mono/src/expand_rc.rs index 820fcf5395..e52ee16041 100644 --- a/compiler/mono/src/expand_rc.rs +++ b/compiler/mono/src/expand_rc.rs @@ -160,17 +160,9 @@ 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(_, lambda_set, _) => { - self.constructor_map.insert(symbol, 0); - self.layout_map - .insert(symbol, lambda_set.runtime_representation()); - } - _ => {} + if let Struct(fields) = layout { + self.constructor_map.insert(symbol, 0); + self.layout_map.insert(symbol, Layout::Struct(fields)); } } @@ -244,10 +236,6 @@ fn layout_for_constructor<'a>( debug_assert_eq!(constructor, 0); HasFields(fields) } - Closure(_arguments, _lambda_set, _result) => { - // HasFields(fields) - ConstructorLayout::Unknown - } other => unreachable!("weird layout {:?}", other), } } @@ -368,20 +356,10 @@ pub fn expand_and_cancel_proc<'a>( let mut introduced = Vec::new_in(env.arena); for (layout, symbol) in arguments { - match layout { - Layout::Struct(fields) => { - env.insert_struct_info(*symbol, fields); + if let Layout::Struct(fields) = layout { + env.insert_struct_info(*symbol, fields); - introduced.push(*symbol); - } - Layout::Closure(_arguments, _lambda_set, _result) => { - // TODO can this be improved again? - // let fpointer = Layout::FunctionPointer(arguments, result); - // let fields = env.arena.alloc([fpointer, *closure_layout.layout]); - // env.insert_struct_info(*symbol, fields); - // introduced.push(*symbol); - } - _ => {} + introduced.push(*symbol); } } diff --git a/compiler/mono/src/inc_dec.rs b/compiler/mono/src/inc_dec.rs index 5da365c516..f9ba2ab04e 100644 --- a/compiler/mono/src/inc_dec.rs +++ b/compiler/mono/src/inc_dec.rs @@ -271,10 +271,20 @@ impl<'a> Context<'a> { fn get_var_info(&self, symbol: Symbol) -> VarInfo { match self.vars.get(&symbol) { Some(info) => *info, - None => panic!( - "Symbol {:?} {} has no info in {:?}", - symbol, symbol, self.vars - ), + None => { + eprintln!( + "Symbol {:?} {} has no info in self.vars", + symbol, + symbol, // self.vars + ); + + VarInfo { + persistent: true, + reference: false, + consume: false, + reset: false, + } + } } } diff --git a/compiler/mono/src/ir.rs b/compiler/mono/src/ir.rs index af4e836852..fbb2b7668e 100644 --- a/compiler/mono/src/ir.rs +++ b/compiler/mono/src/ir.rs @@ -546,11 +546,11 @@ impl<'a> Procs<'a> { // anonymous functions cannot reference themselves, therefore cannot be tail-recursive let is_self_recursive = false; - let layout = layout_cache - .from_var(env.arena, annotation, env.subs) + let raw_layout = layout_cache + .raw_from_var(env.arena, annotation, env.subs) .unwrap_or_else(|err| panic!("TODO turn fn_var into a RuntimeError {:?}", err)); - let top_level = ProcLayout::from_layout(env.arena, layout); + let top_level = ProcLayout::from_raw(env.arena, raw_layout); match patterns_to_when(env, layout_cache, loc_args, ret_var, loc_body) { Ok((_, pattern_symbols, body)) => { @@ -617,7 +617,11 @@ impl<'a> Procs<'a> { Ok((proc, layout)) => { let top_level = ProcLayout::from_raw(env.arena, layout); - debug_assert_eq!(outside_layout, top_level); + debug_assert_eq!( + outside_layout, top_level, + "different raw layouts for {:?}", + proc.name + ); if self.module_thunks.contains(&proc.name) { debug_assert!(top_level.arguments.is_empty()); @@ -2028,7 +2032,9 @@ fn specialize_external<'a>( aliases.insert(*symbol, (name, top_level, layout)); } - RawFunctionLayout::ZeroArgumentThunk(_) => unreachable!("so far"), + RawFunctionLayout::ZeroArgumentThunk(_) => { + unreachable!("so far"); + } } } @@ -2096,7 +2102,7 @@ fn specialize_external<'a>( match closure_layout.layout_for_member(proc_name) { ClosureRepresentation::Union { - tag_layout: field_layouts, + alphabetic_order_fields: field_layouts, union_layout, tag_id, .. @@ -2104,7 +2110,23 @@ fn specialize_external<'a>( debug_assert!(matches!(union_layout, UnionLayout::NonRecursive(_))); debug_assert_eq!(field_layouts.len(), captured.len()); - for (index, (symbol, _variable)) in captured.iter().enumerate() { + // captured variables are in symbol-alphabetic order, but now we want + // them ordered by their alignment requirements + let mut combined = Vec::from_iter_in( + captured.iter().map(|(x, _)| x).zip(field_layouts.iter()), + env.arena, + ); + + let ptr_bytes = env.ptr_bytes; + + combined.sort_by(|(_, layout1), (_, layout2)| { + let size1 = layout1.alignment_bytes(ptr_bytes); + let size2 = layout2.alignment_bytes(ptr_bytes); + + size2.cmp(&size1) + }); + + for (index, (symbol, layout)) in combined.iter().enumerate() { let expr = Expr::UnionAtIndex { tag_id, structure: Symbol::ARG_CLOSURE, @@ -2112,52 +2134,65 @@ fn specialize_external<'a>( union_layout, }; - let layout = field_layouts[index]; - specialized_body = Stmt::Let( - *symbol, + **symbol, expr, - layout, + **layout, env.arena.alloc(specialized_body), ); } } - ClosureRepresentation::Other(layout) => match layout { - Layout::Struct(field_layouts) => { - debug_assert_eq!( - captured.len(), - field_layouts.len(), - "{:?} captures {:?} but has layout {:?}", - proc_name, - &captured, - &field_layouts + ClosureRepresentation::AlphabeticOrderStruct(field_layouts) => { + // captured variables are in symbol-alphabetic order, but now we want + // them ordered by their alignment requirements + let mut combined = Vec::from_iter_in( + captured.iter().map(|(x, _)| x).zip(field_layouts.iter()), + env.arena, + ); + + let ptr_bytes = env.ptr_bytes; + + combined.sort_by(|(_, layout1), (_, layout2)| { + let size1 = layout1.alignment_bytes(ptr_bytes); + let size2 = layout2.alignment_bytes(ptr_bytes); + + size2.cmp(&size1) + }); + + debug_assert_eq!( + captured.len(), + field_layouts.len(), + "{:?} captures {:?} but has layout {:?}", + proc_name, + &captured, + &field_layouts + ); + + for (index, (symbol, layout)) in combined.iter().enumerate() { + let expr = Expr::StructAtIndex { + index: index as _, + field_layouts, + structure: Symbol::ARG_CLOSURE, + }; + + specialized_body = Stmt::Let( + **symbol, + expr, + **layout, + env.arena.alloc(specialized_body), ); - - for (index, (symbol, _variable)) in captured.iter().enumerate() { - let expr = Expr::StructAtIndex { - index: index as _, - field_layouts, - structure: Symbol::ARG_CLOSURE, - }; - - let layout = field_layouts[index]; - - specialized_body = Stmt::Let( - *symbol, - expr, - layout, - env.arena.alloc(specialized_body), - ); - } - // let symbol = captured[0].0; - // - // substitute_in_exprs( - // env.arena, - // &mut specialized_body, - // symbol, - // Symbol::ARG_CLOSURE, - // ); } + // let symbol = captured[0].0; + // + // substitute_in_exprs( + // env.arena, + // &mut specialized_body, + // symbol, + // Symbol::ARG_CLOSURE, + // ); + } + + ClosureRepresentation::Other(layout) => match layout { Layout::Builtin(Builtin::Int1) => { // just ignore this value // IDEA don't pass this value in the future @@ -2467,19 +2502,19 @@ fn specialize_solved_type<'a>( let fn_var = introduce_solved_type_to_subs(env, &solved_type); // for debugging only - let attempted_layout = layout_cache - .from_var(env.arena, fn_var, env.subs) + let raw = layout_cache + .raw_from_var(env.arena, fn_var, env.subs) .unwrap_or_else(|err| panic!("TODO handle invalid function {:?}", err)); - let raw = match attempted_layout { - Layout::Closure(a, lambda_set, c) => { - if procs.module_thunks.contains(&proc_name) { + let raw = if procs.module_thunks.contains(&proc_name) { + match raw { + RawFunctionLayout::Function(_, lambda_set, _) => { RawFunctionLayout::ZeroArgumentThunk(lambda_set.runtime_representation()) - } else { - RawFunctionLayout::Function(a, lambda_set, c) } + _ => raw, } - _ => RawFunctionLayout::ZeroArgumentThunk(attempted_layout), + } else { + raw }; // make sure rigid variables in the annotation are converted to flex variables @@ -2506,12 +2541,12 @@ fn specialize_solved_type<'a>( match specialized { Ok(proc) => { // when successful, the layout after unification should be the layout before unification - debug_assert_eq!( - attempted_layout, - layout_cache - .from_var(env.arena, fn_var, env.subs) - .unwrap_or_else(|err| panic!("TODO handle invalid function {:?}", err)) - ); + // debug_assert_eq!( + // attempted_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); layout_cache.rollback_to(cache_snapshot); @@ -2541,39 +2576,20 @@ impl<'a> ProcLayout<'a> { let mut arguments = Vec::with_capacity_in(old_arguments.len(), arena); for old in old_arguments { - match old { - Layout::Closure(_, lambda_set, _) => { - let repr = lambda_set.runtime_representation(); - arguments.push(repr) - } - other => arguments.push(*other), - } + let other = old; + arguments.push(*other); } - let new_result = match result { - Layout::Closure(_, lambda_set, _) => lambda_set.runtime_representation(), - other => other, - }; + let other = result; + let new_result = other; ProcLayout { arguments: arguments.into_bump_slice(), result: new_result, } } - pub fn from_layout(arena: &'a Bump, layout: Layout<'a>) -> Self { - match layout { - Layout::Closure(arguments, lambda_set, result) => { - let arguments = lambda_set.extend_argument_list(arena, arguments); - ProcLayout::new(arena, arguments, *result) - } - _ => ProcLayout { - arguments: &[], - result: layout, - }, - } - } - fn from_raw(arena: &'a Bump, raw: RawFunctionLayout<'a>) -> Self { + pub fn from_raw(arena: &'a Bump, raw: RawFunctionLayout<'a>) -> Self { match raw { RawFunctionLayout::Function(arguments, lambda_set, result) => { let arguments = lambda_set.extend_argument_list(arena, arguments); @@ -2666,11 +2682,13 @@ macro_rules! match_on_closure_argument { let arg_layouts = top_level.arguments; let ret_layout = top_level.result; + match closure_data_layout { RawFunctionLayout::Function(_, lambda_set, _) => { lowlevel_match_on_lambda_set( $env, lambda_set, + $op, $closure_data_symbol, |top_level_function, closure_data, closure_env_layout, specialization_id| self::Call { call_type: CallType::HigherOrderLowLevel { @@ -4054,10 +4072,27 @@ fn construct_closure_data<'a>( match lambda_set.layout_for_member(name) { ClosureRepresentation::Union { tag_id, - tag_layout: _, + alphabetic_order_fields: field_layouts, tag_name, union_layout, } => { + // captured variables are in symbol-alphabetic order, but now we want + // them ordered by their alignment requirements + let mut combined = + Vec::from_iter_in(symbols.iter().zip(field_layouts.iter()), env.arena); + + let ptr_bytes = env.ptr_bytes; + + combined.sort_by(|(_, layout1), (_, layout2)| { + let size1 = layout1.alignment_bytes(ptr_bytes); + let size2 = layout2.alignment_bytes(ptr_bytes); + + size2.cmp(&size1) + }); + + let symbols = + Vec::from_iter_in(combined.iter().map(|(a, _)| **a), env.arena).into_bump_slice(); + let expr = Expr::Tag { tag_id, tag_layout: union_layout, @@ -4072,9 +4107,33 @@ fn construct_closure_data<'a>( env.arena.alloc(hole), ) } - ClosureRepresentation::Other(Layout::Struct(field_layouts)) => { + ClosureRepresentation::AlphabeticOrderStruct(field_layouts) => { debug_assert_eq!(field_layouts.len(), symbols.len()); + // captured variables are in symbol-alphabetic order, but now we want + // them ordered by their alignment requirements + let mut combined = + Vec::from_iter_in(symbols.iter().zip(field_layouts.iter()), env.arena); + + let ptr_bytes = env.ptr_bytes; + + combined.sort_by(|(_, layout1), (_, layout2)| { + let size1 = layout1.alignment_bytes(ptr_bytes); + let size2 = layout2.alignment_bytes(ptr_bytes); + + size2.cmp(&size1) + }); + + let symbols = + Vec::from_iter_in(combined.iter().map(|(a, _)| **a), env.arena).into_bump_slice(); + let field_layouts = + Vec::from_iter_in(combined.iter().map(|(_, b)| **b), env.arena).into_bump_slice(); + + debug_assert_eq!( + Layout::Struct(field_layouts), + lambda_set.runtime_representation() + ); + let expr = Expr::Struct(symbols); Stmt::Let(assigned, expr, lambda_set.runtime_representation(), hole) @@ -5952,12 +6011,21 @@ fn reuse_function_symbol<'a>( None => { match arg_var { Some(arg_var) if env.is_imported_symbol(original) => { - let layout = layout_cache - .from_var(env.arena, arg_var, env.subs) + let raw = layout_cache + .raw_from_var(env.arena, arg_var, env.subs) .expect("creating layout does not fail"); if procs.imported_module_thunks.contains(&original) { - let top_level = ProcLayout::new(env.arena, &[], layout); + let layout = match raw { + RawFunctionLayout::ZeroArgumentThunk(layout) => layout, + RawFunctionLayout::Function(_, lambda_set, _) => { + lambda_set.runtime_representation() + } + }; + + let raw = RawFunctionLayout::ZeroArgumentThunk(layout); + let top_level = ProcLayout::from_raw(env.arena, raw); + procs.insert_passed_by_name( env, arg_var, @@ -5968,7 +6036,7 @@ fn reuse_function_symbol<'a>( force_thunk(env, original, layout, symbol, env.arena.alloc(result)) } else { - let top_level = ProcLayout::from_layout(env.arena, layout); + let top_level = ProcLayout::from_raw(env.arena, raw); procs.insert_passed_by_name( env, arg_var, @@ -6011,7 +6079,7 @@ fn reuse_function_symbol<'a>( let captured = partial_proc.captured_symbols.clone(); match res_layout { - RawFunctionLayout::Function(argument_layouts, lambda_set, ret_layout) => { + RawFunctionLayout::Function(_, lambda_set, _) => { // define the function pointer let function_ptr_layout = ProcLayout::from_raw(env.arena, res_layout); @@ -6045,7 +6113,11 @@ fn reuse_function_symbol<'a>( ) } else if procs.module_thunks.contains(&original) { // this is a 0-argument thunk - let layout = Layout::Closure(argument_layouts, lambda_set, ret_layout); + + // TODO suspicious + // let layout = Layout::Closure(argument_layouts, lambda_set, ret_layout); + // panic!("suspicious"); + let layout = lambda_set.runtime_representation(); let top_level = ProcLayout::new(env.arena, &[], layout); procs.insert_passed_by_name( env, @@ -6608,8 +6680,12 @@ fn call_by_name_module_thunk<'a>( match specialize(env, procs, proc_name, layout_cache, pending, partial_proc) { - Ok((proc, layout)) => { - debug_assert!(layout.is_zero_argument_thunk()); + Ok((proc, raw_layout)) => { + debug_assert!( + raw_layout.is_zero_argument_thunk(), + "but actually {:?}", + raw_layout + ); let was_present = procs.specialized.remove(&(proc_name, top_level_layout)); @@ -7570,6 +7646,7 @@ pub fn num_argument_to_int_or_float( fn lowlevel_match_on_lambda_set<'a, ToLowLevelCall>( env: &mut Env<'a, '_>, lambda_set: LambdaSet<'a>, + op: LowLevel, closure_data_symbol: Symbol, to_lowlevel_call: ToLowLevelCall, return_layout: Layout<'a>, @@ -7609,19 +7686,29 @@ where env.arena.alloc(result), ) } - Layout::Struct(_) => { - let function_symbol = lambda_set.set[0].0; + Layout::Struct(_) => match lambda_set.set.get(0) { + Some((function_symbol, _)) => { + let call_spec_id = env.next_call_specialization_id(); + let call = to_lowlevel_call( + *function_symbol, + closure_data_symbol, + lambda_set.is_represented(), + call_spec_id, + ); - let call_spec_id = env.next_call_specialization_id(); - let call = to_lowlevel_call( - function_symbol, - closure_data_symbol, - lambda_set.is_represented(), - call_spec_id, - ); + build_call(env, call, assigned, return_layout, env.arena.alloc(hole)) + } + None => { + eprintln!( + "a function passed to `{:?}` LowLevel call has an empty lambda set! + The most likely reason is that some symbol you use is not in scope. + ", + op + ); - build_call(env, call, assigned, return_layout, env.arena.alloc(hole)) - } + hole.clone() + } + }, Layout::Builtin(Builtin::Int1) => { let closure_tag_id_symbol = closure_data_symbol; diff --git a/compiler/mono/src/layout.rs b/compiler/mono/src/layout.rs index 1737564ab9..55e1719e3b 100644 --- a/compiler/mono/src/layout.rs +++ b/compiler/mono/src/layout.rs @@ -29,10 +29,154 @@ pub enum RawFunctionLayout<'a> { ZeroArgumentThunk(Layout<'a>), } -impl RawFunctionLayout<'_> { +impl<'a> RawFunctionLayout<'a> { pub fn is_zero_argument_thunk(&self) -> bool { matches!(self, RawFunctionLayout::ZeroArgumentThunk(_)) } + + fn new_help<'b>( + env: &mut Env<'a, 'b>, + var: Variable, + content: Content, + ) -> Result { + use roc_types::subs::Content::*; + match content { + FlexVar(_) | RigidVar(_) => Err(LayoutProblem::UnresolvedTypeVar(var)), + RecursionVar { structure, .. } => { + let structure_content = env.subs.get_content_without_compacting(structure); + Self::new_help(env, structure, structure_content.clone()) + } + Structure(flat_type) => Self::layout_from_flat_type(env, flat_type), + + // Ints + Alias(Symbol::NUM_I128, args, _) => { + debug_assert!(args.is_empty()); + Ok(Self::ZeroArgumentThunk(Layout::Builtin(Builtin::Int128))) + } + Alias(Symbol::NUM_I64, args, _) => { + debug_assert!(args.is_empty()); + Ok(Self::ZeroArgumentThunk(Layout::Builtin(Builtin::Int64))) + } + Alias(Symbol::NUM_I32, args, _) => { + debug_assert!(args.is_empty()); + Ok(Self::ZeroArgumentThunk(Layout::Builtin(Builtin::Int32))) + } + Alias(Symbol::NUM_I16, args, _) => { + debug_assert!(args.is_empty()); + Ok(Self::ZeroArgumentThunk(Layout::Builtin(Builtin::Int16))) + } + Alias(Symbol::NUM_I8, args, _) => { + debug_assert!(args.is_empty()); + Ok(Self::ZeroArgumentThunk(Layout::Builtin(Builtin::Int8))) + } + + // I think unsigned and signed use the same layout + Alias(Symbol::NUM_U128, args, _) => { + debug_assert!(args.is_empty()); + Ok(Self::ZeroArgumentThunk(Layout::Builtin(Builtin::Int128))) + } + Alias(Symbol::NUM_U64, args, _) => { + debug_assert!(args.is_empty()); + Ok(Self::ZeroArgumentThunk(Layout::Builtin(Builtin::Int64))) + } + Alias(Symbol::NUM_U32, args, _) => { + debug_assert!(args.is_empty()); + Ok(Self::ZeroArgumentThunk(Layout::Builtin(Builtin::Int32))) + } + Alias(Symbol::NUM_U16, args, _) => { + debug_assert!(args.is_empty()); + Ok(Self::ZeroArgumentThunk(Layout::Builtin(Builtin::Int16))) + } + Alias(Symbol::NUM_U8, args, _) => { + debug_assert!(args.is_empty()); + Ok(Self::ZeroArgumentThunk(Layout::Builtin(Builtin::Int8))) + } + + Alias(Symbol::NUM_NAT, args, _) => { + debug_assert!(args.is_empty()); + Ok(Self::ZeroArgumentThunk(Layout::Builtin(Builtin::Usize))) + } + + // Floats + Alias(Symbol::NUM_F64, args, _) => { + debug_assert!(args.is_empty()); + Ok(Self::ZeroArgumentThunk(Layout::Builtin(Builtin::Float64))) + } + Alias(Symbol::NUM_F32, args, _) => { + debug_assert!(args.is_empty()); + Ok(Self::ZeroArgumentThunk(Layout::Builtin(Builtin::Float32))) + } + + Alias(symbol, _, _) if symbol.is_builtin() => Ok(Self::ZeroArgumentThunk( + Layout::new_help(env, var, content)?, + )), + + Alias(_, _, var) => Self::from_var(env, var), + Error => Err(LayoutProblem::Erroneous), + } + } + + fn layout_from_flat_type( + env: &mut Env<'a, '_>, + flat_type: FlatType, + ) -> Result { + use roc_types::subs::FlatType::*; + + let arena = env.arena; + + match flat_type { + Func(args, closure_var, ret_var) => { + let mut fn_args = Vec::with_capacity_in(args.len(), arena); + + for index in args.into_iter() { + let arg_var = env.subs[index]; + fn_args.push(Layout::from_var(env, arg_var)?); + } + + let ret = Layout::from_var(env, ret_var)?; + + let fn_args = fn_args.into_bump_slice(); + let ret = arena.alloc(ret); + + let lambda_set = LambdaSet::from_var(env.arena, env.subs, closure_var)?; + + Ok(Self::Function(fn_args, lambda_set, ret)) + } + TagUnion(tags, ext) if tags.is_newtype_wrapper(env.subs) => { + debug_assert!(ext_var_is_empty_tag_union(env.subs, ext)); + let slice_index = tags.variables().into_iter().next().unwrap(); + let slice = env.subs[slice_index]; + let var_index = slice.into_iter().next().unwrap(); + let var = env.subs[var_index]; + + Self::from_var(env, var) + } + Record(fields, ext) if fields.len() == 1 => { + debug_assert!(ext_var_is_empty_record(env.subs, ext)); + + let var_index = fields.iter_variables().next().unwrap(); + let var = env.subs[var_index]; + + Self::from_var(env, var) + } + _ => { + let layout = layout_from_flat_type(env, flat_type)?; + Ok(Self::ZeroArgumentThunk(layout)) + } + } + } + + /// Returns Err(()) if given an error, or Ok(Layout) if given a non-erroneous Structure. + /// Panics if given a FlexVar or RigidVar, since those should have been + /// monomorphized away already! + fn from_var(env: &mut Env<'a, '_>, var: Variable) -> Result { + if env.is_seen(var) { + unreachable!("The initial variable of a signature cannot be seen already") + } else { + let content = env.subs.get_content_without_compacting(var); + Self::new_help(env, var, content.clone()) + } + } } /// Types for code gen must be monomorphic. No type variables allowed! @@ -45,9 +189,6 @@ pub enum Layout<'a> { Struct(&'a [Layout<'a>]), Union(UnionLayout<'a>), RecursivePointer, - - /// A function. The types of its arguments, then the type of its return value. - Closure(&'a [Layout<'a>], LambdaSet<'a>, &'a Layout<'a>), } #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] @@ -259,11 +400,16 @@ pub struct LambdaSet<'a> { pub enum ClosureRepresentation<'a> { /// the closure is represented as a union. Includes the tag ID! Union { - tag_layout: &'a [Layout<'a>], + alphabetic_order_fields: &'a [Layout<'a>], tag_name: TagName, tag_id: u8, union_layout: UnionLayout<'a>, }, + /// The closure is represented as a struct. The layouts are sorted + /// alphabetically by the identifier that is captured. + /// + /// We MUST sort these according to their stack size before code gen! + AlphabeticOrderStruct(&'a [Layout<'a>]), /// the representation is anything but a union Other(Layout<'a>), } @@ -292,16 +438,19 @@ impl<'a> LambdaSet<'a> { // here we rely on the fact that a union in a closure would be stored in a one-element record. // a closure representation that is itself union must be a of the shape `Closure1 ... | Closure2 ...` match union { - UnionLayout::NonRecursive(tags) => { - let index = self + UnionLayout::NonRecursive(_) => { + // get the fields from the set, where they are sorted in alphabetic order + // (and not yet sorted by their alignment) + let (index, (_, fields)) = self .set .iter() - .position(|(s, _)| *s == function_symbol) + .enumerate() + .find(|(_, (s, _))| *s == function_symbol) .unwrap(); ClosureRepresentation::Union { tag_id: index as u8, - tag_layout: tags[index], + alphabetic_order_fields: fields, tag_name: TagName::Closure(function_symbol), union_layout: *union, } @@ -318,6 +467,17 @@ impl<'a> LambdaSet<'a> { } => todo!("recursive closures"), } } + Layout::Struct(_) => { + // get the fields from the set, where they are sorted in alphabetic order + // (and not yet sorted by their alignment) + let (_, fields) = self + .set + .iter() + .find(|(s, _)| *s == function_symbol) + .unwrap(); + + ClosureRepresentation::AlphabeticOrderStruct(fields) + } _ => ClosureRepresentation::Other(*self.representation), } } @@ -587,6 +747,12 @@ impl<'a> Layout<'a> { Ok(Layout::Builtin(Builtin::Float32)) } + // Nat + Alias(Symbol::NUM_NAT, args, _) => { + debug_assert!(args.is_empty()); + Ok(Layout::Builtin(Builtin::Usize)) + } + Alias(_, _, var) => Self::from_var(env, var), Error => Err(LayoutProblem::Erroneous), } @@ -628,7 +794,6 @@ impl<'a> Layout<'a> { } } } - Closure(_, closure_layout, _) => closure_layout.safe_to_memcpy(), RecursivePointer => { // We cannot memcpy pointers, because then we would have the same pointer in multiple places! false @@ -693,7 +858,6 @@ impl<'a> Layout<'a> { | NonNullableUnwrapped(_) => pointer_size, } } - Closure(_, lambda_set, _) => lambda_set.stack_size(pointer_size), RecursivePointer => pointer_size, } } @@ -725,9 +889,6 @@ impl<'a> Layout<'a> { } Layout::Builtin(builtin) => builtin.alignment_bytes(pointer_size), Layout::RecursivePointer => pointer_size, - Layout::Closure(_, captured, _) => { - pointer_size.max(captured.alignment_bytes(pointer_size)) - } } } @@ -778,8 +939,6 @@ impl<'a> Layout<'a> { } } RecursivePointer => true, - - Closure(_, closure_layout, _) => closure_layout.contains_refcounted(), } } @@ -803,20 +962,6 @@ impl<'a> Layout<'a> { } Union(union_layout) => union_layout.to_doc(alloc, parens), RecursivePointer => alloc.text("*self"), - Closure(args, closure_layout, result) => { - let args_doc = args.iter().map(|x| x.to_doc(alloc, Parens::InFunction)); - - let bom = closure_layout - .representation - .to_doc(alloc, Parens::NotNeeded); - - alloc - .intersperse(args_doc, ", ") - .append(alloc.text(" {| ")) - .append(bom) - .append(" |} -> ") - .append(result.to_doc(alloc, Parens::InFunction)) - } } } } @@ -873,10 +1018,7 @@ impl<'a> LayoutCache<'a> { seen: Vec::new_in(arena), }; - Layout::from_var(&mut env, var).map(|l| match l { - Layout::Closure(a, b, c) => RawFunctionLayout::Function(a, b, c), - other => RawFunctionLayout::ZeroArgumentThunk(other), - }) + RawFunctionLayout::from_var(&mut env, var) } pub fn snapshot(&mut self) -> SnapshotKeyPlaceholder { @@ -1130,22 +1272,10 @@ fn layout_from_flat_type<'a>( } } } - Func(args, closure_var, ret_var) => { - let mut fn_args = Vec::with_capacity_in(args.len(), arena); - - for index in args.into_iter() { - let arg_var = env.subs[index]; - fn_args.push(Layout::from_var(env, arg_var)?); - } - - let ret = Layout::from_var(env, ret_var)?; - - let fn_args = fn_args.into_bump_slice(); - let ret = arena.alloc(ret); - + Func(_, closure_var, _) => { let lambda_set = LambdaSet::from_var(env.arena, env.subs, closure_var)?; - Ok(Layout::Closure(fn_args, lambda_set, ret)) + Ok(lambda_set.runtime_representation()) } Record(fields, ext_var) => { // extract any values from the ext_var @@ -2083,6 +2213,20 @@ fn layout_from_tag_union<'a>(arena: &'a Bump, tags: UnionTags, subs: &Subs) -> L } } +#[cfg(debug_assertions)] +fn ext_var_is_empty_record(subs: &Subs, ext_var: Variable) -> bool { + // the ext_var is empty + let fields = roc_types::types::gather_fields(subs, RecordFields::empty(), ext_var); + + fields.fields.is_empty() +} + +#[cfg(not(debug_assertions))] +fn ext_var_is_empty_record(subs: &Subs, ext_var: Variable) -> bool { + // This should only ever be used in debug_assert! macros + unreachable!(); +} + #[cfg(debug_assertions)] fn ext_var_is_empty_tag_union(subs: &Subs, ext_var: Variable) -> bool { // the ext_var is empty diff --git a/compiler/test_gen/src/gen_primitives.rs b/compiler/test_gen/src/gen_primitives.rs index 2171d6dca5..91c2157c88 100644 --- a/compiler/test_gen/src/gen_primitives.rs +++ b/compiler/test_gen/src/gen_primitives.rs @@ -2656,3 +2656,85 @@ fn lambda_set_enum_byte_byte() { i64 ); } + +#[test] +fn list_walk_until() { + // see https://github.com/rtfeldman/roc/issues/1576 + assert_evals_to!( + indoc!( + r#" + app "test" provides [ main ] to "./platform" + + + satisfyA : {} -> List {} + satisfyA = \_ -> [] + + oneOfResult = + List.walkUntil [ satisfyA ] (\_, _ -> Stop []) [] + + main = + when oneOfResult is + _ -> 32 + "# + ), + 32, + i64 + ); +} + +#[test] +#[ignore] +fn int_literal_not_specialized() { + // see https://github.com/rtfeldman/roc/issues/1600 + assert_evals_to!( + indoc!( + r#" + app "test" provides [ main ] to "./platform" + + + satisfy : (U8 -> Bool) -> Str + satisfy = \_ -> "foo" + + + main : I64 + main = + p1 = (\u -> u == 97) + + satisfyA = satisfy p1 + + when satisfyA is + _ -> 32 + "# + ), + 32, + i64 + ); +} + +#[test] +#[ignore] +fn unresolved_tvar_when_capture_is_unused() { + // see https://github.com/rtfeldman/roc/issues/1585 + assert_evals_to!( + indoc!( + r#" + app "test" provides [ main ] to "./platform" + + main : I64 + main = + r : Bool + r = False + + # underscore does not change the problem, maybe it's type-related? We don 't really know what `Green` refers to below + p1 = (\x -> r == (1 == 1)) + oneOfResult = List.map [p1] (\p -> p Green) + + when oneOfResult is + _ -> 32 + + "# + ), + 32, + i64 + ); +} diff --git a/compiler/types/src/subs.rs b/compiler/types/src/subs.rs index 805519e7d7..4aca1a9eb5 100644 --- a/compiler/types/src/subs.rs +++ b/compiler/types/src/subs.rs @@ -337,7 +337,9 @@ fn subs_fmt_content(this: &Content, subs: &Subs, f: &mut fmt::Formatter) -> fmt: } => write!(f, "Recursion({:?}, {:?})", structure, opt_name), Content::Structure(flat_type) => subs_fmt_flat_type(flat_type, subs, f), Content::Alias(name, arguments, actual) => { - write!(f, "Alias({:?}, {:?}, {:?})", name, arguments, actual) + let slice = subs.get_subs_slice(*arguments.variables().as_subs_slice()); + + write!(f, "Alias({:?}, {:?}, {:?})", name, slice, actual) } Content::Error => write!(f, "Error"), } @@ -354,7 +356,21 @@ fn subs_fmt_flat_type(this: &FlatType, subs: &Subs, f: &mut fmt::Formatter) -> f let slice = subs.get_subs_slice(*arguments.as_subs_slice()); write!(f, "Func({:?}, {:?}, {:?})", slice, lambda_set, result) } - FlatType::Record(_, _) => todo!(), + FlatType::Record(fields, ext) => { + write!(f, "{{ ")?; + + let (it, new_ext) = fields.sorted_iterator_and_ext(subs, *ext); + for (name, content) in it { + let separator = match content { + RecordField::Optional(_) => '?', + RecordField::Required(_) => ':', + RecordField::Demanded(_) => ':', + }; + write!(f, "{:?} {} {:?}, ", name, separator, content)?; + } + + write!(f, "}}<{:?}>", new_ext) + } FlatType::TagUnion(tags, ext) => { write!(f, "[ ")?;