diff --git a/compiler/gen/src/llvm/build.rs b/compiler/gen/src/llvm/build.rs index 31420b47a7..e82d621c97 100644 --- a/compiler/gen/src/llvm/build.rs +++ b/compiler/gen/src/llvm/build.rs @@ -395,7 +395,6 @@ pub fn build_exp_expr<'a, 'ctx, 'env>( } } - dbg!(&field_vals); // If the struct has only one field that isn't zero-sized, // unwrap it. This is what the layout expects us to do. if field_vals.len() == 1 { diff --git a/compiler/gen/tests/gen_num.rs b/compiler/gen/tests/gen_num.rs index cc694e3984..bc46d40a29 100644 --- a/compiler/gen/tests/gen_num.rs +++ b/compiler/gen/tests/gen_num.rs @@ -480,7 +480,7 @@ mod gen_num { } #[test] - fn if_guard_bind_variable() { + fn if_guard_bind_variable_false() { assert_evals_to!( indoc!( r#" @@ -492,7 +492,10 @@ mod gen_num { 42, i64 ); + } + #[test] + fn if_guard_bind_variable_true() { assert_evals_to!( indoc!( r#" diff --git a/compiler/mono/src/decision_tree.rs b/compiler/mono/src/decision_tree.rs index 263cd05d3f..561bfe3cc0 100644 --- a/compiler/mono/src/decision_tree.rs +++ b/compiler/mono/src/decision_tree.rs @@ -1,6 +1,6 @@ use crate::exhaustive::{Ctor, RenderAs, TagId, Union}; -use crate::ir::{DestructType, Env, Expr, JoinPointId, Literal, Pattern, Stmt}; -use crate::layout::{Builtin, Layout}; +use crate::ir::{DestructType, Env, Expr, JoinPointId, Literal, Pattern, Procs, Stmt}; +use crate::layout::{Builtin, Layout, LayoutCache}; use roc_collections::all::{MutMap, MutSet}; use roc_module::ident::TagName; use roc_module::low_level::LowLevel; @@ -882,6 +882,8 @@ type StoresVec<'a> = bumpalo::collections::Vec<'a, (Symbol, Layout<'a>, Expr<'a> pub fn optimize_when<'a>( env: &mut Env<'a, '_>, + procs: &mut Procs<'a>, + layout_cache: &mut LayoutCache<'a>, cond_symbol: Symbol, cond_layout: Layout<'a>, ret_layout: Layout<'a>, @@ -918,6 +920,8 @@ pub fn optimize_when<'a>( let expr = decide_to_branching( env, + procs, + layout_cache, cond_symbol, cond_layout, ret_layout, @@ -1125,8 +1129,14 @@ fn test_to_equality<'a>( } } +// TODO procs and layout are currently unused, but potentially required +// for defining optional fields? +// if not, do remove +#[allow(clippy::too_many_arguments)] fn decide_to_branching<'a>( env: &mut Env<'a, '_>, + procs: &mut Procs<'a>, + layout_cache: &mut LayoutCache<'a>, cond_symbol: Symbol, cond_layout: Layout<'a>, ret_layout: Layout<'a>, @@ -1155,6 +1165,8 @@ fn decide_to_branching<'a>( let pass_expr = decide_to_branching( env, + procs, + layout_cache, cond_symbol, cond_layout.clone(), ret_layout.clone(), @@ -1164,6 +1176,8 @@ fn decide_to_branching<'a>( let fail_expr = decide_to_branching( env, + procs, + layout_cache, cond_symbol, cond_layout.clone(), ret_layout.clone(), @@ -1264,6 +1278,8 @@ fn decide_to_branching<'a>( continuation: env.arena.alloc(cond), }; + // load all the variables (the guard might need them); + current_symbol = accum; } @@ -1323,6 +1339,8 @@ fn decide_to_branching<'a>( let default_branch = decide_to_branching( env, + procs, + layout_cache, cond_symbol, cond_layout.clone(), ret_layout.clone(), @@ -1335,6 +1353,8 @@ fn decide_to_branching<'a>( for (test, decider) in tests { let branch = decide_to_branching( env, + procs, + layout_cache, cond_symbol, cond_layout.clone(), ret_layout.clone(), diff --git a/compiler/mono/src/ir.rs b/compiler/mono/src/ir.rs index af62f5104e..25ecec14d9 100644 --- a/compiler/mono/src/ir.rs +++ b/compiler/mono/src/ir.rs @@ -1007,16 +1007,15 @@ pub fn with_hole<'a>( // WRONG! this is introduces new control flow, and should call `from_can` again if let roc_can::pattern::Pattern::Identifier(symbol) = def.loc_pattern.value { let stmt = with_hole(env, cont.value, procs, layout_cache, assigned, hole); - let expr = with_hole( + + with_hole( env, def.loc_expr.value, procs, layout_cache, symbol, env.arena.alloc(stmt), - ); - - expr + ) } else { todo!() } @@ -1782,17 +1781,20 @@ pub fn from_can<'a>( .from_var(env.arena, def.expr_var, env.subs, env.pointer_size) .expect("invalid layout"); - let mut stores = Vec::new_in(env.arena); - let outer_symbol = env.unique_symbol(); - store_pattern(env, &mono_pattern, outer_symbol, layout, &mut stores).unwrap(); - // convert the continuation let mut stmt = from_can(env, cont.value, procs, layout_cache); - // unpack the body of the def based on the pattern - for (symbol, layout, expr) in stores.iter().rev() { - stmt = Stmt::Let(*symbol, expr.clone(), layout.clone(), env.arena.alloc(stmt)); - } + let outer_symbol = env.unique_symbol(); + stmt = store_pattern( + env, + procs, + layout_cache, + &mono_pattern, + outer_symbol, + layout, + stmt, + ) + .unwrap(); // convert the def body, store in outer_symbol with_hole( @@ -1897,6 +1899,7 @@ fn to_opt_branches<'a>( opt_branches } +#[allow(clippy::too_many_arguments)] fn from_can_when<'a>( env: &mut Env<'a, '_>, cond_var: Variable, @@ -1927,10 +1930,7 @@ fn from_can_when<'a>( let it = opt_branches .into_iter() .map(|(pattern, opt_guard, can_expr)| { - let mut stores = Vec::with_capacity_in(1, env.arena); - let res_stores = - store_pattern(env, &pattern, cond_symbol, cond_layout.clone(), &mut stores); - let mut stmt = match join_point { + let branch_stmt = match join_point { None => from_can(env, can_expr, procs, layout_cache), Some(id) => { let symbol = env.unique_symbol(); @@ -1942,44 +1942,62 @@ fn from_can_when<'a>( }; use crate::decision_tree::Guard; - match res_stores { - Ok(_) => { - for (symbol, layout, expr) in stores.iter().rev() { - stmt = - Stmt::Let(*symbol, expr.clone(), layout.clone(), env.arena.alloc(stmt)); - } + if let Some(loc_expr) = opt_guard { + let id = JoinPointId(env.unique_symbol()); + let symbol = env.unique_symbol(); + let jump = env.arena.alloc(Stmt::Jump(id, env.arena.alloc([symbol]))); - let guard = if let Some(loc_expr) = opt_guard { - let id = JoinPointId(env.unique_symbol()); - let symbol = env.unique_symbol(); - let jump = env.arena.alloc(Stmt::Jump(id, env.arena.alloc([symbol]))); + let guard_stmt = with_hole(env, loc_expr.value, procs, layout_cache, symbol, jump); - let mut stmt = - with_hole(env, loc_expr.value, procs, layout_cache, symbol, jump); - - // guard must have access to bound values - for (symbol, layout, expr) in stores.into_iter().rev() { - stmt = Stmt::Let(symbol, expr, layout, env.arena.alloc(stmt)); - } - - Guard::Guard { id, symbol, stmt } - } else { - Guard::NoGuard - }; - - (pattern, guard, stmt) + match store_pattern( + env, + procs, + layout_cache, + &pattern, + cond_symbol, + cond_layout.clone(), + guard_stmt, + ) { + Ok(new_guard_stmt) => ( + pattern, + Guard::Guard { + id, + symbol, + stmt: new_guard_stmt, + }, + branch_stmt, + ), + Err(msg) => ( + Pattern::Underscore, + Guard::NoGuard, + Stmt::RuntimeError(env.arena.alloc(msg)), + ), + } + } else { + match store_pattern( + env, + procs, + layout_cache, + &pattern, + cond_symbol, + cond_layout.clone(), + branch_stmt, + ) { + Ok(new_branch_stmt) => (pattern, Guard::NoGuard, new_branch_stmt), + Err(msg) => ( + Pattern::Underscore, + Guard::NoGuard, + Stmt::RuntimeError(env.arena.alloc(msg)), + ), } - Err(msg) => ( - Pattern::Underscore, - Guard::NoGuard, - Stmt::RuntimeError(env.arena.alloc(msg)), - ), } }); let mono_branches = Vec::from_iter_in(it, arena); crate::decision_tree::optimize_when( env, + procs, + layout_cache, cond_symbol, cond_layout.clone(), ret_layout, @@ -1987,25 +2005,31 @@ fn from_can_when<'a>( ) } +#[allow(clippy::too_many_arguments)] fn store_pattern<'a>( env: &mut Env<'a, '_>, + procs: &mut Procs<'a>, + layout_cache: &mut LayoutCache<'a>, can_pat: &Pattern<'a>, outer_symbol: Symbol, layout: Layout<'a>, - stored: &mut Vec<'a, (Symbol, Layout<'a>, Expr<'a>)>, -) -> Result<(), String> { + mut stmt: Stmt<'a>, +) -> Result, &'a str> { use Pattern::*; match can_pat { Identifier(symbol) => { - // TODO surely something should happen here? - stored.push((*symbol, layout, Expr::Alias(outer_symbol))); + let expr = Expr::Alias(outer_symbol); + stmt = Stmt::Let(*symbol, expr, layout, env.arena.alloc(stmt)); } Underscore => { - // Since _ is never read, it's safe to reassign it. - // stored.push((Symbol::UNDERSCORE, layout, Expr::Load(outer_symbol))) + // do nothing } - IntLiteral(_) | FloatLiteral(_) | EnumLiteral { .. } | BitLiteral { .. } => {} + IntLiteral(_) + | FloatLiteral(_) + | EnumLiteral { .. } + | BitLiteral { .. } + | StrLiteral(_) => {} AppliedTag { union, arguments, .. } => { @@ -2022,7 +2046,7 @@ fn store_pattern<'a>( arg_layouts.push(layout.clone()); } - for (index, (argument, arg_layout)) in arguments.iter().enumerate() { + for (index, (argument, arg_layout)) in arguments.iter().enumerate().rev() { let load = Expr::AccessAtIndex { is_unwrapped, index: (!is_unwrapped as usize + index) as u64, @@ -2032,57 +2056,79 @@ fn store_pattern<'a>( match argument { Identifier(symbol) => { // store immediately in the given symbol - stored.push((*symbol, arg_layout.clone(), load)); + stmt = Stmt::Let(*symbol, load, arg_layout.clone(), env.arena.alloc(stmt)); } Underscore => { // ignore } - IntLiteral(_) | FloatLiteral(_) | EnumLiteral { .. } | BitLiteral { .. } => {} + IntLiteral(_) + | FloatLiteral(_) + | EnumLiteral { .. } + | BitLiteral { .. } + | StrLiteral(_) => {} _ => { // store the field in a symbol, and continue matching on it let symbol = env.unique_symbol(); - stored.push((symbol, arg_layout.clone(), load)); - store_pattern(env, argument, symbol, arg_layout.clone(), stored)?; + // first recurse, continuing to unpack symbol + stmt = store_pattern( + env, + procs, + layout_cache, + argument, + symbol, + arg_layout.clone(), + stmt, + )?; + + // then store the symbol + stmt = Stmt::Let(symbol, load, arg_layout.clone(), env.arena.alloc(stmt)); } } } } RecordDestructure(destructs, Layout::Struct(sorted_fields)) => { - for (index, destruct) in destructs.iter().enumerate() { - store_record_destruct( + for (index, destruct) in destructs.iter().enumerate().rev() { + stmt = store_record_destruct( env, + procs, + layout_cache, destruct, index as u64, outer_symbol, sorted_fields, - stored, + stmt, )?; } } - Shadowed(region, ident) => { - return Err(format!( - "The pattern at {:?} shadows variable {:?}", - region, ident - )); + RecordDestructure(_, _) => { + unreachable!("a record destructure must always occur on a struct layout"); } - _ => { - panic!("TODO store_pattern for {:?}", can_pat); + + Shadowed(_region, _ident) => { + return Err(&"TODO"); + } + + UnsupportedPattern(_region) => { + return Err(&"TODO"); } } - Ok(()) + Ok(stmt) } +#[allow(clippy::too_many_arguments)] fn store_record_destruct<'a>( env: &mut Env<'a, '_>, + procs: &mut Procs<'a>, + layout_cache: &mut LayoutCache<'a>, destruct: &RecordDestruct<'a>, index: u64, outer_symbol: Symbol, sorted_fields: &'a [Layout<'a>], - stored: &mut Vec<'a, (Symbol, Layout<'a>, Expr<'a>)>, -) -> Result<(), String> { + mut stmt: Stmt<'a>, +) -> Result, &'a str> { use Pattern::*; let load = Expr::AccessAtIndex { @@ -2094,14 +2140,24 @@ fn store_record_destruct<'a>( match &destruct.typ { DestructType::Required => { - stored.push((destruct.symbol, destruct.layout.clone(), load)); + stmt = Stmt::Let( + destruct.symbol, + load, + destruct.layout.clone(), + env.arena.alloc(stmt), + ); } DestructType::Optional(_expr) => { todo!("TODO monomorphize optional field destructure's default expr"); } DestructType::Guard(guard_pattern) => match &guard_pattern { Identifier(symbol) => { - stored.push((*symbol, destruct.layout.clone(), load)); + stmt = Stmt::Let( + *symbol, + load, + destruct.layout.clone(), + env.arena.alloc(stmt), + ); } Underscore => { // important that this is special-cased to do nothing: mono record patterns will extract all the @@ -2116,19 +2172,34 @@ fn store_record_destruct<'a>( // // internally. But `y` is never used, so we must make sure it't not stored/loaded. } - IntLiteral(_) | FloatLiteral(_) | EnumLiteral { .. } | BitLiteral { .. } => {} + IntLiteral(_) + | FloatLiteral(_) + | EnumLiteral { .. } + | BitLiteral { .. } + | StrLiteral(_) => {} + _ => { let symbol = env.unique_symbol(); - stored.push((symbol, destruct.layout.clone(), load)); - store_pattern(env, guard_pattern, symbol, destruct.layout.clone(), stored)?; + stmt = store_pattern( + env, + procs, + layout_cache, + guard_pattern, + symbol, + destruct.layout.clone(), + stmt, + )?; + + stmt = Stmt::Let(symbol, load, destruct.layout.clone(), env.arena.alloc(stmt)); } }, } - Ok(()) + Ok(stmt) } +#[allow(clippy::too_many_arguments)] fn call_by_name<'a>( env: &mut Env<'a, '_>, procs: &mut Procs<'a>, diff --git a/compiler/mono/tests/test_mono.rs b/compiler/mono/tests/test_mono.rs index 026acb1673..2db9df0ef3 100644 --- a/compiler/mono/tests/test_mono.rs +++ b/compiler/mono/tests/test_mono.rs @@ -534,8 +534,8 @@ mod test_mono { indoc!( r#" procedure Num.14 (#Attr.2, #Attr.3): - let Test.8 = lowlevel NumAdd #Attr.2 #Attr.3; - ret Test.8; + let Test.7 = lowlevel NumAdd #Attr.2 #Attr.3; + ret Test.7; let Test.19 = 0i64; let Test.21 = 0i64; @@ -552,11 +552,11 @@ mod test_mono { let Test.17 = lowlevel Eq Test.14 Test.15; let Test.10 = lowlevel And Test.17 Test.16; if Test.10 then - let Test.5 = Index 1 Test.1; - let Test.2 = Index 1 Test.5; - let Test.7 = 1i64; - let Test.6 = CallByName Num.14 Test.2 Test.7; - jump Test.4 Test.6; + let Test.8 = Index 1 Test.1; + let Test.2 = Index 1 Test.8; + let Test.6 = 1i64; + let Test.5 = CallByName Num.14 Test.2 Test.6; + jump Test.4 Test.5; else let Test.9 = 1i64; jump Test.4 Test.9; @@ -760,11 +760,48 @@ mod test_mono { r#" let Test.4 = 2i64; let Test.5 = 3.14f64; - let Test.2 = Struct {Test.4, Test.5}; - let Test.0 = Index 0 Test.2; + let Test.3 = Struct {Test.4, Test.5}; + let Test.0 = Index 0 Test.3; ret Test.0; "# ), ) } + + #[test] + fn if_guard_bind_variable_false() { + compiles_to_ir( + indoc!( + r#" + when 10 is + x if x == 5 -> 0 + _ -> 42 + "# + ), + indoc!( + r#" + procedure Bool.5 (#Attr.2, #Attr.3): + let Test.8 = lowlevel Eq #Attr.2 #Attr.3; + ret Test.8; + + let Test.2 = 10i64; + let Test.11 = true; + let Test.0 = alias Test.2; + let Test.7 = 5i64; + let Test.6 = CallByName Bool.5 Test.0 Test.7; + jump Test.5 Test.6; + joinpoint Test.5 Test.12: + let Test.10 = lowlevel And Test.12 Test.11; + if Test.10 then + let Test.4 = 0i64; + jump Test.3 Test.4; + else + let Test.9 = 42i64; + jump Test.3 Test.9; + joinpoint Test.3 Test.1: + ret Test.1; + "# + ), + ) + } }