From 9c7514c4496b43c511e0798e9e79cabf0e27a7c2 Mon Sep 17 00:00:00 2001 From: Folkert Date: Mon, 23 Nov 2020 23:44:12 +0100 Subject: [PATCH 01/12] refactor record layout generation --- compiler/gen/tests/gen_records.rs | 45 +++++++++++++ compiler/mono/src/layout.rs | 105 ++++++++++++------------------ 2 files changed, 85 insertions(+), 65 deletions(-) diff --git a/compiler/gen/tests/gen_records.rs b/compiler/gen/tests/gen_records.rs index 54f651636b..fafc43fbfa 100644 --- a/compiler/gen/tests/gen_records.rs +++ b/compiler/gen/tests/gen_records.rs @@ -844,4 +844,49 @@ mod gen_records { (bool, bool) ); } + + #[test] + #[ignore] + 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 + ); + } } diff --git a/compiler/mono/src/layout.rs b/compiler/mono/src/layout.rs index ff3b38fbb9..41f0575d6d 100644 --- a/compiler/mono/src/layout.rs +++ b/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::() * 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()); - - sorted_fields.sort_by(|(label1, _), (label2, _)| label1.cmp(label2)); + let sorted_fields = sort_record_fields_help(env, fields_map); // 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 { - use LayoutProblem::*; - - 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) { + for (_, _, res_layout) in sorted_fields { + match res_layout { Ok(layout) => { // Drop any zero-sized fields like {}. if !layout.is_dropped_because_empty() { layouts.push(layout); } } - Err(UnresolvedTypeVar(v)) => { - // Invalid field! - panic!( - 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"); + Err(_) => { + // optional field, ignore + continue; } } } @@ -924,39 +896,42 @@ pub fn sort_record_fields<'a>( }; match roc_types::pretty_print::chase_ext_record(subs, var, &mut fields_map) { - Ok(()) | Err((_, Content::FlexVar(_))) => { - // 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 - } + Ok(()) | Err((_, Content::FlexVar(_))) => sort_record_fields_help(&mut env, fields_map), Err(other) => panic!("invalid content in record variable: {:?}", other), } } +fn sort_record_fields_help<'a>( + env: &mut Env<'a, '_>, + fields_map: MutMap>, +) -> Vec<'a, (Lowercase, Variable, Result, 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, _, _), (label2, _, _)| label1.cmp(label2)); + + sorted_fields +} + #[derive(Clone, Debug, PartialEq, Eq, Hash)] pub enum UnionVariant<'a> { Never, From d9e906b8fb1b2eb4960a96550f1ac39a885e3bff Mon Sep 17 00:00:00 2001 From: rvcas Date: Mon, 23 Nov 2020 19:39:30 -0500 Subject: [PATCH 02/12] feat(List): rename walkRight to walkBackwards --- compiler/builtins/src/std.rs | 2 +- compiler/builtins/src/unique.rs | 2 +- compiler/can/src/builtins.rs | 4 ++-- compiler/gen/src/llvm/build.rs | 4 ++-- compiler/gen/src/llvm/build_list.rs | 2 +- compiler/gen/tests/gen_list.rs | 16 ++++++++-------- compiler/module/src/symbol.rs | 2 +- compiler/solve/tests/solve_expr.rs | 8 ++++---- compiler/solve/tests/solve_uniq_expr.rs | 16 ++++++++-------- 9 files changed, 28 insertions(+), 28 deletions(-) diff --git a/compiler/builtins/src/std.rs b/compiler/builtins/src/std.rs index 83c6205ab7..fcea7197c2 100644 --- a/compiler/builtins/src/std.rs +++ b/compiler/builtins/src/std.rs @@ -491,7 +491,7 @@ pub fn types() -> MutMap { // walkRight : List elem, (elem -> accum -> accum), accum -> accum add_type( - Symbol::LIST_WALK_RIGHT, + Symbol::LIST_WALK_BACKWARDS, top_level_function( vec![ list_type(flex(TVAR1)), diff --git a/compiler/builtins/src/unique.rs b/compiler/builtins/src/unique.rs index 5ef68eff2a..e5a2e5b1ef 100644 --- a/compiler/builtins/src/unique.rs +++ b/compiler/builtins/src/unique.rs @@ -781,7 +781,7 @@ pub fn types() -> MutMap { // , Attr Shared (Attr u a -> b -> b) // , b // -> b - add_type(Symbol::LIST_WALK_RIGHT, { + add_type(Symbol::LIST_WALK_BACKWARDS, { let_tvars! { u, a, b, star1, closure }; unique_function( diff --git a/compiler/can/src/builtins.rs b/compiler/can/src/builtins.rs index 271e9a9d82..5e6e794381 100644 --- a/compiler/can/src/builtins.rs +++ b/compiler/can/src/builtins.rs @@ -71,7 +71,7 @@ pub fn builtin_defs(var_store: &mut VarStore) -> MutMap { Symbol::LIST_JOIN => list_join, Symbol::LIST_MAP => list_map, Symbol::LIST_KEEP_IF => list_keep_if, - Symbol::LIST_WALK_RIGHT => list_walk_right, + Symbol::LIST_WALK_BACKWARDS => list_walk_backwards, Symbol::NUM_ADD => num_add, Symbol::NUM_ADD_CHECKED => num_add_checked, Symbol::NUM_ADD_WRAP => num_add_wrap, @@ -1314,7 +1314,7 @@ fn list_join(symbol: Symbol, var_store: &mut VarStore) -> Def { } /// List.walkRight : List elem, (elem -> accum -> accum), accum -> accum -fn list_walk_right(symbol: Symbol, var_store: &mut VarStore) -> Def { +fn list_walk_backwards(symbol: Symbol, var_store: &mut VarStore) -> Def { let list_var = var_store.fresh(); let func_var = var_store.fresh(); let accum_var = var_store.fresh(); diff --git a/compiler/gen/src/llvm/build.rs b/compiler/gen/src/llvm/build.rs index dbd06bd64c..34e815237a 100644 --- a/compiler/gen/src/llvm/build.rs +++ b/compiler/gen/src/llvm/build.rs @@ -1,7 +1,7 @@ use crate::llvm::build_list::{ allocate_list, empty_list, empty_polymorphic_list, list_append, list_concat, list_contains, list_get_unsafe, list_join, list_keep_if, list_len, list_map, list_prepend, list_repeat, - list_reverse, list_set, list_single, list_sum, list_walk_right, + list_reverse, list_set, list_single, list_sum, list_walk_backwards, }; use crate::llvm::build_str::{ str_concat, str_count_graphemes, str_len, str_split, str_starts_with, CHAR_LAYOUT, @@ -2501,7 +2501,7 @@ fn run_low_level<'a, 'ctx, 'env>( let (default, default_layout) = load_symbol_and_layout(env, scope, &args[2]); - list_walk_right( + list_walk_backwards( env, parent, list, diff --git a/compiler/gen/src/llvm/build_list.rs b/compiler/gen/src/llvm/build_list.rs index 40837e4755..76db9297e3 100644 --- a/compiler/gen/src/llvm/build_list.rs +++ b/compiler/gen/src/llvm/build_list.rs @@ -811,7 +811,7 @@ pub fn list_sum<'a, 'ctx, 'env>( /// List.walkRight : List elem, (elem -> accum -> accum), accum -> accum #[allow(clippy::too_many_arguments)] -pub fn list_walk_right<'a, 'ctx, 'env>( +pub fn list_walk_backwards<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, parent: FunctionValue<'ctx>, list: BasicValueEnum<'ctx>, diff --git a/compiler/gen/tests/gen_list.rs b/compiler/gen/tests/gen_list.rs index 6562936abf..a155dad2b1 100644 --- a/compiler/gen/tests/gen_list.rs +++ b/compiler/gen/tests/gen_list.rs @@ -237,11 +237,11 @@ mod gen_list { } #[test] - fn list_walk_right_empty_all_inline() { + fn list_walk_backwards_empty_all_inline() { assert_evals_to!( indoc!( r#" - List.walkRight [0x1] (\a, b -> a + b) 0 + List.walkBackwards [0x1] (\a, b -> a + b) 0 "# ), 1, @@ -255,7 +255,7 @@ mod gen_list { empty = [] - List.walkRight empty (\a, b -> a + b) 0 + List.walkBackwards empty (\a, b -> a + b) 0 "# ), 0, @@ -264,22 +264,22 @@ mod gen_list { } #[test] - fn list_walk_right_with_str() { + fn list_walk_backwards_with_str() { assert_evals_to!( - r#"List.walkRight [ "x", "y", "z" ] Str.concat "<""#, + r#"List.walkBackwards [ "x", "y", "z" ] Str.concat "<""#, RocStr::from("zyx<"), RocStr ); assert_evals_to!( - r#"List.walkRight [ "Third", "Second", "First" ] Str.concat "Fourth""#, + r#"List.walkBackwards [ "Third", "Second", "First" ] Str.concat "Fourth""#, RocStr::from("FirstSecondThirdFourth"), RocStr ); } #[test] - fn list_walk_right_with_record() { + fn list_walk_backwards_with_record() { assert_evals_to!( indoc!( r#" @@ -295,7 +295,7 @@ mod gen_list { Zero -> { r & zeroes: r.zeroes + 1 } One -> { r & ones: r.ones + 1 } - finalCounts = List.walkRight byte acc initialCounts + finalCounts = List.walkBackwards byte acc initialCounts finalCounts.ones * 10 + finalCounts.zeroes "# diff --git a/compiler/module/src/symbol.rs b/compiler/module/src/symbol.rs index 820ce0a4c5..fd6e1f1a20 100644 --- a/compiler/module/src/symbol.rs +++ b/compiler/module/src/symbol.rs @@ -684,7 +684,7 @@ define_builtins! { 6 LIST_MAP: "map" 7 LIST_LEN: "len" 8 LIST_FOLDL: "foldl" - 9 LIST_WALK_RIGHT: "walkRight" + 9 LIST_WALK_BACKWARDS: "walkBackwards" 10 LIST_CONCAT: "concat" 11 LIST_FIRST: "first" 12 LIST_SINGLE: "single" diff --git a/compiler/solve/tests/solve_expr.rs b/compiler/solve/tests/solve_expr.rs index 060fd27e0f..08583957a6 100644 --- a/compiler/solve/tests/solve_expr.rs +++ b/compiler/solve/tests/solve_expr.rs @@ -2927,11 +2927,11 @@ mod solve_expr { } #[test] - fn list_walk_right() { + fn list_walk_backwards() { infer_eq_without_problem( indoc!( r#" - List.walkRight + List.walkBackwards "# ), "List a, (a, b -> b), b -> b", @@ -2939,7 +2939,7 @@ mod solve_expr { } #[test] - fn list_walk_right_example() { + fn list_walk_backwards_example() { infer_eq_without_problem( indoc!( r#" @@ -2947,7 +2947,7 @@ mod solve_expr { empty = [] - List.walkRight empty (\a, b -> a + b) 0 + List.walkBackwards empty (\a, b -> a + b) 0 "# ), "Int", diff --git a/compiler/solve/tests/solve_uniq_expr.rs b/compiler/solve/tests/solve_uniq_expr.rs index 3dcf4d4aae..76b6d4991b 100644 --- a/compiler/solve/tests/solve_uniq_expr.rs +++ b/compiler/solve/tests/solve_uniq_expr.rs @@ -2236,11 +2236,11 @@ mod solve_uniq_expr { } #[test] - fn list_walk_right_sum() { + fn list_walk_backwards_sum() { infer_eq( indoc!( r#" - sum = \list -> List.walkRight list Num.add 0 + sum = \list -> List.walkBackwards list Num.add 0 sum "# @@ -2321,11 +2321,11 @@ mod solve_uniq_expr { } #[test] - fn list_walk_right_reverse() { + fn list_walk_backwards_reverse() { infer_eq( indoc!( r#" - reverse = \list -> List.walkRight list (\e, l -> List.append l e) [] + reverse = \list -> List.walkBackwards list (\e, l -> List.append l e) [] reverse "# @@ -3133,11 +3133,11 @@ mod solve_uniq_expr { } #[test] - fn list_walk_right() { + fn list_walk_backwards() { infer_eq( indoc!( r#" - List.walkRight + List.walkBackwards "# ), "Attr * (Attr (* | b) (List (Attr b a)), Attr Shared (Attr b a, c -> c), c -> c)", @@ -3145,7 +3145,7 @@ mod solve_uniq_expr { } #[test] - fn list_walk_right_example() { + fn list_walk_backwards_example() { infer_eq( indoc!( r#" @@ -3153,7 +3153,7 @@ mod solve_uniq_expr { empty = [] - List.walkRight empty (\a, b -> a + b) 0 + List.walkBackwards empty (\a, b -> a + b) 0 "# ), "Attr a Int", From 8feab843eaf24ff3acdcd730f4f792eaff30bdd3 Mon Sep 17 00:00:00 2001 From: rvcas Date: Tue, 24 Nov 2020 09:01:03 -0500 Subject: [PATCH 03/12] feat(List): add walk function and fix walkBackwards --- compiler/builtins/src/std.rs | 15 ++++- compiler/builtins/src/unique.rs | 29 ++++++++- compiler/can/src/builtins.rs | 34 +++++++++- compiler/gen/src/llvm/build.rs | 26 +++++++- compiler/gen/src/llvm/build_list.rs | 98 ++++++++++++++++++++++++++++- compiler/gen/tests/gen_list.rs | 4 +- compiler/module/src/low_level.rs | 3 +- compiler/module/src/symbol.rs | 24 +++---- compiler/mono/src/borrow.rs | 3 +- 9 files changed, 210 insertions(+), 26 deletions(-) diff --git a/compiler/builtins/src/std.rs b/compiler/builtins/src/std.rs index fcea7197c2..9a38e3518d 100644 --- a/compiler/builtins/src/std.rs +++ b/compiler/builtins/src/std.rs @@ -489,7 +489,20 @@ pub fn types() -> MutMap { ), ); - // walkRight : List elem, (elem -> accum -> accum), accum -> accum + // walk : List elem, (elem -> accum -> accum), accum -> accum + add_type( + Symbol::LIST_WALK, + top_level_function( + vec![ + list_type(flex(TVAR1)), + closure(vec![flex(TVAR1), flex(TVAR2)], TVAR3, Box::new(flex(TVAR2))), + flex(TVAR2), + ], + Box::new(flex(TVAR2)), + ), + ); + + // walkBackwards : List elem, (elem -> accum -> accum), accum -> accum add_type( Symbol::LIST_WALK_BACKWARDS, top_level_function( diff --git a/compiler/builtins/src/unique.rs b/compiler/builtins/src/unique.rs index e5a2e5b1ef..463e22ee07 100644 --- a/compiler/builtins/src/unique.rs +++ b/compiler/builtins/src/unique.rs @@ -777,7 +777,34 @@ pub fn types() -> MutMap { ) }); - // walkRight : Attr (* | u) (List (Attr u a)) + // walk : Attr (* | u) (List (Attr u a)) + // , Attr Shared (Attr u a -> b -> b) + // , b + // -> b + add_type(Symbol::LIST_WALK, { + let_tvars! { u, a, b, star1, closure }; + + unique_function( + vec![ + SolvedType::Apply( + Symbol::ATTR_ATTR, + vec![ + container(star1, vec![u]), + SolvedType::Apply(Symbol::LIST_LIST, vec![attr_type(u, a)]), + ], + ), + shared(SolvedType::Func( + vec![attr_type(u, a), flex(b)], + Box::new(flex(closure)), + Box::new(flex(b)), + )), + flex(b), + ], + flex(b), + ) + }); + + // walkBackwards : Attr (* | u) (List (Attr u a)) // , Attr Shared (Attr u a -> b -> b) // , b // -> b diff --git a/compiler/can/src/builtins.rs b/compiler/can/src/builtins.rs index 5e6e794381..fda8b43831 100644 --- a/compiler/can/src/builtins.rs +++ b/compiler/can/src/builtins.rs @@ -71,6 +71,7 @@ pub fn builtin_defs(var_store: &mut VarStore) -> MutMap { Symbol::LIST_JOIN => list_join, Symbol::LIST_MAP => list_map, Symbol::LIST_KEEP_IF => list_keep_if, + Symbol::LIST_WALK => list_walk, Symbol::LIST_WALK_BACKWARDS => list_walk_backwards, Symbol::NUM_ADD => num_add, Symbol::NUM_ADD_CHECKED => num_add_checked, @@ -1313,14 +1314,43 @@ fn list_join(symbol: Symbol, var_store: &mut VarStore) -> Def { ) } -/// List.walkRight : List elem, (elem -> accum -> accum), accum -> accum +/// List.walk : List elem, (elem -> accum -> accum), accum -> accum +fn list_walk(symbol: Symbol, var_store: &mut VarStore) -> Def { + let list_var = var_store.fresh(); + let func_var = var_store.fresh(); + let accum_var = var_store.fresh(); + + let body = RunLowLevel { + op: LowLevel::ListWalk, + args: vec![ + (list_var, Var(Symbol::ARG_1)), + (func_var, Var(Symbol::ARG_2)), + (accum_var, Var(Symbol::ARG_3)), + ], + ret_var: accum_var, + }; + + defn( + symbol, + vec![ + (list_var, Symbol::ARG_1), + (func_var, Symbol::ARG_2), + (accum_var, Symbol::ARG_3), + ], + var_store, + body, + accum_var, + ) +} + +/// List.walkBackwards : List elem, (elem -> accum -> accum), accum -> accum fn list_walk_backwards(symbol: Symbol, var_store: &mut VarStore) -> Def { let list_var = var_store.fresh(); let func_var = var_store.fresh(); let accum_var = var_store.fresh(); let body = RunLowLevel { - op: LowLevel::ListWalkRight, + op: LowLevel::ListWalkBackwards, args: vec![ (list_var, Var(Symbol::ARG_1)), (func_var, Var(Symbol::ARG_2)), diff --git a/compiler/gen/src/llvm/build.rs b/compiler/gen/src/llvm/build.rs index 34e815237a..65a7926c36 100644 --- a/compiler/gen/src/llvm/build.rs +++ b/compiler/gen/src/llvm/build.rs @@ -1,7 +1,7 @@ use crate::llvm::build_list::{ allocate_list, empty_list, empty_polymorphic_list, list_append, list_concat, list_contains, list_get_unsafe, list_join, list_keep_if, list_len, list_map, list_prepend, list_repeat, - list_reverse, list_set, list_single, list_sum, list_walk_backwards, + list_reverse, list_set, list_single, list_sum, list_walk, list_walk_backwards, }; use crate::llvm::build_str::{ str_concat, str_count_graphemes, str_len, str_split, str_starts_with, CHAR_LAYOUT, @@ -2491,8 +2491,28 @@ fn run_low_level<'a, 'ctx, 'env>( list_contains(env, parent, elem, elem_layout, list, list_layout) } - ListWalkRight => { - // List.walkRight : List elem, (elem -> accum -> accum), accum -> accum + ListWalk => { + debug_assert_eq!(args.len(), 3); + + let (list, list_layout) = load_symbol_and_layout(env, scope, &args[0]); + + let (func, func_layout) = load_symbol_and_layout(env, scope, &args[1]); + + let (default, default_layout) = load_symbol_and_layout(env, scope, &args[2]); + + list_walk( + env, + parent, + list, + list_layout, + func, + func_layout, + default, + default_layout, + ) + } + ListWalkBackwards => { + // List.walkBackwards : List elem, (elem -> accum -> accum), accum -> accum debug_assert_eq!(args.len(), 3); let (list, list_layout) = load_symbol_and_layout(env, scope, &args[0]); diff --git a/compiler/gen/src/llvm/build_list.rs b/compiler/gen/src/llvm/build_list.rs index 76db9297e3..b3fc8a564b 100644 --- a/compiler/gen/src/llvm/build_list.rs +++ b/compiler/gen/src/llvm/build_list.rs @@ -809,7 +809,98 @@ pub fn list_sum<'a, 'ctx, 'env>( builder.build_load(accum_alloca, "load_final_acum") } -/// List.walkRight : List elem, (elem -> accum -> accum), accum -> accum +/// List.walk : List elem, (elem -> accum -> accum), accum -> accum +pub fn list_walk<'a, 'ctx, 'env>( + env: &Env<'a, 'ctx, 'env>, + parent: FunctionValue<'ctx>, + list: BasicValueEnum<'ctx>, + list_layout: &Layout<'a>, + func: BasicValueEnum<'ctx>, + func_layout: &Layout<'a>, + default: BasicValueEnum<'ctx>, + default_layout: &Layout<'a>, +) -> BasicValueEnum<'ctx> { + let ctx = env.context; + let builder = env.builder; + + let list_wrapper = list.into_struct_value(); + let len = list_len(env.builder, list_wrapper); + + let accum_type = basic_type_from_layout(env.arena, ctx, default_layout, env.ptr_bytes); + let accum_alloca = builder.build_alloca(accum_type, "alloca_walk_right_accum"); + builder.build_store(accum_alloca, default); + + let then_block = ctx.append_basic_block(parent, "then"); + let cont_block = ctx.append_basic_block(parent, "branchcont"); + + let condition = builder.build_int_compare( + IntPredicate::UGT, + len, + ctx.i64_type().const_zero(), + "list_non_empty", + ); + + builder.build_conditional_branch(condition, then_block, cont_block); + + builder.position_at_end(then_block); + + match (func, func_layout) { + (BasicValueEnum::PointerValue(func_ptr), Layout::FunctionPointer(_, _)) => { + let elem_layout = match list_layout { + Layout::Builtin(Builtin::List(_, layout)) => layout, + _ => unreachable!("can only fold over a list"), + }; + + let elem_type = basic_type_from_layout(env.arena, ctx, elem_layout, env.ptr_bytes); + let elem_ptr_type = get_ptr_type(&elem_type, AddressSpace::Generic); + + let list_ptr = load_list_ptr(builder, list_wrapper, elem_ptr_type); + + let walk_right_loop = |_, elem: BasicValueEnum<'ctx>| { + // load current accumulator + let current = builder.build_load(accum_alloca, "retrieve_accum"); + + let call_site_value = + builder.build_call(func_ptr, &[elem, current], "#walk_right_func"); + + // set the calling convention explicitly for this call + call_site_value.set_call_convention(crate::llvm::build::FAST_CALL_CONV); + + let new_current = call_site_value + .try_as_basic_value() + .left() + .unwrap_or_else(|| panic!("LLVM error: Invalid call by pointer.")); + + builder.build_store(accum_alloca, new_current); + }; + + incrementing_elem_loop( + builder, + ctx, + parent, + list_ptr, + len, + "#index", + walk_right_loop, + ); + } + + _ => { + unreachable!( + "Invalid function basic value enum or layout for List.keepIf : {:?}", + (func, func_layout) + ); + } + } + + builder.build_unconditional_branch(cont_block); + + builder.position_at_end(cont_block); + + builder.build_load(accum_alloca, "load_final_acum") +} + +/// List.walkBackwards : List elem, (elem -> accum -> accum), accum -> accum #[allow(clippy::too_many_arguments)] pub fn list_walk_backwards<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, @@ -875,7 +966,7 @@ pub fn list_walk_backwards<'a, 'ctx, 'env>( builder.build_store(accum_alloca, new_current); }; - incrementing_elem_loop( + decrementing_elem_loop( builder, ctx, parent, @@ -1537,6 +1628,7 @@ where let current_index = builder .build_load(index_alloca, index_name) .into_int_value(); + let next_index = builder.build_int_sub(current_index, one, "nextindex"); builder.build_store(index_alloca, next_index); @@ -1546,7 +1638,7 @@ where // #index >= 0 let condition = builder.build_int_compare( - IntPredicate::UGE, + IntPredicate::SGE, next_index, ctx.i64_type().const_zero(), "bounds_check", diff --git a/compiler/gen/tests/gen_list.rs b/compiler/gen/tests/gen_list.rs index a155dad2b1..2f5692169c 100644 --- a/compiler/gen/tests/gen_list.rs +++ b/compiler/gen/tests/gen_list.rs @@ -267,13 +267,13 @@ mod gen_list { fn list_walk_backwards_with_str() { assert_evals_to!( r#"List.walkBackwards [ "x", "y", "z" ] Str.concat "<""#, - RocStr::from("zyx<"), + RocStr::from("xyz<"), RocStr ); assert_evals_to!( r#"List.walkBackwards [ "Third", "Second", "First" ] Str.concat "Fourth""#, - RocStr::from("FirstSecondThirdFourth"), + RocStr::from("ThirdSecondFirstFourth"), RocStr ); } diff --git a/compiler/module/src/low_level.rs b/compiler/module/src/low_level.rs index 5c9a90f1ae..f2d03586b9 100644 --- a/compiler/module/src/low_level.rs +++ b/compiler/module/src/low_level.rs @@ -22,7 +22,8 @@ pub enum LowLevel { ListJoin, ListMap, ListKeepIf, - ListWalkRight, + ListWalk, + ListWalkBackwards, ListSum, NumAdd, NumAddWrap, diff --git a/compiler/module/src/symbol.rs b/compiler/module/src/symbol.rs index fd6e1f1a20..fac0c0a100 100644 --- a/compiler/module/src/symbol.rs +++ b/compiler/module/src/symbol.rs @@ -683,18 +683,18 @@ define_builtins! { 5 LIST_APPEND: "append" 6 LIST_MAP: "map" 7 LIST_LEN: "len" - 8 LIST_FOLDL: "foldl" - 9 LIST_WALK_BACKWARDS: "walkBackwards" - 10 LIST_CONCAT: "concat" - 11 LIST_FIRST: "first" - 12 LIST_SINGLE: "single" - 13 LIST_REPEAT: "repeat" - 14 LIST_REVERSE: "reverse" - 15 LIST_PREPEND: "prepend" - 16 LIST_JOIN: "join" - 17 LIST_KEEP_IF: "keepIf" - 18 LIST_CONTAINS: "contains" - 19 LIST_SUM: "sum" + 8 LIST_WALK_BACKWARDS: "walkBackwards" + 9 LIST_CONCAT: "concat" + 10 LIST_FIRST: "first" + 11 LIST_SINGLE: "single" + 12 LIST_REPEAT: "repeat" + 13 LIST_REVERSE: "reverse" + 14 LIST_PREPEND: "prepend" + 15 LIST_JOIN: "join" + 16 LIST_KEEP_IF: "keepIf" + 17 LIST_CONTAINS: "contains" + 18 LIST_SUM: "sum" + 19 LIST_WALK: "walk" } 5 RESULT: "Result" => { 0 RESULT_RESULT: "Result" imported // the Result.Result type alias diff --git a/compiler/mono/src/borrow.rs b/compiler/mono/src/borrow.rs index 82789d81e0..b6fd077d54 100644 --- a/compiler/mono/src/borrow.rs +++ b/compiler/mono/src/borrow.rs @@ -535,7 +535,8 @@ pub fn lowlevel_borrow_signature(arena: &Bump, op: LowLevel) -> &[bool] { ListMap => arena.alloc_slice_copy(&[owned, irrelevant]), ListKeepIf => arena.alloc_slice_copy(&[owned, irrelevant]), ListContains => arena.alloc_slice_copy(&[borrowed, irrelevant]), - ListWalkRight => arena.alloc_slice_copy(&[borrowed, irrelevant, owned]), + ListWalk => arena.alloc_slice_copy(&[borrowed, irrelevant, owned]), + ListWalkBackwards => arena.alloc_slice_copy(&[borrowed, irrelevant, owned]), ListSum => arena.alloc_slice_copy(&[borrowed]), Eq | NotEq | And | Or | NumAdd | NumAddWrap | NumAddChecked | NumSub | NumMul | NumGt From 0fb2c4ff8b666045906e600356ee2b15927d2d95 Mon Sep 17 00:00:00 2001 From: rvcas Date: Tue, 24 Nov 2020 13:20:05 -0500 Subject: [PATCH 04/12] fix(List): clippy on list_walk and tests for the walk builtin --- compiler/gen/src/llvm/build_list.rs | 1 + compiler/gen/tests/gen_list.rs | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/compiler/gen/src/llvm/build_list.rs b/compiler/gen/src/llvm/build_list.rs index b3fc8a564b..c79a1d64b5 100644 --- a/compiler/gen/src/llvm/build_list.rs +++ b/compiler/gen/src/llvm/build_list.rs @@ -810,6 +810,7 @@ pub fn list_sum<'a, 'ctx, 'env>( } /// List.walk : List elem, (elem -> accum -> accum), accum -> accum +#[allow(clippy::too_many_arguments)] pub fn list_walk<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, parent: FunctionValue<'ctx>, diff --git a/compiler/gen/tests/gen_list.rs b/compiler/gen/tests/gen_list.rs index 432ac501a1..2f26f18045 100644 --- a/compiler/gen/tests/gen_list.rs +++ b/compiler/gen/tests/gen_list.rs @@ -305,6 +305,26 @@ mod gen_list { ); } + #[test] + fn list_walk_with_str() { + assert_evals_to!( + r#"List.walk [ "x", "y", "z" ] Str.concat "<""#, + RocStr::from("zyx<"), + RocStr + ); + + assert_evals_to!( + r#"List.walk [ "Third", "Second", "First" ] Str.concat "Fourth""#, + RocStr::from("FirstSecondThirdFourth"), + RocStr + ); + } + + #[test] + fn list_walk_substraction() { + assert_evals_to!(r#"List.walk [ 1, 2 ] Num.sub 1"#, 2, i64); + } + #[test] fn list_keep_if_empty_list_of_int() { assert_evals_to!( From 517f8f4a4a11d5411c00068f4f9c9bd03fb4dbc6 Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 24 Nov 2020 20:57:10 +0100 Subject: [PATCH 05/12] simplify handling of optional fields --- compiler/mono/src/ir.rs | 181 ++++++++++++++++++---------------------- 1 file changed, 79 insertions(+), 102 deletions(-) diff --git a/compiler/mono/src/ir.rs b/compiler/mono/src/ir.rs index 689eae51f0..86d15daac4 100644 --- a/compiler/mono/src/ir.rs +++ b/compiler/mono/src/ir.rs @@ -5374,8 +5374,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(); - destructs.sort_by(|a, b| a.value.label.cmp(&b.value.label)); + let destructs_by_label = env.arena.alloc(MutMap::default()); + 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 +5387,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(); - let mut opt_destruct = it2.next(); + for (label, variable, res_layout) in sorted_fields.into_iter() { + match res_layout { + Ok(field_layout) => { + // the field is non-optional according to the type - loop { - match (opt_sorted, opt_destruct) { - (Some((label, variable, Ok(field_layout))), Some(destruct)) => { - if destruct.value.label == label { - mono_destructs.push(from_can_record_destruct( - env, - layout_cache, - &destruct.value, - field_layout.clone(), - )); - - opt_sorted = it1.next(); - opt_destruct = it2.next(); - } else { - // insert underscore pattern - 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(); + match destructs_by_label.remove(&label) { + Some(destruct) => { + // this field is destructured by the pattern + mono_destructs.push(from_can_record_destruct( + env, + layout_cache, + &destruct.value, + field_layout.clone(), + )); + } + 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), + }); + } } + + // 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)) => { - if destruct.value.label == label { - opt_destruct = it2.next(); - - mono_destructs.push(RecordDestruct { - 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) - }); - + Err(field_layout) => { + // the field is optional according to the type + match destructs_by_label.remove(&label) { + Some(destruct) => { + // this field is destructured by the pattern 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; - opt_destruct = it2.next(); - } - (None, None) => { - break; + for (_, destruct) in destructs_by_label.drain() { + // this 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()), + }) } + _ => unreachable!("only optional destructs can be optional fields"), } } From 1e4f0e8b0714d99712806c173cdf1bd1eaac98d7 Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 24 Nov 2020 21:28:53 +0100 Subject: [PATCH 06/12] correct alignment in records! --- compiler/gen/tests/gen_records.rs | 1 - compiler/mono/src/decision_tree.rs | 38 +++++++++++++++++------------- compiler/mono/src/layout.rs | 15 +++++++++++- compiler/mono/tests/test_mono.rs | 14 +++++------ 4 files changed, 42 insertions(+), 26 deletions(-) diff --git a/compiler/gen/tests/gen_records.rs b/compiler/gen/tests/gen_records.rs index fafc43fbfa..00b561fe5e 100644 --- a/compiler/gen/tests/gen_records.rs +++ b/compiler/gen/tests/gen_records.rs @@ -846,7 +846,6 @@ mod gen_records { } #[test] - #[ignore] 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 }"), diff --git a/compiler/mono/src/decision_tree.rs b/compiler/mono/src/decision_tree.rs index f079b4d5eb..ec9fff2c5f 100644 --- a/compiler/mono/src/decision_tree.rs +++ b/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 pattern = match destruct.typ { - DestructType::Guard(guard) => guard.clone(), - DestructType::Required => Pattern::Underscore, - DestructType::Optional(_expr) => Pattern::Underscore, - }; + let sub_positions = destructs + .into_iter() + .filter(|destruct| !matches!(destruct.typ, DestructType::Optional(_))) + .enumerate() + .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 { - index: index as u64, - tag_id: *tag_id, - path: Box::new(path.clone()), - }, - Guard::NoGuard, - pattern, - ) - }); + ( + Path::Index { + index: index as u64, + tag_id: *tag_id, + path: Box::new(path.clone()), + }, + Guard::NoGuard, + pattern, + ) + }); start.extend(sub_positions); start.extend(end); diff --git a/compiler/mono/src/layout.rs b/compiler/mono/src/layout.rs index 41f0575d6d..0da6001200 100644 --- a/compiler/mono/src/layout.rs +++ b/compiler/mono/src/layout.rs @@ -927,7 +927,20 @@ fn sort_record_fields_help<'a>( } } - sorted_fields.sort_by(|(label1, _, _), (label2, _, _)| label1.cmp(label2)); + 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 } diff --git a/compiler/mono/tests/test_mono.rs b/compiler/mono/tests/test_mono.rs index ab1dab9af4..cec2e47028 100644 --- a/compiler/mono/tests/test_mono.rs +++ b/compiler/mono/tests/test_mono.rs @@ -1943,14 +1943,14 @@ mod test_mono { ret Test.13; procedure Test.1 (Test.6): - let Test.18 = Index 0 Test.6; + let Test.18 = Index 1 Test.6; let Test.19 = false; let Test.20 = lowlevel Eq Test.19 Test.18; if Test.20 then - let Test.8 = Index 1 Test.6; + let Test.8 = Index 0 Test.6; ret Test.8; else - let Test.10 = Index 1 Test.6; + let Test.10 = Index 0 Test.6; ret Test.10; procedure Test.1 (Test.6): @@ -1971,12 +1971,12 @@ mod test_mono { let Test.32 = false; let Test.26 = Struct {Test.32}; let Test.3 = CallByName Test.1 Test.26; - let Test.24 = true; - let Test.25 = 11i64; + let Test.24 = 11i64; + let Test.25 = true; let Test.23 = Struct {Test.24, Test.25}; let Test.4 = CallByName Test.1 Test.23; - let Test.21 = false; - let Test.22 = 7i64; + let Test.21 = 7i64; + let Test.22 = false; let Test.15 = Struct {Test.21, Test.22}; let Test.2 = CallByName Test.1 Test.15; let Test.14 = CallByName Num.16 Test.2 Test.3; From ccd2e0ecf445eb9649e48c75ce22be5fb5a1fb6c Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 24 Nov 2020 22:01:57 +0100 Subject: [PATCH 07/12] alignment in single element tag unions --- compiler/gen/src/llvm/build.rs | 21 ++++++++++++++++++--- compiler/gen/tests/gen_records.rs | 1 + compiler/gen/tests/gen_tags.rs | 11 +++++++++++ compiler/mono/src/ir.rs | 31 +++++++++++++++++++++++++++---- compiler/mono/src/layout.rs | 9 +++++++++ 5 files changed, 66 insertions(+), 7 deletions(-) diff --git a/compiler/gen/src/llvm/build.rs b/compiler/gen/src/llvm/build.rs index dbd06bd64c..7d90e65de2 100644 --- a/compiler/gen/src/llvm/build.rs +++ b/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(); } diff --git a/compiler/gen/tests/gen_records.rs b/compiler/gen/tests/gen_records.rs index 00b561fe5e..ba202c9bc7 100644 --- a/compiler/gen/tests/gen_records.rs +++ b/compiler/gen/tests/gen_records.rs @@ -853,6 +853,7 @@ mod gen_records { (i64, bool, u8) ); } + #[test] fn blue_and_present() { assert_evals_to!( diff --git a/compiler/gen/tests/gen_tags.rs b/compiler/gen/tests/gen_tags.rs index 7e317015b9..6a89b496c6 100644 --- a/compiler/gen/tests/gen_tags.rs +++ b/compiler/gen/tests/gen_tags.rs @@ -758,4 +758,15 @@ mod gen_tags { i64 ); } + + #[test] + fn alignment_in_single_tag() { + 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) + ); + } } diff --git a/compiler/mono/src/ir.rs b/compiler/mono/src/ir.rs index 86d15daac4..5576eb6c56 100644 --- a/compiler/mono/src/ir.rs +++ b/compiler/mono/src/ir.rs @@ -2384,7 +2384,7 @@ pub fn with_hole<'a>( Tag { variant_var, name: tag_name, - arguments: args, + arguments: mut args, .. } => { use crate::layout::UnionVariant::*; @@ -2421,11 +2421,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() { - field_symbols.push(possible_reuse_symbol(env, procs, &arg.value)); + for (_, symbol, _) in field_symbols_temp.iter() { + 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 +2461,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) => { diff --git a/compiler/mono/src/layout.rs b/compiler/mono/src/layout.rs index 0da6001200..be9409796b 100644 --- a/compiler/mono/src/layout.rs +++ b/compiler/mono/src/layout.rs @@ -1047,6 +1047,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 From 69734e837e00d53fdba760555a0a4170ca2e30ec Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 24 Nov 2020 22:36:09 +0100 Subject: [PATCH 08/12] alignment in pattern match on single element tag union --- compiler/gen/tests/gen_tags.rs | 33 ++++++++++++++++++++++++++++++++- compiler/mono/src/ir.rs | 14 ++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/compiler/gen/tests/gen_tags.rs b/compiler/gen/tests/gen_tags.rs index 6a89b496c6..eabbd1803a 100644 --- a/compiler/gen/tests/gen_tags.rs +++ b/compiler/gen/tests/gen_tags.rs @@ -760,7 +760,7 @@ mod gen_tags { } #[test] - fn alignment_in_single_tag() { + fn alignment_in_single_tag_construction() { assert_evals_to!(indoc!("Three (1 == 1) 32"), (32i64, true), (i64, bool)); assert_evals_to!( @@ -769,4 +769,35 @@ mod gen_tags { (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) + ); + } } diff --git a/compiler/mono/src/ir.rs b/compiler/mono/src/ir.rs index 5576eb6c56..4ee3501a6f 100644 --- a/compiler/mono/src/ir.rs +++ b/compiler/mono/src/ir.rs @@ -5313,6 +5313,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(( From 0f1baef1606cee35b54d522bf4786b79cf6933d9 Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 24 Nov 2020 23:15:32 +0100 Subject: [PATCH 09/12] alignment in multi tag pattern match --- compiler/gen/tests/gen_tags.rs | 90 +++++++++++++++++++++++++++++----- compiler/mono/src/ir.rs | 45 +++++++++++++++-- compiler/mono/src/layout.rs | 9 ++++ 3 files changed, 130 insertions(+), 14 deletions(-) diff --git a/compiler/gen/tests/gen_tags.rs b/compiler/gen/tests/gen_tags.rs index eabbd1803a..9aa5468e6e 100644 --- a/compiler/gen/tests/gen_tags.rs +++ b/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 = \{} -> - y = + wrapper = \{} -> + 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 @@ -778,7 +778,7 @@ mod gen_tags { x = Three (1 == 1) 32 when x is - Three bool int -> + Three bool int -> { bool, int } #" ), @@ -792,7 +792,7 @@ mod gen_tags { x = Three (1 == 1) (if True then Red else if True then Green else Blue) 32 when x is - Three bool color int -> + Three bool color int -> { bool, color, int } #" ), @@ -800,4 +800,72 @@ mod gen_tags { (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) + ); + } } diff --git a/compiler/mono/src/ir.rs b/compiler/mono/src/ir.rs index 4ee3501a6f..89c7a9cd40 100644 --- a/compiler/mono/src/ir.rs +++ b/compiler/mono/src/ir.rs @@ -2472,12 +2472,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 = 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() { - field_symbols.push(possible_reuse_symbol(env, procs, &arg.value)); + for (_, symbol, _) in field_symbols_temp.iter() { + field_symbols.push(*symbol); } let mut layouts: Vec<&'a [Layout<'a>]> = @@ -2498,7 +2519,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); @@ -5370,6 +5395,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()); diff --git a/compiler/mono/src/layout.rs b/compiler/mono/src/layout.rs index be9409796b..336a0103e7 100644 --- a/compiler/mono/src/layout.rs +++ b/compiler/mono/src/layout.rs @@ -1099,6 +1099,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())); } From d821a174131f6c814afff49f4fbbd9ff4b27ae45 Mon Sep 17 00:00:00 2001 From: Folkert Date: Tue, 24 Nov 2020 23:47:57 +0100 Subject: [PATCH 10/12] fix recursive tag unions too --- compiler/mono/src/layout.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/compiler/mono/src/layout.rs b/compiler/mono/src/layout.rs index 336a0103e7..aed2b979d8 100644 --- a/compiler/mono/src/layout.rs +++ b/compiler/mono/src/layout.rs @@ -866,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()); } From 0d15f92f00e6e7f4d15612c78dccf3aacff42052 Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 25 Nov 2020 19:43:57 +0100 Subject: [PATCH 11/12] fix mono tests --- compiler/mono/tests/test_mono.rs | 142 +++++++++++++++---------------- 1 file changed, 71 insertions(+), 71 deletions(-) diff --git a/compiler/mono/tests/test_mono.rs b/compiler/mono/tests/test_mono.rs index cec2e47028..660c0b8cd3 100644 --- a/compiler/mono/tests/test_mono.rs +++ b/compiler/mono/tests/test_mono.rs @@ -189,9 +189,9 @@ mod test_mono { indoc!( r#" procedure Test.0 (): - let Test.8 = 0i64; - let Test.9 = 3i64; - let Test.2 = Just Test.8 Test.9; + let Test.9 = 0i64; + let Test.8 = 3i64; + let Test.2 = Just Test.9 Test.8; let Test.5 = 0i64; let Test.6 = Index 0 Test.2; let Test.7 = lowlevel Eq Test.5 Test.6; @@ -218,10 +218,10 @@ mod test_mono { indoc!( r#" procedure Test.0 (): + let Test.10 = 1i64; let Test.8 = 1i64; - let Test.9 = 1i64; - let Test.10 = 2i64; - let Test.4 = These Test.8 Test.9 Test.10; + let Test.9 = 2i64; + let Test.4 = These Test.10 Test.8 Test.9; switch Test.4: case 2: let Test.1 = Index 1 Test.4; @@ -317,14 +317,14 @@ mod test_mono { let Test.17 = 0i64; let Test.13 = lowlevel NotEq #Attr.3 Test.17; if Test.13 then - let Test.15 = 1i64; - let Test.16 = lowlevel NumDivUnchecked #Attr.2 #Attr.3; - let Test.14 = Ok Test.15 Test.16; + let Test.16 = 1i64; + let Test.15 = lowlevel NumDivUnchecked #Attr.2 #Attr.3; + let Test.14 = Ok Test.16 Test.15; ret Test.14; else - let Test.11 = 0i64; - let Test.12 = Struct {}; - let Test.10 = Err Test.11 Test.12; + let Test.12 = 0i64; + let Test.11 = Struct {}; + let Test.10 = Err Test.12 Test.11; ret Test.10; procedure Test.0 (): @@ -388,9 +388,9 @@ mod test_mono { ret Test.5; procedure Test.0 (): - let Test.10 = 0i64; - let Test.11 = 41i64; - let Test.1 = Just Test.10 Test.11; + let Test.11 = 0i64; + let Test.10 = 41i64; + let Test.1 = Just Test.11 Test.10; let Test.7 = 0i64; let Test.8 = Index 0 Test.1; let Test.9 = lowlevel Eq Test.7 Test.8; @@ -515,11 +515,11 @@ mod test_mono { ret Test.6; procedure Test.0 (): - let Test.17 = 0i64; - let Test.19 = 0i64; - let Test.20 = 41i64; - let Test.18 = Just Test.19 Test.20; - let Test.2 = Just Test.17 Test.18; + let Test.18 = 0i64; + let Test.20 = 0i64; + let Test.19 = 41i64; + let Test.17 = Just Test.20 Test.19; + let Test.2 = Just Test.18 Test.17; joinpoint Test.14: let Test.8 = 1i64; ret Test.8; @@ -562,8 +562,8 @@ mod test_mono { ret Test.6; procedure Test.0 (): - let Test.14 = 2i64; let Test.15 = 3i64; + let Test.14 = 2i64; let Test.3 = Struct {Test.14, Test.15}; joinpoint Test.11: let Test.1 = Index 0 Test.3; @@ -809,9 +809,9 @@ mod test_mono { indoc!( r#" procedure Test.1 (Test.4): - let Test.18 = 1i64; - let Test.19 = 2i64; - let Test.2 = Ok Test.18 Test.19; + let Test.19 = 1i64; + let Test.18 = 2i64; + let Test.2 = Ok Test.19 Test.18; joinpoint Test.8 Test.3: ret Test.3; in @@ -1299,14 +1299,14 @@ mod test_mono { let Test.38 = lowlevel ListLen #Attr.2; let Test.34 = lowlevel NumLt #Attr.3 Test.38; if Test.34 then - let Test.36 = 1i64; - let Test.37 = lowlevel ListGetUnsafe #Attr.2 #Attr.3; - let Test.35 = Ok Test.36 Test.37; + let Test.37 = 1i64; + let Test.36 = lowlevel ListGetUnsafe #Attr.2 #Attr.3; + let Test.35 = Ok Test.37 Test.36; ret Test.35; else - let Test.32 = 0i64; - let Test.33 = Struct {}; - let Test.31 = Err Test.32 Test.33; + let Test.33 = 0i64; + let Test.32 = Struct {}; + let Test.31 = Err Test.33 Test.32; ret Test.31; procedure List.4 (#Attr.2, #Attr.3, #Attr.4): @@ -1320,9 +1320,9 @@ mod test_mono { procedure Test.1 (Test.2): let Test.39 = 0i64; - let Test.28 = CallByName List.3 Test.2 Test.39; + let Test.29 = CallByName List.3 Test.2 Test.39; let Test.30 = 0i64; - let Test.29 = CallByName List.3 Test.2 Test.30; + let Test.28 = CallByName List.3 Test.2 Test.30; let Test.7 = Struct {Test.28, Test.29}; joinpoint Test.25: let Test.18 = Array []; @@ -1763,14 +1763,14 @@ mod test_mono { let Test.15 = lowlevel ListLen #Attr.2; let Test.11 = lowlevel NumLt #Attr.3 Test.15; if Test.11 then - let Test.13 = 1i64; - let Test.14 = lowlevel ListGetUnsafe #Attr.2 #Attr.3; - let Test.12 = Ok Test.13 Test.14; + let Test.14 = 1i64; + let Test.13 = lowlevel ListGetUnsafe #Attr.2 #Attr.3; + let Test.12 = Ok Test.14 Test.13; ret Test.12; else - let Test.9 = 0i64; - let Test.10 = Struct {}; - let Test.8 = Err Test.9 Test.10; + let Test.10 = 0i64; + let Test.9 = Struct {}; + let Test.8 = Err Test.10 Test.9; ret Test.8; procedure Test.1 (Test.2): @@ -1808,14 +1808,14 @@ mod test_mono { indoc!( r#" procedure Test.0 (): - let Test.4 = 0i64; - let Test.6 = 0i64; - let Test.8 = 0i64; + let Test.5 = 0i64; + let Test.7 = 0i64; + let Test.9 = 0i64; let Test.10 = 1i64; - let Test.9 = Z Test.10; - let Test.7 = S Test.8 Test.9; - let Test.5 = S Test.6 Test.7; - let Test.2 = S Test.4 Test.5; + let Test.8 = Z Test.10; + let Test.6 = S Test.9 Test.8; + let Test.4 = S Test.7 Test.6; + let Test.2 = S Test.5 Test.4; ret Test.2; "# ), @@ -1840,14 +1840,14 @@ mod test_mono { indoc!( r#" procedure Test.0 (): - let Test.8 = 0i64; - let Test.10 = 0i64; - let Test.12 = 0i64; + let Test.9 = 0i64; + let Test.11 = 0i64; + let Test.13 = 0i64; let Test.14 = 1i64; - let Test.13 = Z Test.14; - let Test.11 = S Test.12 Test.13; - let Test.9 = S Test.10 Test.11; - let Test.2 = S Test.8 Test.9; + let Test.12 = Z Test.14; + let Test.10 = S Test.13 Test.12; + let Test.8 = S Test.11 Test.10; + let Test.2 = S Test.9 Test.8; let Test.5 = 1i64; let Test.6 = Index 0 Test.2; dec Test.2; @@ -1882,14 +1882,14 @@ mod test_mono { indoc!( r#" procedure Test.0 (): - let Test.14 = 0i64; - let Test.16 = 0i64; - let Test.18 = 0i64; + let Test.15 = 0i64; + let Test.17 = 0i64; + let Test.19 = 0i64; let Test.20 = 1i64; - let Test.19 = Z Test.20; - let Test.17 = S Test.18 Test.19; - let Test.15 = S Test.16 Test.17; - let Test.2 = S Test.14 Test.15; + let Test.18 = Z Test.20; + let Test.16 = S Test.19 Test.18; + let Test.14 = S Test.17 Test.16; + let Test.2 = S Test.15 Test.14; let Test.11 = 0i64; let Test.12 = Index 0 Test.2; let Test.13 = lowlevel Eq Test.11 Test.12; @@ -2010,11 +2010,11 @@ mod test_mono { ret Test.6; procedure Test.0 (): - let Test.17 = 0i64; - let Test.19 = 0i64; - let Test.20 = 41i64; - let Test.18 = Just Test.19 Test.20; - let Test.2 = Just Test.17 Test.18; + let Test.18 = 0i64; + let Test.20 = 0i64; + let Test.19 = 41i64; + let Test.17 = Just Test.20 Test.19; + let Test.2 = Just Test.18 Test.17; joinpoint Test.14: let Test.8 = 1i64; ret Test.8; @@ -2128,14 +2128,14 @@ mod test_mono { let Test.40 = lowlevel ListLen #Attr.2; let Test.36 = lowlevel NumLt #Attr.3 Test.40; if Test.36 then - let Test.38 = 1i64; - let Test.39 = lowlevel ListGetUnsafe #Attr.2 #Attr.3; - let Test.37 = Ok Test.38 Test.39; + let Test.39 = 1i64; + let Test.38 = lowlevel ListGetUnsafe #Attr.2 #Attr.3; + let Test.37 = Ok Test.39 Test.38; ret Test.37; else - let Test.34 = 0i64; - let Test.35 = Struct {}; - let Test.33 = Err Test.34 Test.35; + let Test.35 = 0i64; + let Test.34 = Struct {}; + let Test.33 = Err Test.35 Test.34; ret Test.33; procedure List.4 (#Attr.2, #Attr.3, #Attr.4): @@ -2148,8 +2148,8 @@ mod test_mono { ret #Attr.2; procedure Test.1 (Test.2, Test.3, Test.4): - let Test.31 = CallByName List.3 Test.4 Test.2; let Test.32 = CallByName List.3 Test.4 Test.3; + let Test.31 = CallByName List.3 Test.4 Test.2; let Test.12 = Struct {Test.31, Test.32}; joinpoint Test.28: let Test.21 = Array []; From fb70ce7c71da2dfea87c5743c81550bc541cf3e7 Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 25 Nov 2020 20:35:24 +0100 Subject: [PATCH 12/12] clippy --- compiler/mono/src/ir.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/mono/src/ir.rs b/compiler/mono/src/ir.rs index 20c28c2eb5..72b5080199 100644 --- a/compiler/mono/src/ir.rs +++ b/compiler/mono/src/ir.rs @@ -909,7 +909,7 @@ where } else { let text = format!("{}", symbol); - if text.starts_with(".") { + if text.starts_with('.') { alloc.text("Test").append(text) } else { alloc.text(text)