diff --git a/compiler/gen_dev/src/generic64/storage.rs b/compiler/gen_dev/src/generic64/storage.rs index 93c3734073..c16de57440 100644 --- a/compiler/gen_dev/src/generic64/storage.rs +++ b/compiler/gen_dev/src/generic64/storage.rs @@ -580,7 +580,7 @@ impl< } let base_offset = self.claim_stack_area(sym, struct_size); - if let Layout::Struct(field_layouts) = layout { + if let Layout::Struct { field_layouts, .. } = layout { let mut current_offset = base_offset; for (field, field_layout) in fields.iter().zip(field_layouts.iter()) { self.copy_symbol_to_stack_offset(buf, current_offset, field, field_layout); diff --git a/compiler/gen_llvm/src/llvm/bitcode.rs b/compiler/gen_llvm/src/llvm/bitcode.rs index 303d45255f..d80d626036 100644 --- a/compiler/gen_llvm/src/llvm/bitcode.rs +++ b/compiler/gen_llvm/src/llvm/bitcode.rs @@ -374,7 +374,9 @@ fn build_transform_caller_help<'a, 'ctx, 'env>( } match closure_data_layout.runtime_representation() { - Layout::Struct(&[]) => { + Layout::Struct { + field_layouts: &[], .. + } => { // nothing to add } other => { @@ -694,7 +696,9 @@ pub fn build_compare_wrapper<'a, 'ctx, 'env>( let default = [value1.into(), value2.into()]; let arguments_cast = match closure_data_layout.runtime_representation() { - Layout::Struct(&[]) => { + Layout::Struct { + field_layouts: &[], .. + } => { // nothing to add &default } diff --git a/compiler/gen_llvm/src/llvm/build.rs b/compiler/gen_llvm/src/llvm/build.rs index 0f989fb0fd..1fb64493f4 100644 --- a/compiler/gen_llvm/src/llvm/build.rs +++ b/compiler/gen_llvm/src/llvm/build.rs @@ -714,8 +714,7 @@ fn promote_to_main_function<'a, 'ctx, 'env>( ); // NOTE fake layout; it is only used for debug prints - let roc_main_fn = - function_value_by_func_spec(env, *func_spec, symbol, &[], &Layout::Struct(&[])); + let roc_main_fn = function_value_by_func_spec(env, *func_spec, symbol, &[], &Layout::UNIT); let main_fn_name = "$Test.main"; @@ -1188,8 +1187,8 @@ pub fn build_exp_expr<'a, 'ctx, 'env>( // extract field from a record match (value, layout) { - (StructValue(argument), Layout::Struct(fields)) => { - debug_assert!(!fields.is_empty()); + (StructValue(argument), Layout::Struct { field_layouts, .. }) => { + debug_assert!(!field_layouts.is_empty()); let field_value = env .builder @@ -1201,14 +1200,14 @@ pub fn build_exp_expr<'a, 'ctx, 'env>( ) .unwrap(); - let field_layout = fields[*index as usize]; + let field_layout = field_layouts[*index as usize]; use_roc_value(env, field_layout, field_value, "struct_field_tag") } ( PointerValue(argument), Layout::Union(UnionLayout::NonNullableUnwrapped(fields)), ) => { - let struct_layout = Layout::Struct(fields); + let struct_layout = Layout::struct_no_name_order(fields); let struct_type = basic_type_from_layout(env, &struct_layout); let cast_argument = env @@ -1292,7 +1291,7 @@ pub fn build_exp_expr<'a, 'ctx, 'env>( ) } UnionLayout::NonNullableUnwrapped(field_layouts) => { - let struct_layout = Layout::Struct(field_layouts); + let struct_layout = Layout::struct_no_name_order(field_layouts); let struct_type = basic_type_from_layout(env, &struct_layout); @@ -1341,7 +1340,7 @@ pub fn build_exp_expr<'a, 'ctx, 'env>( debug_assert_ne!(*tag_id != 0, *nullable_id); let field_layouts = other_fields; - let struct_layout = Layout::Struct(field_layouts); + let struct_layout = Layout::struct_no_name_order(field_layouts); let struct_type = basic_type_from_layout(env, &struct_layout); @@ -2024,7 +2023,7 @@ fn lookup_at_index_ptr2<'a, 'ctx, 'env>( ) -> BasicValueEnum<'ctx> { let builder = env.builder; - let struct_layout = Layout::Struct(field_layouts); + let struct_layout = Layout::struct_no_name_order(field_layouts); let struct_type = basic_type_from_layout(env, &struct_layout); let wrapper_type = env @@ -3522,7 +3521,7 @@ fn expose_function_to_host_help_c_abi_gen_test<'a, 'ctx, 'env>( call_roc_function( env, roc_wrapper_function, - &Layout::Struct(&[Layout::u64(), return_layout]), + &Layout::struct_no_name_order(&[Layout::u64(), return_layout]), arguments_for_call, ) }; @@ -3903,7 +3902,7 @@ fn roc_result_layout<'a>( ) -> Layout<'a> { let elements = [Layout::u64(), Layout::usize(target_info), return_layout]; - Layout::Struct(arena.alloc(elements)) + Layout::struct_no_name_order(arena.alloc(elements)) } fn roc_result_type<'a, 'ctx, 'env>( @@ -5363,7 +5362,7 @@ fn run_low_level<'a, 'ctx, 'env>( let (string, _string_layout) = load_symbol_and_layout(scope, &args[0]); let number_layout = match layout { - Layout::Struct(fields) => fields[0], // TODO: why is it sometimes a struct? + Layout::Struct { field_layouts, .. } => field_layouts[0], // TODO: why is it sometimes a struct? _ => unreachable!(), }; diff --git a/compiler/gen_llvm/src/llvm/build_dict.rs b/compiler/gen_llvm/src/llvm/build_dict.rs index 37b81e9129..cbecc59b9a 100644 --- a/compiler/gen_llvm/src/llvm/build_dict.rs +++ b/compiler/gen_llvm/src/llvm/build_dict.rs @@ -735,8 +735,7 @@ pub fn set_from_list<'a, 'ctx, 'env>( let result_alloca = builder.build_alloca(zig_dict_type(env), "result_alloca"); - let alignment = - Alignment::from_key_value_layout(key_layout, &Layout::Struct(&[]), env.target_info); + let alignment = Alignment::from_key_value_layout(key_layout, &Layout::UNIT, env.target_info); let alignment_iv = alignment.as_int_value(env.context); let hash_fn = build_hash_wrapper(env, layout_ids, key_layout); diff --git a/compiler/gen_llvm/src/llvm/build_hash.rs b/compiler/gen_llvm/src/llvm/build_hash.rs index 0db5348d1d..aa89583c77 100644 --- a/compiler/gen_llvm/src/llvm/build_hash.rs +++ b/compiler/gen_llvm/src/llvm/build_hash.rs @@ -50,10 +50,10 @@ fn build_hash_layout<'a, 'ctx, 'env>( hash_builtin(env, layout_ids, seed, val, layout, builtin, when_recursive) } - Layout::Struct(fields) => build_hash_struct( + Layout::Struct { field_layouts, .. } => build_hash_struct( env, layout_ids, - fields, + field_layouts, when_recursive, seed, val.into_struct_value(), @@ -166,7 +166,7 @@ fn build_hash_struct<'a, 'ctx, 'env>( let block = env.builder.get_insert_block().expect("to be in a function"); let di_location = env.builder.get_current_debug_location().unwrap(); - let struct_layout = Layout::Struct(field_layouts); + let struct_layout = Layout::struct_no_name_order(field_layouts); let symbol = Symbol::GENERIC_HASH; let fn_name = layout_ids @@ -248,7 +248,7 @@ fn hash_struct<'a, 'ctx, 'env>( ) -> IntValue<'ctx> { let ptr_bytes = env.target_info; - let layout = Layout::Struct(field_layouts); + let layout = Layout::struct_no_name_order(field_layouts); // Optimization: if the bit representation of equal values is the same // just hash the bits. Caveat here is tags: e.g. `Nothing` in `Just a` @@ -818,7 +818,7 @@ fn hash_ptr_to_struct<'a, 'ctx, 'env>( .build_struct_gep(wrapper_ptr, TAG_DATA_INDEX, "get_tag_data") .unwrap(); - let struct_layout = Layout::Struct(field_layouts); + let struct_layout = Layout::struct_no_name_order(field_layouts); let struct_type = basic_type_from_layout(env, &struct_layout); let struct_ptr = env .builder diff --git a/compiler/gen_llvm/src/llvm/compare.rs b/compiler/gen_llvm/src/llvm/compare.rs index f5d596aae1..9f8a0928a6 100644 --- a/compiler/gen_llvm/src/llvm/compare.rs +++ b/compiler/gen_llvm/src/llvm/compare.rs @@ -161,10 +161,10 @@ fn build_eq<'a, 'ctx, 'env>( build_eq_builtin(env, layout_ids, lhs_val, rhs_val, builtin, when_recursive) } - Layout::Struct(fields) => build_struct_eq( + Layout::Struct { field_layouts, .. } => build_struct_eq( env, layout_ids, - fields, + field_layouts, when_recursive, lhs_val.into_struct_value(), rhs_val.into_struct_value(), @@ -330,11 +330,11 @@ fn build_neq<'a, 'ctx, 'env>( build_neq_builtin(env, layout_ids, lhs_val, rhs_val, builtin, when_recursive) } - Layout::Struct(fields) => { + Layout::Struct { field_layouts, .. } => { let is_equal = build_struct_eq( env, layout_ids, - fields, + field_layouts, when_recursive, lhs_val.into_struct_value(), rhs_val.into_struct_value(), @@ -587,7 +587,7 @@ fn build_struct_eq<'a, 'ctx, 'env>( let block = env.builder.get_insert_block().expect("to be in a function"); let di_location = env.builder.get_current_debug_location().unwrap(); - let struct_layout = Layout::Struct(field_layouts); + let struct_layout = Layout::struct_no_name_order(field_layouts); let symbol = Symbol::GENERIC_EQ; let fn_name = layout_ids @@ -1208,7 +1208,7 @@ fn eq_ptr_to_struct<'a, 'ctx, 'env>( tag1: PointerValue<'ctx>, tag2: PointerValue<'ctx>, ) -> IntValue<'ctx> { - let struct_layout = Layout::Struct(field_layouts); + let struct_layout = Layout::struct_no_name_order(field_layouts); let wrapper_type = basic_type_from_layout(env, &struct_layout); debug_assert!(wrapper_type.is_struct_type()); diff --git a/compiler/gen_llvm/src/llvm/convert.rs b/compiler/gen_llvm/src/llvm/convert.rs index c7d3142d40..8d516e539e 100644 --- a/compiler/gen_llvm/src/llvm/convert.rs +++ b/compiler/gen_llvm/src/llvm/convert.rs @@ -28,7 +28,10 @@ pub fn basic_type_from_layout<'a, 'ctx, 'env>( use Layout::*; match layout { - Struct(sorted_fields) => basic_type_from_record(env, sorted_fields), + Struct { + field_layouts: sorted_fields, + .. + } => basic_type_from_record(env, sorted_fields), LambdaSet(lambda_set) => basic_type_from_layout(env, &lambda_set.runtime_representation()), Union(union_layout) => { use UnionLayout::*; @@ -86,7 +89,10 @@ pub fn basic_type_from_layout_1<'a, 'ctx, 'env>( use Layout::*; match layout { - Struct(sorted_fields) => basic_type_from_record(env, sorted_fields), + Struct { + field_layouts: sorted_fields, + .. + } => basic_type_from_record(env, sorted_fields), LambdaSet(lambda_set) => { basic_type_from_layout_1(env, &lambda_set.runtime_representation()) } diff --git a/compiler/gen_llvm/src/llvm/refcounting.rs b/compiler/gen_llvm/src/llvm/refcounting.rs index 2962361c29..24bf02d093 100644 --- a/compiler/gen_llvm/src/llvm/refcounting.rs +++ b/compiler/gen_llvm/src/llvm/refcounting.rs @@ -280,7 +280,7 @@ fn modify_refcount_struct<'a, 'ctx, 'env>( let block = env.builder.get_insert_block().expect("to be in a function"); let di_location = env.builder.get_current_debug_location().unwrap(); - let layout = Layout::Struct(layouts); + let layout = Layout::struct_no_name_order(layouts); let (_, fn_name) = function_name_from_mode( layout_ids, @@ -440,7 +440,7 @@ fn modify_refcount_builtin<'a, 'ctx, 'env>( } Set(element_layout) => { let key_layout = element_layout; - let value_layout = &Layout::Struct(&[]); + let value_layout = &Layout::UNIT; let function = modify_refcount_dict( env, @@ -619,8 +619,9 @@ fn modify_refcount_layout_build_function<'a, 'ctx, 'env>( } } - Struct(layouts) => { - let function = modify_refcount_struct(env, layout_ids, layouts, mode, when_recursive); + Struct { field_layouts, .. } => { + let function = + modify_refcount_struct(env, layout_ids, &field_layouts, mode, when_recursive); Some(function) } @@ -1312,7 +1313,8 @@ fn build_rec_union_recursive_decrement<'a, 'ctx, 'env>( env.builder.position_at_end(block); - let wrapper_type = basic_type_from_layout(env, &Layout::Struct(field_layouts)); + let wrapper_type = + basic_type_from_layout(env, &Layout::struct_no_name_order(field_layouts)); // cast the opaque pointer to a pointer of the correct shape let struct_ptr = env @@ -1720,7 +1722,8 @@ fn modify_refcount_union_help<'a, 'ctx, 'env>( let block = env.context.append_basic_block(parent, "tag_id_modify"); env.builder.position_at_end(block); - let wrapper_type = basic_type_from_layout(env, &Layout::Struct(field_layouts)); + let wrapper_type = + basic_type_from_layout(env, &Layout::struct_no_name_order(field_layouts)); debug_assert!(wrapper_type.is_struct_type()); let opaque_tag_data_ptr = env diff --git a/compiler/gen_wasm/src/backend.rs b/compiler/gen_wasm/src/backend.rs index 716758fc56..d23a54ccb2 100644 --- a/compiler/gen_wasm/src/backend.rs +++ b/compiler/gen_wasm/src/backend.rs @@ -884,7 +884,7 @@ impl<'a> WasmBackend<'a> { storage: &StoredValue, fields: &'a [Symbol], ) { - if matches!(layout, Layout::Struct(_)) { + if matches!(layout, Layout::Struct { .. }) { match storage { StoredValue::StackMemory { location, size, .. } => { if *size > 0 { diff --git a/compiler/gen_wasm/src/layout.rs b/compiler/gen_wasm/src/layout.rs index b7ad18b2ef..1b70e0757a 100644 --- a/compiler/gen_wasm/src/layout.rs +++ b/compiler/gen_wasm/src/layout.rs @@ -88,7 +88,7 @@ impl WasmLayout { }, Layout::Builtin(Str | Dict(_, _) | Set(_) | List(_)) - | Layout::Struct(_) + | Layout::Struct { .. } | Layout::LambdaSet(_) | Layout::Union(NonRecursive(_)) => Self::StackMemory { size, diff --git a/compiler/gen_wasm/src/low_level.rs b/compiler/gen_wasm/src/low_level.rs index acd4a32455..a5875e1813 100644 --- a/compiler/gen_wasm/src/low_level.rs +++ b/compiler/gen_wasm/src/low_level.rs @@ -212,7 +212,7 @@ impl<'a> LowLevelCall<'a> { } StrToNum => { let number_layout = match self.ret_layout { - Layout::Struct(fields) => fields[0], + Layout::Struct { field_layouts, .. } => field_layouts[0], _ => { internal_error!("Unexpected mono layout {:?} for StrToNum", self.ret_layout) } @@ -711,7 +711,7 @@ impl<'a> LowLevelCall<'a> { // Empty record is always equal to empty record. // There are no runtime arguments to check, so just emit true or false. - Layout::Struct(fields) if fields.is_empty() => { + Layout::Struct { field_layouts, .. } if field_layouts.is_empty() => { backend.code_builder.i32_const(!invert_result as i32); } @@ -722,7 +722,7 @@ impl<'a> LowLevelCall<'a> { } Layout::Builtin(Builtin::Dict(_, _) | Builtin::Set(_) | Builtin::List(_)) - | Layout::Struct(_) + | Layout::Struct { .. } | Layout::Union(_) | Layout::LambdaSet(_) => { // Don't want Zig calling convention here, we're calling internal Roc functions diff --git a/compiler/load/src/file.rs b/compiler/load/src/file.rs index 0a119ecb23..0cfa433b12 100644 --- a/compiler/load/src/file.rs +++ b/compiler/load/src/file.rs @@ -2101,7 +2101,7 @@ fn finish_specialization( EntryPoint { layout: roc_mono::ir::ProcLayout { arguments: &[], - result: Layout::Struct(&[]), + result: Layout::struct_no_name_order(&[]), }, symbol, } diff --git a/compiler/mono/src/alias_analysis.rs b/compiler/mono/src/alias_analysis.rs index f42b554628..810e3c3595 100644 --- a/compiler/mono/src/alias_analysis.rs +++ b/compiler/mono/src/alias_analysis.rs @@ -308,7 +308,7 @@ fn build_entry_point( let block = builder.add_block(); - let type_id = layout_spec(&mut builder, &Layout::Struct(layouts))?; + let type_id = layout_spec(&mut builder, &Layout::struct_no_name_order(layouts))?; let argument = builder.add_unknown_with(block, &[], type_id)?; @@ -349,7 +349,10 @@ fn proc_spec<'a>(proc: &Proc<'a>) -> Result<(FuncDef, MutSet>)> let value_id = stmt_spec(&mut builder, &mut env, block, &proc.ret_layout, &proc.body)?; let root = BlockExpr(block, value_id); - let arg_type_id = layout_spec(&mut builder, &Layout::Struct(&argument_layouts))?; + let arg_type_id = layout_spec( + &mut builder, + &Layout::struct_no_name_order(&argument_layouts), + )?; let ret_type_id = layout_spec(&mut builder, &proc.ret_layout)?; let spec = builder.build(arg_type_id, ret_type_id, root)?; @@ -1135,7 +1138,7 @@ fn call_spec( // ListFindUnsafe returns { value: v, found: Bool=Int1 } let output_layouts = vec![argument_layouts[0], Layout::Builtin(Builtin::Bool)]; - let output_layout = Layout::Struct(&output_layouts); + let output_layout = Layout::struct_no_name_order(&output_layouts); let output_type = layout_spec(builder, &output_layout)?; let loop_body = |builder: &mut FuncDefBuilder, block, output| { @@ -1672,7 +1675,9 @@ fn layout_spec_help( match layout { Builtin(builtin) => builtin_spec(builder, builtin, when_recursive), - Struct(fields) => build_recursive_tuple_type(builder, fields, when_recursive), + Struct { field_layouts, .. } => { + build_recursive_tuple_type(builder, field_layouts, when_recursive) + } LambdaSet(lambda_set) => layout_spec_help( builder, &lambda_set.runtime_representation(), diff --git a/compiler/mono/src/code_gen_help/equality.rs b/compiler/mono/src/code_gen_help/equality.rs index d58d095274..cf4fbcff17 100644 --- a/compiler/mono/src/code_gen_help/equality.rs +++ b/compiler/mono/src/code_gen_help/equality.rs @@ -32,7 +32,7 @@ pub fn eq_generic<'a>( } Layout::Builtin(Builtin::Dict(_, _) | Builtin::Set(_)) => eq_todo(), Layout::Builtin(Builtin::List(elem_layout)) => eq_list(root, ident_ids, ctx, elem_layout), - Layout::Struct(field_layouts) => eq_struct(root, ident_ids, ctx, field_layouts), + Layout::Struct { field_layouts, .. } => eq_struct(root, ident_ids, ctx, field_layouts), Layout::Union(union_layout) => eq_tag_union(root, ident_ids, ctx, union_layout), Layout::LambdaSet(_) => unreachable!("`==` is not defined on functions"), Layout::RecursivePointer => { diff --git a/compiler/mono/src/code_gen_help/mod.rs b/compiler/mono/src/code_gen_help/mod.rs index e74e4058ed..55d99fc266 100644 --- a/compiler/mono/src/code_gen_help/mod.rs +++ b/compiler/mono/src/code_gen_help/mod.rs @@ -15,7 +15,7 @@ mod equality; mod refcount; const LAYOUT_BOOL: Layout = Layout::Builtin(Builtin::Bool); -const LAYOUT_UNIT: Layout = Layout::Struct(&[]); +const LAYOUT_UNIT: Layout = Layout::UNIT; const ARG_1: Symbol = Symbol::ARG_1; const ARG_2: Symbol = Symbol::ARG_2; @@ -354,9 +354,15 @@ impl<'a> CodeGenHelp<'a> { Layout::Builtin(_) => layout, - Layout::Struct(fields) => { - let new_fields_iter = fields.iter().map(|f| self.replace_rec_ptr(ctx, *f)); - Layout::Struct(self.arena.alloc_slice_fill_iter(new_fields_iter)) + Layout::Struct { + field_layouts, + field_order_hash, + } => { + let new_fields_iter = field_layouts.iter().map(|f| self.replace_rec_ptr(ctx, *f)); + Layout::Struct { + field_layouts: self.arena.alloc_slice_fill_iter(new_fields_iter), + field_order_hash, + } } Layout::Union(UnionLayout::NonRecursive(tags)) => { @@ -462,7 +468,7 @@ fn layout_needs_helper_proc(layout: &Layout, op: HelperOp) -> bool { Layout::Builtin(Builtin::Dict(_, _) | Builtin::Set(_) | Builtin::List(_)) => true, - Layout::Struct(fields) => !fields.is_empty(), + Layout::Struct { field_layouts, .. } => !field_layouts.is_empty(), Layout::Union(UnionLayout::NonRecursive(tags)) => !tags.is_empty(), diff --git a/compiler/mono/src/code_gen_help/refcount.rs b/compiler/mono/src/code_gen_help/refcount.rs index 272e503d18..60e15b9f73 100644 --- a/compiler/mono/src/code_gen_help/refcount.rs +++ b/compiler/mono/src/code_gen_help/refcount.rs @@ -12,7 +12,7 @@ use crate::layout::{Builtin, Layout, TagIdIntType, UnionLayout}; use super::{CodeGenHelp, Context, HelperOp}; const LAYOUT_BOOL: Layout = Layout::Builtin(Builtin::Bool); -const LAYOUT_UNIT: Layout = Layout::Struct(&[]); +const LAYOUT_UNIT: Layout = Layout::UNIT; const LAYOUT_PTR: Layout = Layout::RecursivePointer; const LAYOUT_U32: Layout = Layout::Builtin(Builtin::Int(IntWidth::U32)); @@ -69,7 +69,7 @@ pub fn refcount_stmt<'a>( } // Struct and non-recursive Unions are stack-only, so DecRef is a no-op - Layout::Struct(_) => following, + Layout::Struct { .. } => following, Layout::Union(UnionLayout::NonRecursive(_)) => following, // Inline the refcounting code instead of making a function. Don't iterate fields, @@ -111,7 +111,7 @@ pub fn refcount_generic<'a>( refcount_list(root, ident_ids, ctx, &layout, elem_layout, structure) } Layout::Builtin(Builtin::Dict(_, _) | Builtin::Set(_)) => rc_todo(), - Layout::Struct(field_layouts) => { + Layout::Struct { field_layouts, .. } => { refcount_struct(root, ident_ids, ctx, field_layouts, structure) } Layout::Union(union_layout) => { @@ -135,7 +135,7 @@ pub fn is_rc_implemented_yet(layout: &Layout) -> bool { Layout::Builtin(Builtin::Dict(..) | Builtin::Set(_)) => false, Layout::Builtin(Builtin::List(elem_layout)) => is_rc_implemented_yet(elem_layout), Layout::Builtin(_) => true, - Layout::Struct(fields) => fields.iter().all(is_rc_implemented_yet), + Layout::Struct { field_layouts, .. } => field_layouts.iter().all(is_rc_implemented_yet), Layout::Union(union_layout) => match union_layout { NonRecursive(tags) => tags .iter() diff --git a/compiler/mono/src/decision_tree.rs b/compiler/mono/src/decision_tree.rs index b37391d9cd..39c28f49d8 100644 --- a/compiler/mono/src/decision_tree.rs +++ b/compiler/mono/src/decision_tree.rs @@ -747,7 +747,11 @@ fn to_relevant_branch_help<'a>( // the test matches the constructor of this pattern match layout { - UnionLayout::NonRecursive([[Layout::Struct([_])]]) => { + UnionLayout::NonRecursive( + [[Layout::Struct { + field_layouts: [_], .. + }]], + ) => { // a one-element record equivalent // Theory: Unbox doesn't have any value for us debug_assert_eq!(arguments.len(), 1); @@ -1235,7 +1239,7 @@ fn path_to_expr_help<'a>( layout = inner_layout; } - Layout::Struct(field_layouts) => { + Layout::Struct { field_layouts, .. } => { debug_assert!(field_layouts.len() > 1); let inner_expr = Expr::StructAtIndex { diff --git a/compiler/mono/src/ir.rs b/compiler/mono/src/ir.rs index 6a4c1e5449..c1d7741233 100644 --- a/compiler/mono/src/ir.rs +++ b/compiler/mono/src/ir.rs @@ -1125,7 +1125,7 @@ impl<'a> Param<'a> { pub const EMPTY: Self = Param { symbol: Symbol::EMPTY_PARAM, borrow: false, - layout: Layout::Struct(&[]), + layout: Layout::UNIT, }; } @@ -2436,7 +2436,7 @@ fn specialize_external<'a>( let closure_data_layout = match opt_closure_layout { Some(lambda_set) => Layout::LambdaSet(lambda_set), - None => Layout::Struct(&[]), + None => Layout::UNIT, }; // I'm not sure how to handle the closure case, does it ever occur? @@ -3985,7 +3985,7 @@ pub fn with_hole<'a>( .unwrap_or_else(|err| panic!("TODO turn fn_var into a RuntimeError {:?}", err)); let field_layouts = match &record_layout { - Layout::Struct(layouts) => *layouts, + Layout::Struct { field_layouts, .. } => *field_layouts, other => arena.alloc([*other]), }; @@ -4701,7 +4701,9 @@ fn construct_closure_data<'a>( Vec::from_iter_in(combined.iter().map(|(_, b)| **b), env.arena).into_bump_slice(); debug_assert_eq!( - Layout::Struct(field_layouts), + // NB: this may be wrong! If it comes up, we may need to hash the closure + // argument name order. + Layout::struct_no_name_order(field_layouts,), lambda_set.runtime_representation() ); @@ -4785,9 +4787,7 @@ fn convert_tag_union<'a>( "The `[]` type has no constructors, source var {:?}", variant_var ), - Unit | UnitWithArguments => { - Stmt::Let(assigned, Expr::Struct(&[]), Layout::Struct(&[]), hole) - } + Unit | UnitWithArguments => Stmt::Let(assigned, Expr::Struct(&[]), Layout::UNIT, hole), BoolUnion { ttrue, .. } => Stmt::Let( assigned, Expr::Literal(Literal::Bool(tag_name == ttrue)), @@ -5096,7 +5096,7 @@ fn sorted_field_symbols<'a>( // Note it does not catch the use of `[]` currently. use roc_can::expr::Expr; arg.value = Expr::RuntimeError(RuntimeError::VoidValue); - Layout::Struct(&[]) + Layout::UNIT } Err(LayoutProblem::Erroneous) => { // something went very wrong @@ -5191,7 +5191,10 @@ fn register_capturing_closure<'a>( Content::Structure(FlatType::Func(_, closure_var, _)) => { match LambdaSet::from_var(env.arena, env.subs, closure_var, env.target_info) { Ok(lambda_set) => { - if let Layout::Struct(&[]) = lambda_set.runtime_representation() { + if let Layout::Struct { + field_layouts: &[], .. + } = lambda_set.runtime_representation() + { CapturedSymbols::None } else { let mut temp = Vec::from_iter_in(captured_symbols, env.arena); @@ -6255,7 +6258,7 @@ fn store_pattern_help<'a>( let mut fields = Vec::with_capacity_in(arguments.len(), env.arena); fields.extend(arguments.iter().map(|x| x.1)); - let layout = Layout::Struct(fields.into_bump_slice()); + let layout = Layout::struct_no_name_order(fields.into_bump_slice()); return store_newtype_pattern( env, @@ -6676,7 +6679,7 @@ fn force_thunk<'a>( } fn let_empty_struct<'a>(assigned: Symbol, hole: &'a Stmt<'a>) -> Stmt<'a> { - Stmt::Let(assigned, Expr::Struct(&[]), Layout::Struct(&[]), hole) + Stmt::Let(assigned, Expr::Struct(&[]), Layout::UNIT, hole) } /// If the symbol is a function, make sure it is properly specialized @@ -8457,7 +8460,7 @@ where env.arena.alloc(result), ) } - Layout::Struct(_) => match lambda_set.set.get(0) { + Layout::Struct { .. } => match lambda_set.set.get(0) { Some((function_symbol, _)) => { let call_spec_id = env.next_call_specialization_id(); let update_mode = env.next_update_mode_id(); @@ -8630,7 +8633,10 @@ fn match_on_lambda_set<'a>( env.arena.alloc(result), ) } - Layout::Struct(fields) => { + Layout::Struct { + field_layouts, + field_order_hash, + } => { let function_symbol = lambda_set.set[0].0; union_lambda_set_branch_help( @@ -8638,7 +8644,10 @@ fn match_on_lambda_set<'a>( function_symbol, lambda_set, closure_data_symbol, - Layout::Struct(fields), + Layout::Struct { + field_layouts, + field_order_hash, + }, argument_symbols, argument_layouts, return_layout, @@ -8797,7 +8806,9 @@ fn union_lambda_set_branch_help<'a>( hole: &'a Stmt<'a>, ) -> Stmt<'a> { let (argument_layouts, argument_symbols) = match closure_data_layout { - Layout::Struct(&[]) + Layout::Struct { + field_layouts: &[], .. + } | Layout::Builtin(Builtin::Bool) | Layout::Builtin(Builtin::Int(IntWidth::U8)) => { (argument_layouts_slice, argument_symbols_slice) @@ -8924,7 +8935,9 @@ fn enum_lambda_set_branch<'a>( let assigned = result_symbol; let (argument_layouts, argument_symbols) = match closure_data_layout { - Layout::Struct(&[]) + Layout::Struct { + field_layouts: &[], .. + } | Layout::Builtin(Builtin::Bool) | Layout::Builtin(Builtin::Int(IntWidth::U8)) => { (argument_layouts_slice, argument_symbols_slice) diff --git a/compiler/mono/src/layout.rs b/compiler/mono/src/layout.rs index b04993c9d6..abf61dd010 100644 --- a/compiler/mono/src/layout.rs +++ b/compiler/mono/src/layout.rs @@ -11,8 +11,9 @@ use roc_types::subs::{ Content, FlatType, RecordFields, Subs, UnionTags, UnsortedUnionTags, Variable, }; use roc_types::types::{gather_fields_unsorted_iter, RecordField}; -use std::collections::hash_map::Entry; +use std::collections::hash_map::{DefaultHasher, Entry}; use std::collections::HashMap; +use std::hash::{Hash, Hasher}; use ven_pretty::{DocAllocator, DocBuilder}; // if your changes cause this number to go down, great! @@ -26,6 +27,7 @@ static_assertions::assert_eq_size!([usize; 3], LambdaSet); pub type TagIdIntType = u16; pub const MAX_ENUM_SIZE: usize = (std::mem::size_of::() * 8) as usize; const GENERATE_NULLABLE: bool = true; +const IRRELEVANT_FIELD_HASH: u64 = 0; #[derive(Debug, Clone)] pub enum LayoutProblem { @@ -205,10 +207,19 @@ impl<'a> RawFunctionLayout<'a> { #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] pub enum Layout<'a> { Builtin(Builtin<'a>), - /// A layout that is empty (turns into the empty struct in LLVM IR - /// but for our purposes, not zero-sized, so it does not get dropped from data structures - /// this is important for closures that capture zero-sized values - Struct(&'a [Layout<'a>]), + Struct { + /// Two different struct types can have the same layout, for example + /// { a: U8, b: I64 } + /// { a: I64, b: U8 } + /// both have the layout {I64, U8}. Not distinguishing the order of record fields can cause + /// us problems during monomorphization when we specialize the same type in different ways, + /// so keep a hash of the record order for disambiguation. This still of course may result + /// in collisions, but it's unlikely. + /// + /// See also https://github.com/rtfeldman/roc/issues/2535. + field_order_hash: u64, + field_layouts: &'a [Layout<'a>], + }, Union(UnionLayout<'a>), LambdaSet(LambdaSet<'a>), RecursivePointer, @@ -417,7 +428,9 @@ impl<'a> UnionLayout<'a> { fn tags_alignment_bytes(tags: &[&[Layout]], target_info: TargetInfo) -> u32 { tags.iter() - .map(|fields| Layout::Struct(fields).alignment_bytes(target_info)) + .map(|field_layouts| { + Layout::struct_no_name_order(field_layouts).alignment_bytes(target_info) + }) .max() .unwrap_or(0) } @@ -426,14 +439,14 @@ impl<'a> UnionLayout<'a> { let allocation = match self { UnionLayout::NonRecursive(_) => unreachable!("not heap-allocated"), UnionLayout::Recursive(tags) => Self::tags_alignment_bytes(tags, target_info), - UnionLayout::NonNullableUnwrapped(fields) => { - Layout::Struct(fields).alignment_bytes(target_info) + UnionLayout::NonNullableUnwrapped(field_layouts) => { + Layout::struct_no_name_order(field_layouts).alignment_bytes(target_info) } UnionLayout::NullableWrapped { other_tags, .. } => { Self::tags_alignment_bytes(other_tags, target_info) } UnionLayout::NullableUnwrapped { other_fields, .. } => { - Layout::Struct(other_fields).alignment_bytes(target_info) + Layout::struct_no_name_order(other_fields).alignment_bytes(target_info) } }; @@ -495,12 +508,12 @@ impl<'a> UnionLayout<'a> { let mut alignment_bytes = 0; for field_layouts in variant_field_layouts { - let mut data = Layout::Struct(field_layouts); + let mut data = Layout::struct_no_name_order(field_layouts); let fields_and_id; if let Some(id_layout) = id_data_layout { fields_and_id = [data, id_layout]; - data = Layout::Struct(&fields_and_id); + data = Layout::struct_no_name_order(&fields_and_id); } let (variant_size, variant_alignment) = data.stack_size_and_alignment(target_info); @@ -590,7 +603,10 @@ impl<'a> LambdaSet<'a> { } pub fn is_represented(&self) -> Option> { - if let Layout::Struct(&[]) = self.representation { + if let Layout::Struct { + field_layouts: &[], .. + } = self.representation + { None } else { Some(*self.representation) @@ -648,7 +664,7 @@ impl<'a> LambdaSet<'a> { } => todo!("recursive closures"), } } - Layout::Struct(_) => { + 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 @@ -673,7 +689,9 @@ impl<'a> LambdaSet<'a> { argument_layouts } else { match self.representation { - Layout::Struct(&[]) => { + Layout::Struct { + field_layouts: &[], .. + } => { // this function does not have anything in its closure, and the lambda set is a // singleton, so we pass no extra argument argument_layouts @@ -769,7 +787,7 @@ impl<'a> LambdaSet<'a> { } Newtype { arguments: layouts, .. - } => Layout::Struct(layouts.into_bump_slice()), + } => Layout::struct_no_name_order(layouts.into_bump_slice()), Wrapped(variant) => { use WrappedVariant::*; @@ -865,7 +883,10 @@ pub const fn round_up_to_alignment(width: u32, alignment: u32) -> u32 { impl<'a> Layout<'a> { pub const VOID: Self = Layout::Union(UnionLayout::NonRecursive(&[])); - pub const UNIT: Self = Layout::Struct(&[]); + pub const UNIT: Self = Layout::Struct { + field_layouts: &[], + field_order_hash: IRRELEVANT_FIELD_HASH, + }; fn new_help<'b>( env: &mut Env<'a, 'b>, @@ -926,7 +947,7 @@ impl<'a> Layout<'a> { match self { Builtin(builtin) => builtin.safe_to_memcpy(), - Struct(fields) => fields + Struct { field_layouts, .. } => field_layouts .iter() .all(|field_layout| field_layout.safe_to_memcpy()), Union(variant) => { @@ -990,10 +1011,10 @@ impl<'a> Layout<'a> { match self { Builtin(builtin) => builtin.stack_size(target_info), - Struct(fields) => { + Struct { field_layouts, .. } => { let mut sum = 0; - for field_layout in *fields { + for field_layout in *field_layouts { sum += field_layout.stack_size(target_info); } @@ -1020,7 +1041,7 @@ impl<'a> Layout<'a> { pub fn alignment_bytes(&self, target_info: TargetInfo) -> u32 { match self { - Layout::Struct(fields) => fields + Layout::Struct { field_layouts, .. } => field_layouts .iter() .map(|x| x.alignment_bytes(target_info)) .max() @@ -1069,7 +1090,7 @@ impl<'a> Layout<'a> { pub fn allocation_alignment_bytes(&self, target_info: TargetInfo) -> u32 { match self { Layout::Builtin(builtin) => builtin.allocation_alignment_bytes(target_info), - Layout::Struct(_) => unreachable!("not heap-allocated"), + Layout::Struct { .. } => unreachable!("not heap-allocated"), Layout::Union(union_layout) => union_layout.allocation_alignment_bytes(target_info), Layout::LambdaSet(lambda_set) => lambda_set .runtime_representation() @@ -1103,7 +1124,7 @@ impl<'a> Layout<'a> { match self { Builtin(builtin) => builtin.is_refcounted(), - Struct(fields) => fields.iter().any(|f| f.contains_refcounted()), + Struct { field_layouts, .. } => field_layouts.iter().any(|f| f.contains_refcounted()), Union(variant) => { use UnionLayout::*; @@ -1134,8 +1155,8 @@ impl<'a> Layout<'a> { match self { Builtin(builtin) => builtin.to_doc(alloc, parens), - Struct(fields) => { - let fields_doc = fields.iter().map(|x| x.to_doc(alloc, parens)); + Struct { field_layouts, .. } => { + let fields_doc = field_layouts.iter().map(|x| x.to_doc(alloc, parens)); alloc .text("{") @@ -1147,6 +1168,14 @@ impl<'a> Layout<'a> { RecursivePointer => alloc.text("*self"), } } + + /// Used to build a `Layout::Struct` where the field name order is irrelevant. + pub fn struct_no_name_order(field_layouts: &'a [Layout]) -> Self { + Self::Struct { + field_layouts, + field_order_hash: IRRELEVANT_FIELD_HASH, + } + } } /// Avoid recomputing Layout from Variable multiple times. @@ -1590,6 +1619,14 @@ fn layout_from_flat_type<'a>( size2.cmp(&size1).then(label1.cmp(label2)) }); + let field_order_hash = { + let mut field_order_hasher = DefaultHasher::new(); + pairs + .iter() + .for_each(|(label, _)| label.hash(&mut field_order_hasher)); + field_order_hasher.finish() + }; + let mut layouts = Vec::from_iter_in(pairs.into_iter().map(|t| t.1), arena); if layouts.len() == 1 { @@ -1597,7 +1634,10 @@ fn layout_from_flat_type<'a>( // unwrap it. Ok(layouts.pop().unwrap()) } else { - Ok(Layout::Struct(layouts.into_bump_slice())) + Ok(Layout::Struct { + field_order_hash, + field_layouts: layouts.into_bump_slice(), + }) } } TagUnion(tags, ext_var) => { @@ -2224,6 +2264,9 @@ pub fn union_sorted_tags_help<'a>( } else { UnionVariant::Newtype { tag_name, + // NB: It may be the case that the order of the layouts chosen here must be + // hashed, in case they differ for two specializations of the same type. If + // there is a problem with newtypes not specializing correctly, this might be why! arguments: layouts, } } @@ -2430,7 +2473,8 @@ fn layout_from_tag_union<'a>( let answer1 = if field_layouts.len() == 1 { field_layouts[0] } else { - Layout::Struct(field_layouts.into_bump_slice()) + // NB: does the field name order here matter, when it comes from the tag payload order? + Layout::struct_no_name_order(field_layouts.into_bump_slice()) }; answer1 diff --git a/compiler/test_mono/generated/issue_2535_polymorphic_fields_referenced_in_list.txt b/compiler/test_mono/generated/issue_2535_polymorphic_fields_referenced_in_list.txt new file mode 100644 index 0000000000..a30515bee8 --- /dev/null +++ b/compiler/test_mono/generated/issue_2535_polymorphic_fields_referenced_in_list.txt @@ -0,0 +1,19 @@ +procedure Test.1 (): + let Test.11 : Builtin(Int(I64)) = 2i64; + let Test.12 : Builtin(Int(U8)) = 1i64; + let Test.10 : Struct { field_order_hash: 4700212371939112988, field_layouts: [Builtin(Int(I64)), Builtin(Int(U8))] } = Struct {Test.11, Test.12}; + ret Test.10; + +procedure Test.1 (): + let Test.7 : Builtin(Int(I64)) = 1i64; + let Test.8 : Builtin(Int(U8)) = 2i64; + let Test.6 : Struct { field_order_hash: 17343052842552570643, field_layouts: [Builtin(Int(I64)), Builtin(Int(U8))] } = Struct {Test.7, Test.8}; + ret Test.6; + +procedure Test.0 (): + let Test.9 : Struct { field_order_hash: 4700212371939112988, field_layouts: [Builtin(Int(I64)), Builtin(Int(U8))] } = CallByName Test.1; + let Test.3 : Builtin(Int(U8)) = StructAtIndex 1 Test.9; + let Test.5 : Struct { field_order_hash: 17343052842552570643, field_layouts: [Builtin(Int(I64)), Builtin(Int(U8))] } = CallByName Test.1; + let Test.4 : Builtin(Int(U8)) = StructAtIndex 1 Test.5; + let Test.2 : Builtin(List(Builtin(Int(U8)))) = Array [Test.3, Test.4]; + ret Test.2; diff --git a/compiler/test_mono/src/tests.rs b/compiler/test_mono/src/tests.rs index 6903714eaa..28bb791955 100644 --- a/compiler/test_mono/src/tests.rs +++ b/compiler/test_mono/src/tests.rs @@ -1249,6 +1249,24 @@ fn aliased_polymorphic_closure() { ) } +#[mono_test] +fn issue_2535_polymorphic_fields_referenced_in_list() { + indoc!( + r#" + app "test" provides [ nums ] to "./platform" + + alpha = { a: 1, b: 2 } + + nums : List U8 + nums = + [ + alpha.a, + alpha.b, + ] + "# + ) +} + // #[ignore] // #[mono_test] // fn static_str_closure() { diff --git a/repl_eval/src/eval.rs b/repl_eval/src/eval.rs index f97ce94494..483d7b6be6 100644 --- a/repl_eval/src/eval.rs +++ b/repl_eval/src/eval.rs @@ -331,7 +331,7 @@ fn jit_to_ast_help<'a, A: ReplApp<'a>>( Layout::Builtin(other) => { todo!("add support for rendering builtin {:?} to the REPL", other) } - Layout::Struct(field_layouts) => { + Layout::Struct { field_layouts, .. } => { let struct_addr_to_ast = |mem: &'a A::Memory, addr: usize| match content { Content::Structure(FlatType::Record(fields, _)) => { Ok(struct_to_ast(env, mem, addr, field_layouts, *fields)) @@ -382,7 +382,7 @@ fn jit_to_ast_help<'a, A: ReplApp<'a>>( }; let fields = [Layout::u64(), *layout]; - let layout = Layout::Struct(&fields); + let layout = Layout::struct_no_name_order(&fields); let result_stack_size = layout.stack_size(env.target_info); @@ -516,7 +516,7 @@ fn addr_to_ast<'a, M: ReplAppMemory>( str_to_ast(env.arena, arena_str) } - (_, Layout::Struct(field_layouts)) => match content { + (_, Layout::Struct{field_layouts, ..}) => match content { Content::Structure(FlatType::Record(fields, _)) => { struct_to_ast(env, mem, addr, field_layouts, *fields) } @@ -796,7 +796,7 @@ fn single_tag_union_to_ast<'a, M: ReplAppMemory>( sequence_of_expr(env, mem, addr, it, WhenRecursive::Unreachable).into_bump_slice() } else if field_layouts.is_empty() && !payload_vars.is_empty() { // happens for e.g. `Foo Bar` where unit structures are nested and the inner one is dropped - let it = payload_vars.iter().copied().zip([&Layout::Struct(&[])]); + let it = payload_vars.iter().copied().zip([&Layout::UNIT]); sequence_of_expr(env, mem, addr, it, WhenRecursive::Unreachable).into_bump_slice() } else { unreachable!() @@ -864,7 +864,7 @@ fn struct_to_ast<'a, M: ReplAppMemory>( env, mem, addr, - &Layout::Struct(field_layouts), + &Layout::struct_no_name_order(field_layouts), WhenRecursive::Unreachable, inner_content, ),