make arbitrary AccessAtIndex work

it now uses that actual layout, not a hardcoded one

compiler/gen/src/crane/build.rs

@@ -205,7 +205,7 @@ pub fn build_expr<'a, B: Backend>(
                 .ins()
                 .stack_addr(cfg.pointer_type(), slot, Offset32::new(0))
         }
-        Tag { union_size, tag_layout, arguments , ..} if *union_size == 1 => {
+        Tag { tag_layout, arguments , tag_id, union_size, .. } => {
             let cfg = env.cfg;
             let ptr_bytes = cfg.pointer_bytes() as u32;
 
@@ -220,11 +220,18 @@ pub fn build_expr<'a, B: Backend>(
                 slot_size
             ));
 
-            let it = std::iter::empty().chain(arguments.iter());
-
             // Create instructions for storing each field's expression
             let mut offset = 0;
-            for (field_expr, field_layout) in it {
+
+            // still need to insert the tag discriminator for non-single unions
+            // when there are no arguments, e.g. `Nothing : Maybe a`
+            if *union_size > 1 {
+                let val = builder.ins().iconst(types::I64, *tag_id as i64);
+                builder.ins().stack_store(val, slot, Offset32::new(0));
+                offset += ptr_bytes;
+            }
+
+            for (field_expr, field_layout) in arguments.iter() {
                 let val = build_expr(env, &scope, module, builder, field_expr, procs);
 
                 let field_size = field_layout.stack_size(ptr_bytes);
@@ -236,42 +243,6 @@ pub fn build_expr<'a, B: Backend>(
                 offset += field_size;
             }
 
-            builder
-                .ins()
-                .stack_addr(cfg.pointer_type(), slot, Offset32::new(0))
-        }
-        Tag { tag_id,  tag_layout, arguments , ..} => {
-            let cfg = env.cfg;
-            let ptr_bytes = cfg.pointer_bytes() as u32;
-
-            // NOTE: all variants of a tag union must have the same size, so (among other things)
-            // it's easy to quickly index them in arrays. Therefore the size of this tag doens't
-            // depend on the tag arguments, but solely on the layout of the whole tag union
-            let slot_size = tag_layout.stack_size(ptr_bytes);
-
-            // Create a slot
-            let slot = builder.create_stack_slot(StackSlotData::new(
-                StackSlotKind::ExplicitSlot,
-                slot_size
-            ));
-
-            // put the discriminant in the first slot
-            let discriminant = (Expr::Int(*tag_id as i64), Layout::Builtin(Builtin::Int64));
-            let it = std::iter::once(&discriminant).chain(arguments.iter());
-
-            // Create instructions for storing each field's expression
-            let mut offset = 0;
-            for (field_expr, field_layout) in it {
-                let val = build_expr(env, &scope, module, builder, field_expr, procs);
-
-                let field_size = field_layout.stack_size(ptr_bytes);
-                let field_offset = i32::try_from(offset)
-                    .expect("TODO handle field size conversion to i32");
-
-                builder.ins().stack_store(val, slot, Offset32::new(field_offset));
-
-                offset += field_size;
-            }
 
             builder
                 .ins()
@@ -337,6 +308,7 @@ pub fn build_expr<'a, B: Backend>(
             let mut offset = 0;
 
 
+
             for (field_index, field_layout) in field_layouts.iter().enumerate() {
                 if *index == field_index as u64 {
                     let offset = i32::try_from(offset)

compiler/gen/src/llvm/build.rs

@@ -352,18 +352,16 @@ pub fn build_expr<'a, 'ctx, 'env>(
             BasicValueEnum::StructValue(struct_val.into_struct_value())
         }
         Tag {
-            tag_id,
             arguments,
             tag_layout,
+            union_size,
+            tag_id,
             ..
         } => {
             let ptr_size = env.pointer_bytes;
 
             let whole_size = tag_layout.stack_size(ptr_size);
             let mut filler = tag_layout.stack_size(ptr_size);
-            // put the discriminant in the first slot
-            let discriminant = (Expr::Int(*tag_id as i64), Layout::Builtin(Builtin::Int64));
-            let it = std::iter::once(&discriminant).chain(arguments.iter());
 
             let ctx = env.context;
             let builder = env.builder;
@@ -373,7 +371,24 @@ pub fn build_expr<'a, 'ctx, 'env>(
             let mut field_types = Vec::with_capacity_in(num_fields, env.arena);
             let mut field_vals = Vec::with_capacity_in(num_fields, env.arena);
 
-            for (field_expr, field_layout) in it {
+            // insert the discriminant value
+            if *union_size > 1 {
+                let val = env
+                    .context
+                    .i64_type()
+                    .const_int(*tag_id as u64, true)
+                    .into();
+
+                let field_type = env.context.i64_type().into();
+
+                field_types.push(field_type);
+                field_vals.push(val);
+
+                let field_size = ptr_size;
+                filler -= field_size;
+            }
+
+            for (field_expr, field_layout) in arguments.iter() {
                 let val = build_expr(env, &scope, parent, field_expr, procs);
                 let field_type = basic_type_from_layout(env.arena, env.context, &field_layout);
 

compiler/mono/src/decision_tree.rs

@@ -44,7 +44,7 @@ pub enum Test<'a> {
         tag_id: u8,
         tag_name: TagName,
         union: crate::pattern::Union,
-        arguments: Vec<Pattern<'a>>,
+        arguments: Vec<(Pattern<'a>, Layout<'a>)>,
     },
     IsInt(i64),
     // float patterns are stored as u64 so they are comparable/hashable
@@ -261,7 +261,7 @@ fn test_at_path<'a>(selected_path: &Path, branch: Branch<'a>) -> Option<Test<'a>
                 tag_id: *tag_id,
                 tag_name: tag_name.clone(),
                 union: union.clone(),
-                arguments: arguments.clone().into_iter().map(|v| v.0).collect(),
+                arguments: arguments.to_vec(),
             }),
             BitLiteral(v) => Some(IsBit(*v)),
             EnumLiteral { tag_id, enum_size } => Some(IsByte {
@@ -711,7 +711,7 @@ fn decide_to_branching<'a>(
     cond_symbol: Symbol,
     cond_layout: Layout<'a>,
     ret_layout: Layout<'a>,
-    decider: Decider<Choice<'a>>,
+    decider: Decider<'a, Choice<'a>>,
     jumps: &Vec<(u64, Expr<'a>)>,
 ) -> Expr<'a> {
     use Choice::*;
@@ -733,18 +733,29 @@ fn decide_to_branching<'a>(
 
             for (path, test) in test_chain {
                 match test {
-                    Test::IsCtor { tag_id, union, .. } => {
+                    Test::IsCtor {
+                        tag_id,
+                        union,
+                        arguments,
+                        ..
+                    } => {
                         let lhs = Expr::Int(tag_id as i64);
 
-                        // NOTE this is hardcoded, and should be made dynamic
+                        let mut field_layouts =
-                        let field_layouts = env.arena.alloc([
+                            bumpalo::collections::Vec::with_capacity_in(arguments.len(), env.arena);
-                            Layout::Builtin(Builtin::Int64),
+
-                            Layout::Builtin(Builtin::Int64),
+                        if union.alternatives.len() > 1 {
-                            Layout::Builtin(Builtin::Int64),
+                            // the tag discriminant
-                        ]);
+                            field_layouts.push(Layout::Builtin(Builtin::Int64));
+                        }
+
+                        for (_, layout) in arguments {
+                            field_layouts.push(layout);
+                        }
+
                         let rhs = Expr::AccessAtIndex {
                             index: 0,
-                            field_layouts,
+                            field_layouts: field_layouts.into_bump_slice(),
                             expr: env.arena.alloc(Expr::Load(cond_symbol)),
                             is_unwrapped: union.alternatives.len() == 1,
                         };

compiler/mono/src/expr.rs

@@ -8,7 +8,7 @@ use roc_module::ident::{Ident, Lowercase, TagName};
 use roc_module::symbol::{IdentIds, ModuleId, Symbol};
 use roc_region::all::{Located, Region};
 use roc_types::subs::{Content, ContentHash, FlatType, Subs, Variable};
-use std::hash::{Hash, Hasher};
+use std::hash::Hash;
 
 #[derive(Clone, Debug, PartialEq, Default)]
 pub struct Procs<'a> {
@@ -1195,7 +1195,7 @@ fn specialize_proc_body<'a>(
 
 /// A pattern, including possible problems (e.g. shadowing) so that
 /// codegen can generate a runtime error if this pattern is reached.
-#[derive(Clone, Debug, PartialEq, Eq)]
+#[derive(Clone, Debug, PartialEq, Eq, Hash)]
 pub enum Pattern<'a> {
     Identifier(Symbol),
     Underscore,
@@ -1231,76 +1231,6 @@ pub struct RecordDestruct<'a> {
     pub guard: Option<Pattern<'a>>,
 }
 
-impl<'a> Hash for Pattern<'a> {
-    fn hash<H: Hasher>(&self, state: &mut H) {
-        use Pattern::*;
-
-        match self {
-            Identifier(symbol) => {
-                state.write_u8(0);
-                symbol.hash(state);
-            }
-            Underscore => {
-                state.write_u8(1);
-            }
-
-            IntLiteral(v) => {
-                state.write_u8(2);
-                v.hash(state);
-            }
-            FloatLiteral(v) => {
-                state.write_u8(3);
-                v.hash(state);
-            }
-            BitLiteral(v) => {
-                state.write_u8(4);
-                v.hash(state);
-            }
-            EnumLiteral { tag_id, enum_size } => {
-                state.write_u8(5);
-                tag_id.hash(state);
-                enum_size.hash(state);
-            }
-            StrLiteral(v) => {
-                state.write_u8(6);
-                v.hash(state);
-            }
-
-            RecordDestructure(fields, _layout) => {
-                state.write_u8(7);
-                fields.hash(state);
-                // layout is ignored!
-            }
-
-            AppliedTag {
-                tag_name,
-                arguments,
-                union,
-                ..
-            } => {
-                state.write_u8(8);
-                tag_name.hash(state);
-                for (pat, _) in arguments {
-                    pat.hash(state);
-                }
-                union.hash(state);
-                // layout is ignored!
-            }
-
-            Shadowed(region, ident) => {
-                state.write_u8(9);
-                region.hash(state);
-                ident.hash(state);
-            }
-
-            UnsupportedPattern(region) => {
-                state.write_u8(10);
-                region.hash(state);
-            }
-        }
-    }
-}
-
 fn from_can_pattern<'a>(
     env: &mut Env<'a, '_>,
     can_pattern: &roc_can::pattern::Pattern,

compiler/mono/src/layout.rs

@@ -110,24 +110,16 @@ impl<'a> Layout<'a> {
 
                 sum
             }
-            Union(fields) => {
+            Union(fields) => fields
-                // the tag gets converted to a u8, so 1 byte.
+                .iter()
-                // But for one-tag unions, we don't store the tag, so 0 bytes
+                .map(|tag_layout| {
-                let discriminant_size: u32 = if fields.len() > 1 { pointer_size } else { 0 };
+                    tag_layout
-
+                        .iter()
-                let max_tag_size: u32 = fields
+                        .map(|field| field.stack_size(pointer_size))
-                    .iter()
+                        .sum()
-                    .map(|tag_layout| {
+                })
-                        tag_layout
+                .max()
-                            .iter()
+                .unwrap_or_default(),
-                            .map(|field| field.stack_size(pointer_size))
-                            .sum()
-                    })
-                    .max()
-                    .unwrap_or_default();
-
-                discriminant_size + max_tag_size
-            }
             FunctionPointer(_, _) => pointer_size,
         }
     }
@@ -390,9 +382,19 @@ pub fn layout_from_tag_union<'a>(
                     Ok(Layout::Builtin(Builtin::Byte))
                 }
             } else {
+                let add_discriminant = tags.len() != 1;
                 let mut layouts = Vec::with_capacity_in(tags.len(), arena);
+
                 for arguments in tags.values() {
-                    let mut arg_layouts = Vec::with_capacity_in(arguments.len(), arena);
+                    // add a field for the discriminant if there is more than one tag in the union
+                    let mut arg_layouts = if add_discriminant {
+                        let discriminant = Layout::Builtin(Builtin::Int64);
+                        let mut result = Vec::with_capacity_in(arguments.len() + 1, arena);
+                        result.push(discriminant);
+                        result
+                    } else {
+                        Vec::with_capacity_in(arguments.len(), arena)
+                    };
 
                     for arg in arguments {
                         arg_layouts.push(Layout::from_var(arena, *arg, subs, pointer_size)?);

compiler/mono/src/pattern.rs

@@ -12,6 +12,7 @@ pub struct Union {
 #[derive(Clone, Debug, PartialEq, Eq, Hash)]
 pub struct Ctor {
     pub name: TagName,
+    // pub tag_id: u8,
     pub arity: usize,
 }