fix bug with record field ordering in repl eval

crates/repl_eval/src/eval.rs

@@ -1,4 +1,4 @@
-use bumpalo::collections::Vec;
+use bumpalo::collections::{CollectIn, Vec};
 use bumpalo::Bump;
 use roc_types::types::AliasKind;
 use std::cmp::{max_by_key, min_by_key};
@@ -10,8 +10,8 @@ use roc_module::ident::TagName;
 use roc_module::symbol::{Interns, ModuleId, Symbol};
 use roc_mono::ir::ProcLayout;
 use roc_mono::layout::{
-    self, union_sorted_tags_pub, Builtin, InLayout, Layout, LayoutCache, LayoutInterner,
-    TLLayoutInterner, UnionLayout, UnionVariant, WrappedVariant,
+    self, cmp_fields, union_sorted_tags_pub, Builtin, InLayout, Layout, LayoutCache,
+    LayoutInterner, TLLayoutInterner, UnionLayout, UnionVariant, WrappedVariant,
 };
 use roc_parse::ast::{AssignedField, Collection, Expr, Pattern, StrLiteral};
 use roc_region::all::{Loc, Region};
@@ -365,7 +365,6 @@ fn jit_to_ast_help<'a, A: ReplApp<'a>>(
                 (Alias(Symbol::NUM_UNSIGNED8 | Symbol::NUM_U8, ..), U8) => num_helper!(u8),
                 (_, U8) => {
                     // This is not a number, it's a tag union or something else
-                    dbg!(&alias_content);
                     app.call_function(main_fn_name, |_mem: &A::Memory, num: u8| {
                         byte_to_ast(env, num, env.subs.get_content_without_compacting(raw_var))
                     })
@@ -559,9 +558,11 @@ fn addr_to_ast<'a, M: ReplAppMemory>(
         (_, Layout::Builtin(Builtin::Bool)) => {
             // TODO: bits are not as expected here.
             // num is always false at the moment.
-            let num: bool = mem.deref_bool(addr);
+            let num: u8 = mem.deref_u8(addr);
 
-            bool_to_ast(env, num, raw_content)
+            debug_assert!(num == 0 || num == 1);
+
+            bool_to_ast(env, num != 0, raw_content)
         }
         (_, Layout::Builtin(Builtin::Int(int_width))) => {
             use IntWidth::*;
@@ -1053,16 +1054,34 @@ fn struct_to_ast<'a, 'env, M: ReplAppMemory>(
         // We'll advance this as we iterate through the fields
         let mut field_addr = addr;
 
-        // We recalculate the layouts here because we will have compiled the record so that its fields
-        // are sorted by descending alignment, and then alphabetic, but the type of the record is
-        // always only sorted alphabetically. We want to arrange the rendered record in the order of
-        // the type.
-        for (label, field) in record_fields.sorted_iterator(subs, Variable::EMPTY_RECORD) {
+        // the type checker stores record fiels in alphabetical order
+        let alphabetical_fields: Vec<_> = record_fields
+            .sorted_iterator(subs, Variable::EMPTY_RECORD)
+            .map(|(l, field)| {
+                let layout = env
+                    .layout_cache
+                    .from_var(arena, field.into_inner(), env.subs)
+                    .unwrap();
+
+                (l, field, layout)
+            })
+            .collect_in(arena);
+
+        // but the memory representation sorts first by size (and uses field name as a tie breaker)
+        let mut in_memory_fields = alphabetical_fields;
+        in_memory_fields.sort_by(|(label1, _, layout1), (label2, _, layout2)| {
+            cmp_fields(
+                &env.layout_cache.interner,
+                label1,
+                *layout1,
+                label2,
+                *layout2,
+                env.target_info,
+            )
+        });
+
+        for (label, field, field_layout) in in_memory_fields {
             let field_var = field.into_inner();
-            let field_layout = env
-                .layout_cache
-                .from_var(arena, field.into_inner(), env.subs)
-                .unwrap();
 
             let loc_expr = &*arena.alloc(Loc {
                 value: addr_to_ast(
@@ -1091,6 +1110,15 @@ fn struct_to_ast<'a, 'env, M: ReplAppMemory>(
             field_addr += env.layout_cache.interner.stack_size(field_layout) as usize;
         }
 
+        // to the user we want to present the fields in alphabetical order again, so re-sort
+        fn sort_key<'a, T>(loc_field: &'a Loc<AssignedField<'a, T>>) -> &'a str {
+            match &loc_field.value {
+                AssignedField::RequiredValue(field_name, _, _) => field_name.value,
+                _ => unreachable!("was not added to output"),
+            }
+        }
+
+        output.sort_by(|a, b| sort_key(a).cmp(sort_key(b)));
         let output = output.into_bump_slice();
 
         Expr::Record(Collection::with_items(output))

crates/repl_expect/src/app.rs

@@ -17,7 +17,15 @@ macro_rules! deref_number {
 }
 
 impl ReplAppMemory for ExpectMemory {
-    deref_number!(deref_bool, bool);
+    fn deref_bool(&self, addr: usize) -> bool {
+        let ptr = unsafe { self.start.add(addr) } as *const u8;
+        let value = unsafe { std::ptr::read_unaligned(ptr) };
+
+        // bool values should only ever be 0 or 1
+        debug_assert!(value == 0 || value == 1);
+
+        value != 0
+    }
 
     deref_number!(deref_u8, u8);
     deref_number!(deref_u16, u16);

crates/repl_expect/src/lib.rs

@@ -977,7 +977,7 @@ mod test {
     }
 
     #[test]
-    fn foobar() {
+    fn adjacent_lists() {
         run_expect_test(
             indoc!(
                 r#"
@@ -1039,4 +1039,41 @@ mod test {
             ),
         );
     }
+
+    #[test]
+    fn record_field_ordering() {
+        run_expect_test(
+            indoc!(
+                r#"
+                interface Test exposes [] imports []
+
+                Request : {
+                    fieldA : [Get, Post],
+                    fieldB : Str,
+                }
+
+                expect
+
+                    actual : Request
+                    actual = {
+                        fieldA: Get,
+                        fieldB: "/things?id=2",
+                    }
+
+                    expected : Request
+                    expected = {
+                        fieldA: Get,
+                        fieldB: "/things?id=1",
+                    }
+                    actual == expected
+                "#
+            ),
+            indoc!(
+                r#"
+                This expectation failed:
+
+                "#
+            ),
+        );
+    }
 }

examples/parser/Parser/Http.roc

@@ -134,6 +134,8 @@ expect
         """
     actual =
         parseStr request requestText
+
+    expected : Result Request [ParsingFailure Str, ParsingIncomplete Str]
     expected = Ok {
         method: Get,
         uri: "/things?id=1",