Merge pull request #738 from rtfeldman/fix-alignment

Fix alignment

compiler/gen/src/llvm/build.rs

@@ -735,7 +735,12 @@ pub fn build_exp_expr<'a, 'ctx, 'env>(
                 // Insert field exprs into struct_val
                 for (index, field_val) in field_vals.into_iter().enumerate() {
                     struct_val = builder
-                        .build_insert_value(struct_val, field_val, index as u32, "insert_field")
+                        .build_insert_value(
+                            struct_val,
+                            field_val,
+                            index as u32,
+                            "insert_record_field",
+                        )
                         .unwrap();
                 }
 
@@ -785,7 +790,12 @@ pub fn build_exp_expr<'a, 'ctx, 'env>(
                 // Insert field exprs into struct_val
                 for (index, field_val) in field_vals.into_iter().enumerate() {
                     struct_val = builder
-                        .build_insert_value(struct_val, field_val, index as u32, "insert_field")
+                        .build_insert_value(
+                            struct_val,
+                            field_val,
+                            index as u32,
+                            "insert_single_tag_field",
+                        )
                         .unwrap();
                 }
 
@@ -848,7 +858,12 @@ pub fn build_exp_expr<'a, 'ctx, 'env>(
             // Insert field exprs into struct_val
             for (index, field_val) in field_vals.into_iter().enumerate() {
                 struct_val = builder
-                    .build_insert_value(struct_val, field_val, index as u32, "insert_field")
+                    .build_insert_value(
+                        struct_val,
+                        field_val,
+                        index as u32,
+                        "insert_multi_tag_field",
+                    )
                     .unwrap();
             }
 

compiler/gen/tests/gen_records.rs

@@ -844,4 +844,49 @@ mod gen_records {
             (bool, bool)
         );
     }
+
+    #[test]
+    fn alignment_in_record() {
+        assert_evals_to!(
+            indoc!("{ c: 32, b: if True then Red else if True then Green else Blue, a: 1 == 1 }"),
+            (32i64, true, 2u8),
+            (i64, bool, u8)
+        );
+    }
+
+    #[test]
+    fn blue_and_present() {
+        assert_evals_to!(
+            indoc!(
+                r#"
+                f = \r ->
+                    when r is
+                        { x: Blue, y ? 3 } -> y
+                        { x: Red, y ? 5 } -> y
+
+                f { x: Blue, y: 7 }
+                "#
+            ),
+            7,
+            i64
+        );
+    }
+
+    #[test]
+    fn blue_and_absent() {
+        assert_evals_to!(
+            indoc!(
+                r#"
+                f = \r ->
+                    when r is
+                        { x: Blue, y ? 3 } -> y
+                        { x: Red, y ? 5 } -> y
+
+                f { x: Blue }
+                "#
+            ),
+            3,
+            i64
+        );
+    }
 }

compiler/gen/tests/gen_tags.rs

@@ -481,7 +481,7 @@ mod gen_tags {
         assert_evals_to!(
             indoc!(
                 r#"
-                wrapper = \{} -> 
+                wrapper = \{} ->
                     when 2 is
                         2 if False -> 0
                         _ -> 42
@@ -499,7 +499,7 @@ mod gen_tags {
         assert_evals_to!(
             indoc!(
                 r#"
-                wrapper = \{} -> 
+                wrapper = \{} ->
                     when 2 is
                         2 if True -> 42
                         _ -> 0
@@ -517,7 +517,7 @@ mod gen_tags {
         assert_evals_to!(
             indoc!(
                 r#"
-                wrapper = \{} -> 
+                wrapper = \{} ->
                     when 2 is
                         _ if False -> 0
                         _ -> 42
@@ -637,7 +637,7 @@ mod gen_tags {
                 x : Maybe (Maybe Int)
                 x = Just (Just 41)
 
-                main = 
+                main =
                     x
                 "#
             ),
@@ -701,11 +701,11 @@ mod gen_tags {
         assert_evals_to!(
             indoc!(
                 r#"
-            wrapper = \{} -> 
+            wrapper = \{} ->
                 x : [ Red, White, Blue ]
                 x = Blue
 
-                y = 
+                y =
                     when x is
                         Red -> 1
                         White -> 2
@@ -726,8 +726,8 @@ mod gen_tags {
         assert_evals_to!(
             indoc!(
                 r#"
-                wrapper = \{} -> 
+                wrapper = \{} ->
-                    y = 
+                    y =
                         when 1 + 2 is
                             3 -> 3
                             1 -> 1
@@ -745,7 +745,7 @@ mod gen_tags {
         assert_evals_to!(
             indoc!(
                 r#"
-            y = 
+            y =
                 if 1 + 2 > 0 then
                     3
                 else
@@ -758,4 +758,114 @@ mod gen_tags {
             i64
         );
     }
+
+    #[test]
+    fn alignment_in_single_tag_construction() {
+        assert_evals_to!(indoc!("Three (1 == 1) 32"), (32i64, true), (i64, bool));
+
+        assert_evals_to!(
+            indoc!("Three (1 == 1) (if True then Red else if True then Green else Blue) 32"),
+            (32i64, true, 2u8),
+            (i64, bool, u8)
+        );
+    }
+
+    #[test]
+    fn alignment_in_single_tag_pattern_match() {
+        assert_evals_to!(
+            indoc!(
+                r"#
+                x = Three (1 == 1) 32
+
+                when x is
+                    Three bool int ->
+                        { bool, int }
+                #"
+            ),
+            (32i64, true),
+            (i64, bool)
+        );
+
+        assert_evals_to!(
+            indoc!(
+                r"#
+                x = Three (1 == 1) (if True then Red else if True then Green else Blue) 32
+
+                when x is
+                    Three bool color int ->
+                        { bool, color, int }
+                #"
+            ),
+            (32i64, true, 2u8),
+            (i64, bool, u8)
+        );
+    }
+
+    #[test]
+    fn alignment_in_multi_tag_construction() {
+        assert_evals_to!(
+            indoc!(
+                r"#
+                x : [ Three Bool Int, Empty ]
+                x = Three (1 == 1) 32
+
+                x
+
+                #"
+            ),
+            (1, 32i64, true),
+            (i64, i64, bool)
+        );
+
+        assert_evals_to!(
+            indoc!(
+                r"#
+                x : [ Three Bool [ Red, Green, Blue ] Int, Empty ]
+                x = Three (1 == 1) (if True then Red else if True then Green else Blue) 32
+
+                x
+                #"
+            ),
+            (1, 32i64, true, 2u8),
+            (i64, i64, bool, u8)
+        );
+    }
+
+    #[test]
+    fn alignment_in_multi_tag_pattern_match() {
+        assert_evals_to!(
+            indoc!(
+                r"#
+                x : [ Three Bool Int, Empty ]
+                x = Three (1 == 1) 32
+
+                when x is
+                    Three bool int ->
+                        { bool, int }
+
+                    Empty ->
+                        { bool: False, int: 0 }
+                #"
+            ),
+            (32i64, true),
+            (i64, bool)
+        );
+
+        assert_evals_to!(
+            indoc!(
+                r"#
+                x : [ Three Bool [ Red, Green, Blue ] Int, Empty ]
+                x = Three (1 == 1) (if True then Red else if True then Green else Blue) 32
+
+                when x is
+                    Three bool color int ->
+                        { bool, color, int }
+                    Empty ->
+                        { bool: False, color: Red, int: 0 }
+                #"
+            ),
+            (32i64, true, 2u8),
+            (i64, bool, u8)
+        );
+    }
 }

compiler/mono/src/decision_tree.rs

@@ -412,7 +412,7 @@ fn test_at_path<'a>(selected_path: &Path, branch: &Branch<'a>, all_tests: &mut V
                                 arguments.push((Pattern::Underscore, destruct.layout.clone()));
                             }
                             DestructType::Optional(_expr) => {
-                                arguments.push((Pattern::Underscore, destruct.layout.clone()));
+                                // do nothing
                             }
                         }
                     }
@@ -540,23 +540,27 @@ fn to_relevant_branch_help<'a>(
                 ..
             } => {
                 debug_assert!(test_name == &TagName::Global(RECORD_TAG_NAME.into()));
-                let sub_positions = destructs.into_iter().enumerate().map(|(index, destruct)| {
+                let sub_positions = destructs
-                    let pattern = match destruct.typ {
+                    .into_iter()
-                        DestructType::Guard(guard) => guard.clone(),
+                    .filter(|destruct| !matches!(destruct.typ, DestructType::Optional(_)))
-                        DestructType::Required => Pattern::Underscore,
+                    .enumerate()
-                        DestructType::Optional(_expr) => Pattern::Underscore,
+                    .map(|(index, destruct)| {
-                    };
+                        let pattern = match destruct.typ {
+                            DestructType::Guard(guard) => guard.clone(),
+                            DestructType::Required => Pattern::Underscore,
+                            DestructType::Optional(_expr) => unreachable!("because of the filter"),
+                        };
 
-                    (
+                        (
-                        Path::Index {
+                            Path::Index {
-                            index: index as u64,
+                                index: index as u64,
-                            tag_id: *tag_id,
+                                tag_id: *tag_id,
-                            path: Box::new(path.clone()),
+                                path: Box::new(path.clone()),
-                        },
+                            },
-                        Guard::NoGuard,
+                            Guard::NoGuard,
-                        pattern,
+                            pattern,
-                    )
+                        )
-                });
+                    });
                 start.extend(sub_positions);
                 start.extend(end);
 

compiler/mono/src/ir.rs

@@ -907,7 +907,13 @@ where
     if PRETTY_PRINT_IR_SYMBOLS {
         alloc.text(format!("{:?}", symbol))
     } else {
-        alloc.text(format!("{}", symbol))
+        let text = format!("{}", symbol);
+
+        if text.starts_with('.') {
+            alloc.text("Test").append(text)
+        } else {
+            alloc.text(text)
+        }
     }
 }
 
@@ -917,7 +923,7 @@ where
     D::Doc: Clone,
     A: Clone,
 {
-    alloc.text(format!("{}", symbol.0))
+    symbol_to_doc(alloc, symbol.0)
 }
 
 impl<'a> Expr<'a> {
@@ -1101,7 +1107,9 @@ impl<'a> Stmt<'a> {
                     .chain(std::iter::once(default_doc));
                 //
                 alloc
-                    .text(format!("switch {}:", cond_symbol))
+                    .text("switch ")
+                    .append(symbol_to_doc(alloc, *cond_symbol))
+                    .append(":")
                     .append(alloc.hardline())
                     .append(
                         alloc.intersperse(branches_docs, alloc.hardline().append(alloc.hardline())),
@@ -1115,7 +1123,9 @@ impl<'a> Stmt<'a> {
                 fail,
                 ..
             } => alloc
-                .text(format!("if {} then", branching_symbol))
+                .text("if ")
+                .append(symbol_to_doc(alloc, *branching_symbol))
+                .append(" then")
                 .append(alloc.hardline())
                 .append(pass.to_doc(alloc).indent(4))
                 .append(alloc.hardline())
@@ -2384,7 +2394,7 @@ pub fn with_hole<'a>(
         Tag {
             variant_var,
             name: tag_name,
-            arguments: args,
+            arguments: mut args,
             ..
         } => {
             use crate::layout::UnionVariant::*;
@@ -2421,11 +2431,34 @@ pub fn with_hole<'a>(
                 }
 
                 Unwrapped(field_layouts) => {
+                    let mut field_symbols_temp =
+                        Vec::with_capacity_in(field_layouts.len(), env.arena);
+
+                    for (var, arg) in args.drain(..) {
+                        // Layout will unpack this unwrapped tack if it only has one (non-zero-sized) field
+                        let layout = layout_cache
+                            .from_var(env.arena, var, env.subs)
+                            .unwrap_or_else(|err| {
+                                panic!("TODO turn fn_var into a RuntimeError {:?}", err)
+                            });
+
+                        let alignment = layout.alignment_bytes(8);
+
+                        let symbol = possible_reuse_symbol(env, procs, &arg.value);
+                        field_symbols_temp.push((
+                            alignment,
+                            symbol,
+                            ((var, arg), &*env.arena.alloc(symbol)),
+                        ));
+                    }
+                    field_symbols_temp.sort_by(|a, b| b.0.cmp(&a.0));
+
                     let mut field_symbols = Vec::with_capacity_in(field_layouts.len(), env.arena);
 
-                    for (_, arg) in args.iter() {
+                    for (_, symbol, _) in field_symbols_temp.iter() {
-                        field_symbols.push(possible_reuse_symbol(env, procs, &arg.value));
+                        field_symbols.push(*symbol);
                     }
+
                     let field_symbols = field_symbols.into_bump_slice();
 
                     // Layout will unpack this unwrapped tack if it only has one (non-zero-sized) field
@@ -2438,7 +2471,7 @@ pub fn with_hole<'a>(
                     // even though this was originally a Tag, we treat it as a Struct from now on
                     let stmt = Stmt::Let(assigned, Expr::Struct(field_symbols), layout, hole);
 
-                    let iter = args.into_iter().rev().zip(field_symbols.iter().rev());
+                    let iter = field_symbols_temp.into_iter().map(|(_, _, data)| data);
                     assign_to_symbols(env, procs, layout_cache, iter, stmt)
                 }
                 Wrapped(sorted_tag_layouts) => {
@@ -2449,12 +2482,33 @@ pub fn with_hole<'a>(
                         .find(|(_, (key, _))| key == &tag_name)
                         .expect("tag must be in its own type");
 
+                    let mut field_symbols_temp = Vec::with_capacity_in(args.len(), env.arena);
+
+                    for (var, arg) in args.drain(..) {
+                        // Layout will unpack this unwrapped tack if it only has one (non-zero-sized) field
+                        let layout = layout_cache
+                            .from_var(env.arena, var, env.subs)
+                            .unwrap_or_else(|err| {
+                                panic!("TODO turn fn_var into a RuntimeError {:?}", err)
+                            });
+
+                        let alignment = layout.alignment_bytes(8);
+
+                        let symbol = possible_reuse_symbol(env, procs, &arg.value);
+                        field_symbols_temp.push((
+                            alignment,
+                            symbol,
+                            ((var, arg), &*env.arena.alloc(symbol)),
+                        ));
+                    }
+                    field_symbols_temp.sort_by(|a, b| b.0.cmp(&a.0));
+
                     let mut field_symbols: Vec<Symbol> = Vec::with_capacity_in(args.len(), arena);
                     let tag_id_symbol = env.unique_symbol();
                     field_symbols.push(tag_id_symbol);
 
-                    for (_, arg) in args.iter() {
+                    for (_, symbol, _) in field_symbols_temp.iter() {
-                        field_symbols.push(possible_reuse_symbol(env, procs, &arg.value));
+                        field_symbols.push(*symbol);
                     }
 
                     let mut layouts: Vec<&'a [Layout<'a>]> =
@@ -2475,7 +2529,11 @@ pub fn with_hole<'a>(
                     };
 
                     let mut stmt = Stmt::Let(assigned, tag, layout, hole);
-                    let iter = args.into_iter().rev().zip(field_symbols.iter().rev());
+                    let iter = field_symbols_temp
+                        .drain(..)
+                        .map(|x| x.2 .0)
+                        .rev()
+                        .zip(field_symbols.iter().rev());
 
                     stmt = assign_to_symbols(env, procs, layout_cache, iter, stmt);
 
@@ -5290,6 +5348,20 @@ pub fn from_can_pattern<'a>(
                         }],
                     };
 
+                    let mut arguments = arguments.clone();
+
+                    arguments.sort_by(|arg1, arg2| {
+                        let ptr_bytes = 8;
+
+                        let layout1 = layout_cache.from_var(env.arena, arg1.0, env.subs).unwrap();
+                        let layout2 = layout_cache.from_var(env.arena, arg2.0, env.subs).unwrap();
+
+                        let size1 = layout1.alignment_bytes(ptr_bytes);
+                        let size2 = layout2.alignment_bytes(ptr_bytes);
+
+                        size2.cmp(&size1)
+                    });
+
                     let mut mono_args = Vec::with_capacity_in(arguments.len(), env.arena);
                     for ((_, loc_pat), layout) in arguments.iter().zip(field_layouts.iter()) {
                         mono_args.push((
@@ -5333,6 +5405,20 @@ pub fn from_can_pattern<'a>(
                     let mut mono_args = Vec::with_capacity_in(arguments.len(), env.arena);
                     // disregard the tag discriminant layout
 
+                    let mut arguments = arguments.clone();
+
+                    arguments.sort_by(|arg1, arg2| {
+                        let ptr_bytes = 8;
+
+                        let layout1 = layout_cache.from_var(env.arena, arg1.0, env.subs).unwrap();
+                        let layout2 = layout_cache.from_var(env.arena, arg2.0, env.subs).unwrap();
+
+                        let size1 = layout1.alignment_bytes(ptr_bytes);
+                        let size2 = layout2.alignment_bytes(ptr_bytes);
+
+                        size2.cmp(&size1)
+                    });
+
                     // TODO make this assert pass, it currently does not because
                     // 0-sized values are dropped out
                     // debug_assert_eq!(arguments.len(), argument_layouts[1..].len());
@@ -5374,8 +5460,8 @@ pub fn from_can_pattern<'a>(
 
             // sorted fields based on the destruct
             let mut mono_destructs = Vec::with_capacity_in(destructs.len(), env.arena);
-            let mut destructs = destructs.clone();
+            let destructs_by_label = env.arena.alloc(MutMap::default());
-            destructs.sort_by(|a, b| a.value.label.cmp(&b.value.label));
+            destructs_by_label.extend(destructs.iter().map(|x| (&x.value.label, x)));
 
             let mut field_layouts = Vec::with_capacity_in(sorted_fields.len(), env.arena);
 
@@ -5387,119 +5473,96 @@ pub fn from_can_pattern<'a>(
             // in the source the field is not matche in the source language.
             //
             // Optional fields somewhat complicate the matter here
-            let mut it1 = sorted_fields.into_iter();
-            let mut opt_sorted = it1.next();
 
-            let mut it2 = destructs.iter();
+            for (label, variable, res_layout) in sorted_fields.into_iter() {
-            let mut opt_destruct = it2.next();
+                match res_layout {
+                    Ok(field_layout) => {
+                        // the field is non-optional according to the type
 
-            loop {
+                        match destructs_by_label.remove(&label) {
-                match (opt_sorted, opt_destruct) {
+                            Some(destruct) => {
-                    (Some((label, variable, Ok(field_layout))), Some(destruct)) => {
+                                // this field is destructured by the pattern
-                        if destruct.value.label == label {
+                                mono_destructs.push(from_can_record_destruct(
-                            mono_destructs.push(from_can_record_destruct(
+                                    env,
-                                env,
+                                    layout_cache,
-                                layout_cache,
+                                    &destruct.value,
-                                &destruct.value,
+                                    field_layout.clone(),
-                                field_layout.clone(),
+                                ));
-                            ));
+                            }
-
+                            None => {
-                            opt_sorted = it1.next();
+                                // this field is not destructured by the pattern
-                            opt_destruct = it2.next();
+                                // put in an underscore
-                        } else {
+                                mono_destructs.push(RecordDestruct {
-                            // insert underscore pattern
+                                    label: label.clone(),
-                            mono_destructs.push(RecordDestruct {
+                                    symbol: env.unique_symbol(),
-                                label: label.clone(),
+                                    variable,
-                                symbol: env.unique_symbol(),
+                                    layout: field_layout.clone(),
-                                variable,
+                                    typ: DestructType::Guard(Pattern::Underscore),
-                                layout: field_layout.clone(),
+                                });
-                                typ: DestructType::Guard(Pattern::Underscore),
+                            }
-                            });
-
-                            opt_sorted = it1.next();
                         }
+
+                        // the layout of this field is part of the layout of the record
                         field_layouts.push(field_layout);
                     }
-                    (Some((label, variable, Err(field_layout))), Some(destruct)) => {
+                    Err(field_layout) => {
-                        if destruct.value.label == label {
+                        // the field is optional according to the type
-                            opt_destruct = it2.next();
+                        match destructs_by_label.remove(&label) {
-
+                            Some(destruct) => {
-                            mono_destructs.push(RecordDestruct {
+                                // this field is destructured by the pattern
-                                label: destruct.value.label.clone(),
-                                symbol: destruct.value.symbol,
-                                layout: field_layout,
-                                variable,
-                                typ: match &destruct.value.typ {
-                                    roc_can::pattern::DestructType::Optional(_, loc_expr) => {
-                                        // if we reach this stage, the optional field is not present
-                                        // so use the default
-                                        DestructType::Optional(loc_expr.value.clone())
-                                    }
-                                    _ => unreachable!(
-                                        "only optional destructs can be optional fields"
-                                    ),
-                                },
-                            });
-                        }
-                        opt_sorted = it1.next();
-                    }
-
-                    (Some((label, variable, Err(field_layout))), None) => {
-                        // the remainder of the fields (from the type) is not matched on in
-                        // this pattern; to fill it out, we put underscores
-                        mono_destructs.push(RecordDestruct {
-                            label: label.clone(),
-                            symbol: env.unique_symbol(),
-                            variable,
-                            layout: field_layout.clone(),
-                            typ: DestructType::Guard(Pattern::Underscore),
-                        });
-
-                        opt_sorted = it1.next();
-                    }
-
-                    (Some((label, variable, Ok(field_layout))), None) => {
-                        // the remainder of the fields (from the type) is not matched on in
-                        // this pattern; to fill it out, we put underscores
-                        mono_destructs.push(RecordDestruct {
-                            label: label.clone(),
-                            symbol: env.unique_symbol(),
-                            variable,
-                            layout: field_layout.clone(),
-                            typ: DestructType::Guard(Pattern::Underscore),
-                        });
-
-                        field_layouts.push(field_layout);
-                        opt_sorted = it1.next();
-                    }
-                    (None, Some(destruct)) => {
-                        // destruct is not in the type, but is in the pattern
-                        // it must be an optional field, and we will use the default
-                        match &destruct.value.typ {
-                            roc_can::pattern::DestructType::Optional(field_var, loc_expr) => {
-                                let field_layout = layout_cache
-                                    .from_var(env.arena, *field_var, env.subs)
-                                    .unwrap_or_else(|err| {
-                                        panic!("TODO turn fn_var into a RuntimeError {:?}", err)
-                                    });
-
                                 mono_destructs.push(RecordDestruct {
                                     label: destruct.value.label.clone(),
                                     symbol: destruct.value.symbol,
-                                    variable: destruct.value.var,
                                     layout: field_layout,
-                                    typ: DestructType::Optional(loc_expr.value.clone()),
+                                    variable,
-                                })
+                                    typ: match &destruct.value.typ {
+                                        roc_can::pattern::DestructType::Optional(_, loc_expr) => {
+                                            // if we reach this stage, the optional field is not present
+                                            // so use the default
+                                            DestructType::Optional(loc_expr.value.clone())
+                                        }
+                                        _ => unreachable!(
+                                            "only optional destructs can be optional fields"
+                                        ),
+                                    },
+                                });
+                            }
+                            None => {
+                                // this field is not destructured by the pattern
+                                // put in an underscore
+                                mono_destructs.push(RecordDestruct {
+                                    label: label.clone(),
+                                    symbol: env.unique_symbol(),
+                                    variable,
+                                    layout: field_layout.clone(),
+                                    typ: DestructType::Guard(Pattern::Underscore),
+                                });
                             }
-                            _ => unreachable!("only optional destructs can be optional fields"),
                         }
+                    }
+                }
+            }
 
-                        opt_sorted = None;
+            for (_, destruct) in destructs_by_label.drain() {
-                        opt_destruct = it2.next();
+                // this destruct is not in the type, but is in the pattern
-                    }
+                // it must be an optional field, and we will use the default
-                    (None, None) => {
+                match &destruct.value.typ {
-                        break;
+                    roc_can::pattern::DestructType::Optional(field_var, loc_expr) => {
+                        let field_layout = layout_cache
+                            .from_var(env.arena, *field_var, env.subs)
+                            .unwrap_or_else(|err| {
+                                panic!("TODO turn fn_var into a RuntimeError {:?}", err)
+                            });
+
+                        mono_destructs.push(RecordDestruct {
+                            label: destruct.value.label.clone(),
+                            symbol: destruct.value.symbol,
+                            variable: destruct.value.var,
+                            layout: field_layout,
+                            typ: DestructType::Optional(loc_expr.value.clone()),
+                        })
                     }
+                    _ => unreachable!("only optional destructs can be optional fields"),
                 }
             }
 

compiler/mono/src/layout.rs

@@ -4,6 +4,7 @@ use roc_collections::all::{default_hasher, MutMap, MutSet};
 use roc_module::ident::{Lowercase, TagName};
 use roc_module::symbol::{Interns, Symbol};
 use roc_types::subs::{Content, FlatType, Subs, Variable};
+use roc_types::types::RecordField;
 use std::collections::HashMap;
 
 pub const MAX_ENUM_SIZE: usize = (std::mem::size_of::<u8>() * 8) as usize;
@@ -789,59 +790,30 @@ fn layout_from_flat_type<'a>(
             }
         }
         Record(fields, ext_var) => {
-            // Sort the fields by label
-            let mut sorted_fields = Vec::with_capacity_in(fields.len(), arena);
-            sorted_fields.extend(fields.into_iter());
-
             // extract any values from the ext_var
             let mut fields_map = MutMap::default();
+            fields_map.extend(fields);
             match roc_types::pretty_print::chase_ext_record(subs, ext_var, &mut fields_map) {
                 Ok(()) | Err((_, Content::FlexVar(_))) => {}
                 Err(_) => unreachable!("this would have been a type error"),
             }
 
-            sorted_fields.extend(fields_map.into_iter());
+            let sorted_fields = sort_record_fields_help(env, fields_map);
-
-            sorted_fields.sort_by(|(label1, _), (label2, _)| label1.cmp(label2));
 
             // Determine the layouts of the fields, maintaining sort order
             let mut layouts = Vec::with_capacity_in(sorted_fields.len(), arena);
 
-            for (label, field) in sorted_fields {
+            for (_, _, res_layout) in sorted_fields {
-                use LayoutProblem::*;
+                match res_layout {
-
-                let field_var = {
-                    use roc_types::types::RecordField::*;
-                    match field {
-                        Optional(_) => {
-                            // when an optional field reaches this stage, the field was truly
-                            // optional, and not unified to be demanded or required
-                            // therefore, there is no such field on the record, and we ignore this
-                            // field from now on.
-                            continue;
-                        }
-                        Required(var) => var,
-                        Demanded(var) => var,
-                    }
-                };
-
-                match Layout::from_var(env, field_var) {
                     Ok(layout) => {
                         // Drop any zero-sized fields like {}.
                         if !layout.is_dropped_because_empty() {
                             layouts.push(layout);
                         }
                     }
-                    Err(UnresolvedTypeVar(v)) => {
+                    Err(_) => {
-                        // Invalid field!
+                        // optional field, ignore
-                        panic!(
+                        continue;
-                            r"I hit an unresolved type var {:?} when determining the layout of {:?} of record field: {:?} : {:?}",
-                            field_var, v, label, field
-                        );
-                    }
-                    Err(Erroneous) => {
-                        // Invalid field!
-                        panic!("TODO gracefully handle record with invalid field.var");
                     }
                 }
             }
@@ -894,6 +866,15 @@ fn layout_from_flat_type<'a>(
                     tag_layout.push(Layout::from_var(env, var)?);
                 }
 
+                tag_layout.sort_by(|layout1, layout2| {
+                    let ptr_bytes = 8;
+
+                    let size1 = layout1.alignment_bytes(ptr_bytes);
+                    let size2 = layout2.alignment_bytes(ptr_bytes);
+
+                    size2.cmp(&size1)
+                });
+
                 tag_layouts.push(tag_layout.into_bump_slice());
             }
 
@@ -924,39 +905,55 @@ pub fn sort_record_fields<'a>(
     };
 
     match roc_types::pretty_print::chase_ext_record(subs, var, &mut fields_map) {
-        Ok(()) | Err((_, Content::FlexVar(_))) => {
+        Ok(()) | Err((_, Content::FlexVar(_))) => sort_record_fields_help(&mut env, fields_map),
-            // Sort the fields by label
-            let mut sorted_fields = Vec::with_capacity_in(fields_map.len(), arena);
-
-            use roc_types::types::RecordField;
-            for (label, field) in fields_map {
-                let var = match field {
-                    RecordField::Demanded(v) => v,
-                    RecordField::Required(v) => v,
-                    RecordField::Optional(v) => {
-                        let layout =
-                            Layout::from_var(&mut env, v).expect("invalid layout from var");
-                        sorted_fields.push((label, v, Err(layout)));
-                        continue;
-                    }
-                };
-
-                let layout = Layout::from_var(&mut env, var).expect("invalid layout from var");
-
-                // Drop any zero-sized fields like {}
-                if !layout.is_dropped_because_empty() {
-                    sorted_fields.push((label, var, Ok(layout)));
-                }
-            }
-
-            sorted_fields.sort_by(|(label1, _, _), (label2, _, _)| label1.cmp(label2));
-
-            sorted_fields
-        }
         Err(other) => panic!("invalid content in record variable: {:?}", other),
     }
 }
 
+fn sort_record_fields_help<'a>(
+    env: &mut Env<'a, '_>,
+    fields_map: MutMap<Lowercase, RecordField<Variable>>,
+) -> Vec<'a, (Lowercase, Variable, Result<Layout<'a>, Layout<'a>>)> {
+    // Sort the fields by label
+    let mut sorted_fields = Vec::with_capacity_in(fields_map.len(), env.arena);
+
+    for (label, field) in fields_map {
+        let var = match field {
+            RecordField::Demanded(v) => v,
+            RecordField::Required(v) => v,
+            RecordField::Optional(v) => {
+                let layout = Layout::from_var(env, v).expect("invalid layout from var");
+                sorted_fields.push((label, v, Err(layout)));
+                continue;
+            }
+        };
+
+        let layout = Layout::from_var(env, var).expect("invalid layout from var");
+
+        // Drop any zero-sized fields like {}
+        if !layout.is_dropped_because_empty() {
+            sorted_fields.push((label, var, Ok(layout)));
+        }
+    }
+
+    sorted_fields.sort_by(
+        |(label1, _, res_layout1), (label2, _, res_layout2)| match res_layout1 {
+            Ok(layout1) | Err(layout1) => match res_layout2 {
+                Ok(layout2) | Err(layout2) => {
+                    let ptr_bytes = 8;
+
+                    let size1 = layout1.alignment_bytes(ptr_bytes);
+                    let size2 = layout2.alignment_bytes(ptr_bytes);
+
+                    size2.cmp(&size1).then(label1.cmp(label2))
+                }
+            },
+        },
+    );
+
+    sorted_fields
+}
+
 #[derive(Clone, Debug, PartialEq, Eq, Hash)]
 pub enum UnionVariant<'a> {
     Never,
@@ -1059,6 +1056,15 @@ pub fn union_sorted_tags_help<'a>(
                 }
             }
 
+            layouts.sort_by(|layout1, layout2| {
+                let ptr_bytes = 8;
+
+                let size1 = layout1.alignment_bytes(ptr_bytes);
+                let size2 = layout2.alignment_bytes(ptr_bytes);
+
+                size2.cmp(&size1)
+            });
+
             if layouts.is_empty() {
                 if contains_zero_sized {
                     UnionVariant::UnitWithArguments
@@ -1102,6 +1108,15 @@ pub fn union_sorted_tags_help<'a>(
                     }
                 }
 
+                arg_layouts.sort_by(|layout1, layout2| {
+                    let ptr_bytes = 8;
+
+                    let size1 = layout1.alignment_bytes(ptr_bytes);
+                    let size2 = layout2.alignment_bytes(ptr_bytes);
+
+                    size2.cmp(&size1)
+                });
+
                 answer.push((tag_name, arg_layouts.into_bump_slice()));
             }