Remove field_order_hash from struct layouts

crates/compiler/gen_wasm/src/low_level.rs

@@ -6,10 +6,7 @@ use roc_module::low_level::LowLevel;
 use roc_module::symbol::Symbol;
 use roc_mono::code_gen_help::HelperOp;
 use roc_mono::ir::{HigherOrderLowLevel, PassedFunction, ProcLayout};
-use roc_mono::layout::{
+use roc_mono::layout::{Builtin, InLayout, Layout, LayoutInterner, LayoutRepr, UnionLayout};
-    Builtin, FieldOrderHash, InLayout, Layout, LayoutInterner, LayoutRepr, SemanticRepr,
-    UnionLayout,
-};
 use roc_mono::low_level::HigherOrder;
 
 use crate::backend::{ProcLookupData, ProcSource, WasmBackend};
@@ -700,7 +697,6 @@ impl<'a> LowLevelCall<'a> {
                     backend
                         .layout_interner
                         .insert_no_semantic(LayoutRepr::Struct {
-                            field_order_hash: FieldOrderHash::from_ordered_fields(&[]),
                             field_layouts: backend.env.arena.alloc([elem_layout]),
                         });
                 let dec_fn = backend.get_refcount_fn_index(in_memory_layout, HelperOp::Dec);
@@ -749,7 +745,6 @@ impl<'a> LowLevelCall<'a> {
                     backend
                         .layout_interner
                         .insert_no_semantic(LayoutRepr::Struct {
-                            field_order_hash: FieldOrderHash::from_ordered_fields(&[]),
                             field_layouts: backend.env.arena.alloc([elem_layout]),
                         });
                 let dec_fn = backend.get_refcount_fn_index(in_memory_layout, HelperOp::Dec);
@@ -2677,7 +2672,6 @@ fn list_map_n<'a>(
             let el_ptr = backend
                 .layout_interner
                 .insert_no_semantic(LayoutRepr::Struct {
-                    field_order_hash: FieldOrderHash::from_ordered_fields(&[]),
                     field_layouts: backend.env.arena.alloc([*el]),
                 });
             let idx = backend.get_refcount_fn_index(el_ptr, HelperOp::Dec);
@@ -2719,7 +2713,6 @@ fn ensure_symbol_is_in_memory<'a>(
             let in_memory_layout = backend
                 .layout_interner
                 .insert_no_semantic(LayoutRepr::Struct {
-                    field_order_hash: FieldOrderHash::from_ordered_fields(&[]), // don't care
                     field_layouts: arena.alloc([layout]),
                 });
             (frame_ptr, offset, in_memory_layout)

crates/compiler/mono/src/code_gen_help/mod.rs

@@ -541,17 +541,13 @@ impl<'a> CodeGenHelp<'a> {
 
             LayoutRepr::Builtin(_) => return layout,
 
-            LayoutRepr::Struct {
+            LayoutRepr::Struct { field_layouts } => {
-                field_layouts,
-                field_order_hash,
-            } => {
                 let mut new_field_layouts = Vec::with_capacity_in(field_layouts.len(), self.arena);
                 for f in field_layouts.iter() {
                     new_field_layouts.push(self.replace_rec_ptr(ctx, layout_interner, *f));
                 }
                 LayoutRepr::Struct {
                     field_layouts: new_field_layouts.into_bump_slice(),
-                    field_order_hash,
                 }
             }
 

crates/compiler/mono/src/ir.rs

@@ -9059,10 +9059,7 @@ fn match_on_lambda_set<'a>(
                 env.arena.alloc(result),
             )
         }
-        ClosureCallOptions::Struct {
+        ClosureCallOptions::Struct { field_layouts } => {
-            field_layouts,
-            field_order_hash: _,
-        } => {
             let function_symbol = match lambda_set.iter_set().next() {
                 Some(function_symbol) => function_symbol,
                 None => {

crates/compiler/mono/src/layout.rs

@@ -21,9 +21,9 @@ use roc_types::types::{
     TupleElemsError,
 };
 use std::cmp::Ordering;
-use std::collections::hash_map::{DefaultHasher, Entry};
+use std::collections::hash_map::Entry;
 use std::collections::HashMap;
-use std::hash::{Hash, Hasher};
+use std::hash::Hash;
 use ven_pretty::{DocAllocator, DocBuilder};
 
 mod intern;
@@ -37,17 +37,17 @@ pub use semantic::SemanticRepr;
 // please change it to the lower number.
 // if it went up, maybe check that the change is really required
 roc_error_macros::assert_sizeof_aarch64!(Builtin, 2 * 8);
-roc_error_macros::assert_sizeof_aarch64!(Layout, 8 * 8);
+roc_error_macros::assert_sizeof_aarch64!(Layout, 9 * 8);
 roc_error_macros::assert_sizeof_aarch64!(UnionLayout, 3 * 8);
 roc_error_macros::assert_sizeof_aarch64!(LambdaSet, 5 * 8);
 
 roc_error_macros::assert_sizeof_wasm!(Builtin, 2 * 4);
-roc_error_macros::assert_sizeof_wasm!(Layout, 8 * 4);
+roc_error_macros::assert_sizeof_wasm!(Layout, 9 * 4);
 roc_error_macros::assert_sizeof_wasm!(UnionLayout, 3 * 4);
 roc_error_macros::assert_sizeof_wasm!(LambdaSet, 5 * 4);
 
 roc_error_macros::assert_sizeof_default!(Builtin, 2 * 8);
-roc_error_macros::assert_sizeof_default!(Layout, 8 * 8);
+roc_error_macros::assert_sizeof_default!(Layout, 9 * 8);
 roc_error_macros::assert_sizeof_default!(UnionLayout, 3 * 8);
 roc_error_macros::assert_sizeof_default!(LambdaSet, 5 * 8);
 
@@ -654,38 +654,6 @@ impl<'a> RawFunctionLayout<'a> {
     }
 }
 
-#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)]
-pub struct FieldOrderHash(u64);
-
-impl FieldOrderHash {
-    // NB: This should really be a proper "zero" hash via `DefaultHasher::new().finish()`, but Rust
-    // stdlib hashers are not (yet) compile-time-computable.
-    const ZERO_FIELD_HASH: Self = Self(0);
-    const IRRELEVANT_NON_ZERO_FIELD_HASH: Self = Self(1);
-
-    pub fn from_ordered_fields(fields: &[&str]) -> Self {
-        if fields.is_empty() {
-            // HACK: we must make sure this is always equivalent to a `ZERO_FIELD_HASH`.
-            return Self::ZERO_FIELD_HASH;
-        }
-
-        let mut hasher = DefaultHasher::new();
-        fields.iter().for_each(|field| field.hash(&mut hasher));
-        Self(hasher.finish())
-    }
-
-    pub fn from_ordered_tuple_elems(elems: &[usize]) -> Self {
-        if elems.is_empty() {
-            // HACK: we must make sure this is always equivalent to a `ZERO_FIELD_HASH`.
-            return Self::ZERO_FIELD_HASH;
-        }
-
-        let mut hasher = DefaultHasher::new();
-        elems.iter().for_each(|elem| elem.hash(&mut hasher));
-        Self(hasher.finish())
-    }
-}
-
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)]
 pub struct Layout<'a> {
     pub repr: LayoutRepr<'a>,
@@ -696,19 +664,7 @@ pub struct Layout<'a> {
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)]
 pub enum LayoutRepr<'a> {
     Builtin(Builtin<'a>),
-    Struct {
+    Struct { field_layouts: &'a [InLayout<'a>] },
-        /// 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/roc-lang/roc/issues/2535.
-        field_order_hash: FieldOrderHash,
-        field_layouts: &'a [InLayout<'a>],
-    },
     Boxed(InLayout<'a>),
     Union(UnionLayout<'a>),
     LambdaSet(LambdaSet<'a>),
@@ -1433,10 +1389,7 @@ pub enum ClosureCallOptions<'a> {
     /// One of a few capturing functions can be called to
     Union(UnionLayout<'a>),
     /// The closure is one function, whose captures are represented as a struct.
-    Struct {
+    Struct { field_layouts: &'a [InLayout<'a>] },
-        field_layouts: &'a [InLayout<'a>],
-        field_order_hash: FieldOrderHash,
-    },
     /// The closure is one function that captures a single identifier, whose value is unwrapped.
     UnwrappedCapture(InLayout<'a>),
     /// The closure dispatches to multiple possible functions, none of which capture.
@@ -1717,15 +1670,9 @@ impl<'a> LambdaSet<'a> {
                 }
                 ClosureCallOptions::Union(union_layout)
             }
-            LayoutRepr::Struct {
+            LayoutRepr::Struct { field_layouts } => {
-                field_layouts,
-                field_order_hash,
-            } => {
                 debug_assert_eq!(self.set.len(), 1);
-                ClosureCallOptions::Struct {
+                ClosureCallOptions::Struct { field_layouts }
-                    field_layouts,
-                    field_order_hash,
-                }
             }
             layout => {
                 debug_assert!(self.has_enum_dispatch_repr());
@@ -2518,10 +2465,7 @@ impl<'a> Layout<'a> {
             Self::UNIT_NAKED
         } else {
             Self {
-                repr: LayoutRepr::Struct {
+                repr: LayoutRepr::Struct { field_layouts },
-                    field_layouts,
-                    field_order_hash: FieldOrderHash::IRRELEVANT_NON_ZERO_FIELD_HASH,
-                },
                 semantic: SemanticRepr::None,
             }
         }
@@ -3268,8 +3212,6 @@ fn layout_from_flat_type<'a>(
                     .map(|(label, _)| &*arena.alloc_str(label.as_str())),
                 arena,
             );
-            let field_order_hash =
-                FieldOrderHash::from_ordered_fields(ordered_field_names.as_slice());
 
             let result = if sortables.len() == 1 {
                 // If the record has only one field that isn't zero-sized,
@@ -3279,7 +3221,6 @@ fn layout_from_flat_type<'a>(
                 let layouts = Vec::from_iter_in(sortables.into_iter().map(|t| t.1), arena);
                 let struct_layout = Layout {
                     repr: LayoutRepr::Struct {
-                        field_order_hash,
                         field_layouts: layouts.into_bump_slice(),
                     },
                     semantic: SemanticRepr::record(ordered_field_names.into_bump_slice()),
@@ -3316,23 +3257,16 @@ fn layout_from_flat_type<'a>(
                 )
             });
 
-            let ordered_field_names =
-                Vec::from_iter_in(sortables.iter().map(|(index, _)| *index), arena);
-            let field_order_hash =
-                FieldOrderHash::from_ordered_tuple_elems(ordered_field_names.as_slice());
-
             let result = if sortables.len() == 1 {
                 // If the tuple has only one field that isn't zero-sized,
                 // unwrap it.
                 Ok(sortables.pop().unwrap().1)
             } else {
-                let layouts = Vec::from_iter_in(sortables.into_iter().map(|t| t.1), arena);
+                let field_layouts =
+                    Vec::from_iter_in(sortables.into_iter().map(|t| t.1), arena).into_bump_slice();
                 let struct_layout = Layout {
-                    repr: LayoutRepr::Struct {
+                    repr: LayoutRepr::Struct { field_layouts },
-                        field_order_hash,
+                    semantic: SemanticRepr::tuple(field_layouts.len()),
-                        field_layouts: layouts.into_bump_slice(),
-                    },
-                    semantic: SemanticRepr::tuple(layouts.len()),
                 };
 
                 Ok(env.cache.put_in(struct_layout))

crates/compiler/mono/src/layout/intern.rs

@@ -14,7 +14,7 @@ use roc_target::TargetInfo;
 
 use crate::layout::LayoutRepr;
 
-use super::{Builtin, FieldOrderHash, LambdaSet, Layout, SeenRecPtrs, SemanticRepr, UnionLayout};
+use super::{Builtin, LambdaSet, Layout, SeenRecPtrs, SemanticRepr, UnionLayout};
 
 macro_rules! cache_interned_layouts {
     ($($i:literal, $name:ident, $vis:vis, $layout:expr)*; $total_constants:literal) => {
@@ -123,10 +123,7 @@ impl<'a> Layout<'a> {
         semantic: SemanticRepr::None,
     };
     pub(super) const UNIT_NAKED: Self = Layout {
-        repr: LayoutRepr::Struct {
+        repr: LayoutRepr::Struct { field_layouts: &[] },
-            field_layouts: &[],
-            field_order_hash: FieldOrderHash::ZERO_FIELD_HASH,
-        },
         semantic: SemanticRepr::EMPTY_RECORD,
     };
 
@@ -1052,11 +1049,7 @@ mod reify {
             LayoutRepr::Builtin(builtin) => {
                 LayoutRepr::Builtin(reify_builtin(arena, interner, slot, builtin))
             }
-            LayoutRepr::Struct {
+            LayoutRepr::Struct { field_layouts } => LayoutRepr::Struct {
-                field_order_hash,
-                field_layouts,
-            } => LayoutRepr::Struct {
-                field_order_hash,
                 field_layouts: reify_layout_slice(arena, interner, slot, field_layouts),
             },
             LayoutRepr::Boxed(lay) => LayoutRepr::Boxed(reify_layout(arena, interner, slot, lay)),
@@ -1264,19 +1257,7 @@ mod equiv {
                         }
                     }
                 }
-                (
+                (Struct { field_layouts: fl1 }, Struct { field_layouts: fl2 }) => {
-                    Struct {
-                        field_order_hash: foh1,
-                        field_layouts: fl1,
-                    },
-                    Struct {
-                        field_order_hash: foh2,
-                        field_layouts: fl2,
-                    },
-                ) => {
-                    if foh1 != foh2 {
-                        return false;
-                    }
                     equiv_fields!(fl1, fl2)
                 }
                 (Boxed(b1), Boxed(b2)) => stack.push((b1, b2)),
@@ -1371,12 +1352,8 @@ pub mod dbg {
                     .debug_tuple("Builtin")
                     .field(&DbgBuiltin(self.0, b))
                     .finish(),
-                LayoutRepr::Struct {
+                LayoutRepr::Struct { field_layouts } => f
-                    field_order_hash,
-                    field_layouts,
-                } => f
                     .debug_struct("Struct")
-                    .field("hash", &field_order_hash)
                     .field("fields", &DbgFields(self.0, field_layouts))
                     .finish(),
                 LayoutRepr::Boxed(b) => f.debug_tuple("Boxed").field(&Dbg(self.0, b)).finish(),
@@ -1633,7 +1610,6 @@ mod insert_recursive_layout {
                     (
                         LayoutRepr::Builtin(Builtin::List(l1)),
                         LayoutRepr::Struct {
-                            field_order_hash: _,
                             field_layouts: &[l2],
                         },
                     ) => match (interner.get(l1).repr, interner.get(l2).repr) {