always Cond on a symbol

this will make 'beans' easier

compiler/gen/src/crane/build.rs

@@ -83,8 +83,10 @@ pub fn build_expr<'a, B: Backend>(
             cond_layout,
             ret_layout,
         } => {
+            let cond_value = load_symbol(env, scope, builder, *cond);
+
             let branch = Branch2 {
-                cond,
+                cond: cond_value,
                 pass,
                 fail,
                 cond_layout,
@@ -115,7 +117,7 @@ pub fn build_expr<'a, B: Backend>(
             let mut scope = im_rc::HashMap::clone(scope);
             let cfg = env.cfg;
 
-            let ptr_size = cfg.pointer_bytes()  as u32;
+            let ptr_size = cfg.pointer_bytes() as u32;
             for (name, layout, expr) in stores.iter() {
                 let stack_size = layout.stack_size(ptr_size);
 
@@ -127,10 +129,8 @@ pub fn build_expr<'a, B: Backend>(
                 let val = build_expr(env, &scope, module, builder, &expr, procs);
                 let expr_type = type_from_layout(cfg, &layout);
 
-                let slot = builder.create_stack_slot(StackSlotData::new(
+                let slot = builder
-                    StackSlotKind::ExplicitSlot,
+                    .create_stack_slot(StackSlotData::new(StackSlotKind::ExplicitSlot, stack_size));
-                    stack_size,
-                ));
 
                 builder.ins().stack_store(val, slot, Offset32::new(0));
 
@@ -175,37 +175,7 @@ pub fn build_expr<'a, B: Backend>(
 
             results[0]
         }
-        Load(name) => match scope.get(name) {
+        Load(name) => load_symbol(env, scope, builder, *name),
-            Some(ScopeEntry::Stack { expr_type, slot }) => {
-                builder
-                    .ins()
-                    .stack_load(*expr_type, *slot, Offset32::new(0))
-            }
-            Some(ScopeEntry::Arg { param, .. }) => *param,
-            Some(ScopeEntry::Heap { expr_type, ptr }) => {
-                builder
-                    .ins()
-                    .load(*expr_type, MemFlags::new(), *ptr, Offset32::new(0))
-            }
-            Some(ScopeEntry::Func { .. }) => {
-                panic!("TODO I don't yet know how to return fn pointers")
-            }
-            Some(ScopeEntry::ZeroSized) => {
-                // Create a slot
-                let slot = builder.create_stack_slot(StackSlotData::new(
-                    StackSlotKind::ExplicitSlot,
-                    0
-                ));
-
-                builder
-                    .ins()
-                    .stack_addr(env.cfg.pointer_type(), slot, Offset32::new(0))
-            }
-            None => panic!(
-                "Could not resolve lookup for {:?} because no ScopeEntry was found for {:?} in scope {:?}",
-                name, name, scope
-            ),
-        },
         Struct(sorted_fields) => {
             let cfg = env.cfg;
 
@@ -217,10 +187,8 @@ pub fn build_expr<'a, B: Backend>(
             }
 
             // Create a slot
-            let slot = builder.create_stack_slot(StackSlotData::new(
+            let slot = builder
-                StackSlotKind::ExplicitSlot,
+                .create_stack_slot(StackSlotData::new(StackSlotKind::ExplicitSlot, slot_size));
-                slot_size
-            ));
 
             // Create instructions for storing each field's expression
             // NOTE assumes that all fields have the same width!
@@ -238,7 +206,11 @@ pub fn build_expr<'a, B: Backend>(
                 .ins()
                 .stack_addr(cfg.pointer_type(), slot, Offset32::new(0))
         }
-        Tag { tag_layout, arguments, .. } => {
+        Tag {
+            tag_layout,
+            arguments,
+            ..
+        } => {
             let cfg = env.cfg;
             let ptr_bytes = cfg.pointer_bytes() as u32;
 
@@ -248,10 +220,8 @@ pub fn build_expr<'a, B: Backend>(
             let slot_size = tag_layout.stack_size(ptr_bytes);
 
             // Create a slot
-            let slot = builder.create_stack_slot(StackSlotData::new(
+            let slot = builder
-                StackSlotKind::ExplicitSlot,
+                .create_stack_slot(StackSlotData::new(StackSlotKind::ExplicitSlot, slot_size));
-                slot_size
-            ));
 
             // Create instructions for storing each field's expression
             let mut offset = 0;
@@ -260,10 +230,12 @@ pub fn build_expr<'a, B: Backend>(
                 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)
+                let field_offset =
-                    .expect("TODO handle field size conversion to i32");
+                    i32::try_from(offset).expect("TODO handle field size conversion to i32");
 
-                builder.ins().stack_store(val, slot, Offset32::new(field_offset));
+                builder
+                    .ins()
+                    .stack_store(val, slot, Offset32::new(field_offset));
 
                 offset += field_size;
             }
@@ -281,8 +253,6 @@ pub fn build_expr<'a, B: Backend>(
             let cfg = env.cfg;
             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)
@@ -301,8 +271,10 @@ pub fn build_expr<'a, B: Backend>(
                 offset += field_layout.stack_size(ptr_bytes);
             }
 
-            panic!("field access out of bounds: index {:?} in layouts {:?}", index, field_layouts)
+            panic!(
-
+                "field access out of bounds: index {:?} in layouts {:?}",
+                index, field_layouts
+            )
         }
 
         Str(str_literal) => {
@@ -375,14 +347,18 @@ pub fn build_expr<'a, B: Backend>(
 
             // Store the length
             {
-                let length = builder.ins().iconst(env.ptr_sized_int(), elems.len() as i64);
+                let length = builder
+                    .ins()
+                    .iconst(env.ptr_sized_int(), elems.len() as i64);
                 let offset = Offset32::new((Builtin::WRAPPER_LEN * ptr_bytes) as i32);
 
                 builder.ins().stack_store(length, slot, offset);
             }
 
             // Return the pointer to the wrapper
-            builder.ins().stack_addr(cfg.pointer_type(), slot, Offset32::new(0))
+            builder
+                .ins()
+                .stack_addr(cfg.pointer_type(), slot, Offset32::new(0))
         }
         _ => {
             panic!("I don't yet know how to crane build {:?}", expr);
@@ -390,8 +366,47 @@ pub fn build_expr<'a, B: Backend>(
     }
 }
 
+fn load_symbol<'a>(
+    env: &mut Env<'a>,
+    scope: &Scope,
+    builder: &mut FunctionBuilder,
+    name: Symbol,
+) -> Value {
+    match scope.get(&name) {
+            Some(ScopeEntry::Stack { expr_type, slot }) => {
+                builder
+                    .ins()
+                    .stack_load(*expr_type, *slot, Offset32::new(0))
+            }
+            Some(ScopeEntry::Arg { param, .. }) => *param,
+            Some(ScopeEntry::Heap { expr_type, ptr }) => {
+                builder
+                    .ins()
+                    .load(*expr_type, MemFlags::new(), *ptr, Offset32::new(0))
+            }
+            Some(ScopeEntry::Func { .. }) => {
+                panic!("TODO I don't yet know how to return fn pointers")
+            }
+            Some(ScopeEntry::ZeroSized) => {
+                // Create a slot
+                let slot = builder.create_stack_slot(StackSlotData::new(
+                    StackSlotKind::ExplicitSlot,
+                    0
+                ));
+
+                builder
+                    .ins()
+                    .stack_addr(env.cfg.pointer_type(), slot, Offset32::new(0))
+            }
+            None => panic!(
+                "Could not resolve lookup for {:?} because no ScopeEntry was found for {:?} in scope {:?}",
+                name, name, scope
+            ),
+        }
+}
+
 struct Branch2<'a> {
-    cond: &'a Expr<'a>,
+    cond: Value,
     cond_layout: &'a Layout<'a>,
     pass: &'a Expr<'a>,
     fail: &'a Expr<'a>,
@@ -417,7 +432,7 @@ fn build_branch2<'a, B: Backend>(
 
     builder.declare_var(ret, ret_type);
 
-    let cond = build_expr(env, scope, module, builder, branch.cond, procs);
+    let cond = branch.cond;
     let pass_block = builder.create_block();
     let fail_block = builder.create_block();
 
@@ -763,8 +778,10 @@ fn call_by_name<'a, B: Backend>(
         Symbol::BOOL_OR => {
             debug_assert!(args.len() == 2);
 
+            let cond = build_arg(&args[0], env, scope, module, builder, procs);
+
             let branch2 = Branch2 {
-                cond: &args[0].0,
+                cond,
                 cond_layout: &Layout::Builtin(Builtin::Bool),
                 pass: &Expr::Bool(true),
                 fail: &args[1].0,
@@ -775,8 +792,10 @@ fn call_by_name<'a, B: Backend>(
         Symbol::BOOL_AND => {
             debug_assert!(args.len() == 2);
 
+            let cond = build_arg(&args[0], env, scope, module, builder, procs);
+
             let branch2 = Branch2 {
-                cond: &args[0].0,
+                cond,
                 cond_layout: &Layout::Builtin(Builtin::Bool),
                 pass: &args[1].0,
                 fail: &Expr::Bool(false),

compiler/gen/src/llvm/build.rs

@@ -46,7 +46,7 @@ pub fn build_expr<'a, 'ctx, 'env>(
     env: &Env<'a, 'ctx, 'env>,
     scope: &Scope<'a, 'ctx>,
     parent: FunctionValue<'ctx>,
-    expr: &Expr<'a>,
+    expr: &'a Expr<'a>,
     procs: &Procs<'a>,
 ) -> BasicValueEnum<'ctx> {
     use roc_mono::expr::Expr::*;
@@ -193,12 +193,7 @@ pub fn build_expr<'a, 'ctx, 'env>(
                 .left()
                 .unwrap_or_else(|| panic!("LLVM error: Invalid call by pointer."))
         }
-        Load(symbol) => match scope.get(symbol) {
+        Load(symbol) => load_symbol(env, scope, symbol),
-            Some((_, ptr)) => env
-                .builder
-                .build_load(*ptr, symbol.ident_string(&env.interns)),
-            None => panic!("Could not find a var for {:?} in scope {:?}", symbol, scope),
-        },
         Str(str_literal) => {
             if str_literal.is_empty() {
                 panic!("TODO build an empty string in LLVM");
@@ -514,6 +509,19 @@ pub fn build_expr<'a, 'ctx, 'env>(
     }
 }
 
+fn load_symbol<'a, 'ctx, 'env>(
+    env: &Env<'a, 'ctx, 'env>,
+    scope: &Scope<'a, 'ctx>,
+    symbol: &Symbol,
+) -> BasicValueEnum<'ctx> {
+    match scope.get(symbol) {
+        Some((_, ptr)) => env
+            .builder
+            .build_load(*ptr, symbol.ident_string(&env.interns)),
+        None => panic!("Could not find a var for {:?} in scope {:?}", symbol, scope),
+    }
+}
+
 /// Cast a struct to another struct of the same (or smaller?) size
 fn cast_struct_struct<'ctx>(
     builder: &Builder<'ctx>,
@@ -563,7 +571,7 @@ fn extract_tag_discriminant<'a, 'ctx, 'env>(
 }
 
 struct Branch2<'a> {
-    cond: &'a Expr<'a>,
+    cond: &'a Symbol,
     pass: &'a Expr<'a>,
     fail: &'a Expr<'a>,
     ret_layout: Layout<'a>,
@@ -579,7 +587,7 @@ fn build_branch2<'a, 'ctx, 'env>(
     let ret_layout = cond.ret_layout;
     let ret_type = basic_type_from_layout(env.arena, env.context, &ret_layout, env.ptr_bytes);
 
-    let cond_expr = build_expr(env, scope, parent, cond.cond, procs);
+    let cond_expr = load_symbol(env, scope, cond.cond);
 
     match cond_expr {
         IntValue(value) => {

compiler/mono/src/decision_tree.rs

@@ -1105,16 +1105,32 @@ fn decide_to_branching<'a>(
                 jumps,
             ));
 
-            let cond = boolean_all(env.arena, tests);
+            let condition = boolean_all(env.arena, tests);
 
             let cond_layout = Layout::Builtin(Builtin::Bool);
 
-            Expr::Cond {
+            if let Expr::Load(symbol) = condition {
-                cond: env.arena.alloc(cond),
+                Expr::Cond {
-                cond_layout,
+                    cond: symbol,
-                pass,
+                    cond_layout,
-                fail,
+                    pass,
-                ret_layout,
+                    fail,
+                    ret_layout,
+                }
+            } else {
+                let cond_symbol = env.fresh_symbol();
+                let stores = vec![(cond_symbol, cond_layout.clone(), condition)];
+
+                Expr::Store(
+                    env.arena.alloc(stores),
+                    env.arena.alloc(Expr::Cond {
+                        cond: cond_symbol,
+                        cond_layout,
+                        pass,
+                        fail,
+                        ret_layout,
+                    }),
+                )
             }
         }
         FanOut {

compiler/mono/src/expr.rs

@@ -152,7 +152,7 @@ pub enum Expr<'a> {
         // The left-hand side of the conditional comparison and the right-hand side.
         // These are stored separately because there are different machine instructions
         // for e.g. "compare float and jump" vs. "compare integer and jump"
-        cond: &'a Expr<'a>,
+        cond: Symbol,
         cond_layout: Layout<'a>,
         // What to do if the condition either passes or fails
         pass: &'a Expr<'a>,
@@ -646,13 +646,22 @@ fn from_can<'a>(
             for (loc_cond, loc_then) in branches.into_iter().rev() {
                 let cond = from_can(env, loc_cond.value, procs, None);
                 let then = from_can(env, loc_then.value, procs, None);
-                expr = Expr::Cond {
+
-                    cond: env.arena.alloc(cond),
+                let cond_symbol = env.fresh_symbol();
+
+                let cond_expr = Expr::Cond {
+                    cond: cond_symbol,
                     cond_layout: cond_layout.clone(),
                     pass: env.arena.alloc(then),
                     fail: env.arena.alloc(expr),
                     ret_layout: ret_layout.clone(),
                 };
+
+                expr = Expr::Store(
+                    env.arena
+                        .alloc(vec![(cond_symbol, Layout::Builtin(Builtin::Bool), cond)]),
+                    env.arena.alloc(cond_expr),
+                );
             }
 
             expr

compiler/mono/tests/test_mono.rs

@@ -16,6 +16,7 @@ mod test_mono {
     use roc_module::symbol::{Interns, Symbol};
     use roc_mono::expr::Expr::{self, *};
     use roc_mono::expr::Procs;
+    use roc_mono::layout;
     use roc_mono::layout::{Builtin, Layout};
     use roc_types::subs::Subs;
 
@@ -160,13 +161,23 @@ mod test_mono {
                 use self::Builtin::*;
                 use Layout::Builtin;
 
-                Cond {
+                let home = test_home();
-                    cond: &Expr::Bool(true),
+                let gen_symbol_0 = Interns::from_index(home, 0);
-                    cond_layout: Builtin(Bool),
+
-                    pass: &Expr::Str("bar"),
+                Store(
-                    fail: &Expr::Str("foo"),
+                    &[(
-                    ret_layout: Builtin(Str),
+                        gen_symbol_0,
-                }
+                        Layout::Builtin(layout::Builtin::Bool),
+                        Expr::Bool(true),
+                    )],
+                    &Cond {
+                        cond: gen_symbol_0,
+                        cond_layout: Builtin(Bool),
+                        pass: &Expr::Str("bar"),
+                        fail: &Expr::Str("foo"),
+                        ret_layout: Builtin(Str),
+                    },
+                )
             },
         )
     }
@@ -186,19 +197,37 @@ mod test_mono {
                 use self::Builtin::*;
                 use Layout::Builtin;
 
-                Cond {
+                let home = test_home();
-                    cond: &Expr::Bool(true),
+                let gen_symbol_0 = Interns::from_index(home, 1);
-                    cond_layout: Builtin(Bool),
+                let gen_symbol_1 = Interns::from_index(home, 0);
-                    pass: &Expr::Str("bar"),
+
-                    fail: &Cond {
+                Store(
-                        cond: &Expr::Bool(false),
+                    &[(
+                        gen_symbol_0,
+                        Layout::Builtin(layout::Builtin::Bool),
+                        Expr::Bool(true),
+                    )],
+                    &Cond {
+                        cond: gen_symbol_0,
                         cond_layout: Builtin(Bool),
-                        pass: &Expr::Str("foo"),
+                        pass: &Expr::Str("bar"),
-                        fail: &Expr::Str("baz"),
+                        fail: &Store(
+                            &[(
+                                gen_symbol_1,
+                                Layout::Builtin(layout::Builtin::Bool),
+                                Expr::Bool(false),
+                            )],
+                            &Cond {
+                                cond: gen_symbol_1,
+                                cond_layout: Builtin(Bool),
+                                pass: &Expr::Str("foo"),
+                                fail: &Expr::Str("baz"),
+                                ret_layout: Builtin(Str),
+                            },
+                        ),
                         ret_layout: Builtin(Str),
                     },
-                    ret_layout: Builtin(Str),
+                )
-                }
             },
         )
     }
@@ -218,19 +247,27 @@ mod test_mono {
                 use Layout::Builtin;
 
                 let home = test_home();
+                let gen_symbol_0 = Interns::from_index(home, 1);
                 let symbol_x = Interns::from_index(home, 0);
 
                 Store(
                     &[(
                         symbol_x,
                         Builtin(Str),
-                        Cond {
+                        Store(
-                            cond: &Expr::Bool(true),
+                            &[(
-                            cond_layout: Builtin(Bool),
+                                gen_symbol_0,
-                            pass: &Expr::Str("bar"),
+                                Layout::Builtin(layout::Builtin::Bool),
-                            fail: &Expr::Str("foo"),
+                                Expr::Bool(true),
-                            ret_layout: Builtin(Str),
+                            )],
-                        },
+                            &Cond {
+                                cond: gen_symbol_0,
+                                cond_layout: Builtin(Bool),
+                                pass: &Expr::Str("bar"),
+                                fail: &Expr::Str("foo"),
+                                ret_layout: Builtin(Str),
+                            },
+                        ),
                     )],
                     &Load(symbol_x),
                 )

compiler/mono/tests/test_opt.rs

@@ -114,13 +114,12 @@ mod test_opt {
             }
 
             Cond {
-                cond,
+                cond: _,
                 cond_layout: _,
                 pass,
                 fail,
                 ret_layout: _,
             } => {
-                extract_named_calls_help(cond, calls, unexpected_calls);
                 extract_named_calls_help(pass, calls, unexpected_calls);
                 extract_named_calls_help(fail, calls, unexpected_calls);
             }