From f732eb3e83be46100e091119a69d26415673dd72 Mon Sep 17 00:00:00 2001 From: Folkert Date: Sun, 20 Sep 2020 17:01:00 +0200 Subject: [PATCH 01/10] crash upon integer overflow --- compiler/builtins/src/std.rs | 20 +++++++++++++ compiler/builtins/src/unique.rs | 20 +++++++++++++ compiler/gen/src/llvm/build.rs | 48 ++++++++++++++++++++++++++++++-- compiler/gen/tests/gen_num.rs | 14 ++++++++++ compiler/module/src/low_level.rs | 2 ++ compiler/module/src/symbol.rs | 2 ++ compiler/mono/src/borrow.rs | 7 ++--- 7 files changed, 107 insertions(+), 6 deletions(-) diff --git a/compiler/builtins/src/std.rs b/compiler/builtins/src/std.rs index a46e1c842c..acdae6f60a 100644 --- a/compiler/builtins/src/std.rs +++ b/compiler/builtins/src/std.rs @@ -187,6 +187,26 @@ pub fn types() -> MutMap { ), ); + // addChecked or (+) : Num a, Num a -> Result (Num a) [ IntOverflow ]* + let int_overflow = SolvedType::TagUnion( + vec![(TagName::Global("IntOverflow".into()), vec![])], + Box::new(SolvedType::Wildcard), + ); + + add_type( + Symbol::NUM_ADD_CHECKED, + SolvedType::Func( + vec![num_type(flex(TVAR1)), num_type(flex(TVAR1))], + Box::new(result_type(num_type(flex(TVAR1)), int_overflow)), + ), + ); + + // addWrap or (+) : Int, Int -> Int + add_type( + Symbol::NUM_ADD_WRAP, + SolvedType::Func(vec![int_type(), int_type()], Box::new(int_type())), + ); + // sub or (-) : Num a, Num a -> Num a add_type( Symbol::NUM_SUB, diff --git a/compiler/builtins/src/unique.rs b/compiler/builtins/src/unique.rs index a04bc1bfdc..a5dfb09b6f 100644 --- a/compiler/builtins/src/unique.rs +++ b/compiler/builtins/src/unique.rs @@ -274,6 +274,26 @@ pub fn types() -> MutMap { unique_function(vec![num_type(u, num), num_type(v, num)], num_type(w, num)) }); + // addChecked or (+) : Num a, Num a -> Result (Num a) [ IntOverflow ]* + let int_overflow = SolvedType::TagUnion( + vec![(TagName::Global("IntOverflow".into()), vec![])], + Box::new(SolvedType::Wildcard), + ); + + add_type(Symbol::NUM_ADD_CHECKED, { + let_tvars! { u, v, w, num, result, star }; + unique_function( + vec![num_type(u, num), num_type(v, num)], + result_type(result, num_type(w, num), lift(star, int_overflow)), + ) + }); + + // addWrap or (+) : Int, Int -> Int + add_type(Symbol::NUM_ADD_WRAP, { + let_tvars! { u, v, w }; + unique_function(vec![int_type(u), int_type(v)], int_type(w)) + }); + // sub or (-) : Num a, Num a -> Num a add_type(Symbol::NUM_SUB, { let_tvars! { u, v, w, num }; diff --git a/compiler/gen/src/llvm/build.rs b/compiler/gen/src/llvm/build.rs index 388cd6b162..18bc2bcc93 100644 --- a/compiler/gen/src/llvm/build.rs +++ b/compiler/gen/src/llvm/build.rs @@ -270,6 +270,12 @@ fn add_intrinsics<'ctx>(ctx: &'ctx Context, module: &Module<'ctx>) { LLVM_FLOOR_F64, f64_type.fn_type(&[f64_type.into()], false), ); + + add_intrinsic(module, LLVM_SADD_WITH_OVERFLOW_I64, { + let fields = [i64_type.into(), i1_type.into()]; + ctx.struct_type(&fields, false) + .fn_type(&[i64_type.into(), i64_type.into()], false) + }); } static LLVM_MEMSET_I64: &str = "llvm.memset.p0i8.i64"; @@ -282,6 +288,7 @@ static LLVM_COS_F64: &str = "llvm.cos.f64"; static LLVM_POW_F64: &str = "llvm.pow.f64"; static LLVM_CEILING_F64: &str = "llvm.ceil.f64"; static LLVM_FLOOR_F64: &str = "llvm.floor.f64"; +static LLVM_SADD_WITH_OVERFLOW_I64: &str = "llvm.sadd.with.overflow.i64"; fn add_intrinsic<'ctx>( module: &Module<'ctx>, @@ -2219,7 +2226,7 @@ fn run_low_level<'a, 'ctx, 'env>( } NumAdd | NumSub | NumMul | NumLt | NumLte | NumGt | NumGte | NumRemUnchecked - | NumDivUnchecked | NumPow | NumPowInt => { + | NumAddWrap | NumAddChecked | NumDivUnchecked | NumPow | NumPowInt => { debug_assert_eq!(args.len(), 2); let (lhs_arg, lhs_layout) = load_symbol_and_layout(env, scope, &args[0]); @@ -2234,6 +2241,7 @@ fn run_low_level<'a, 'ctx, 'env>( match lhs_builtin { Int128 | Int64 | Int32 | Int16 | Int8 => build_int_binop( env, + parent, lhs_arg.into_int_value(), lhs_layout, rhs_arg.into_int_value(), @@ -2413,6 +2421,7 @@ where fn build_int_binop<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, + parent: FunctionValue<'ctx>, lhs: IntValue<'ctx>, _lhs_layout: &Layout<'a>, rhs: IntValue<'ctx>, @@ -2425,7 +2434,40 @@ fn build_int_binop<'a, 'ctx, 'env>( let bd = env.builder; match op { - NumAdd => bd.build_int_add(lhs, rhs, "add_int").into(), + NumAdd => { + // TODO + let builder = env.builder; + let context = env.context; + + let result = env + .call_intrinsic(LLVM_SADD_WITH_OVERFLOW_I64, &[lhs.into(), rhs.into()]) + .into_struct_value(); + + let add_result = bd.build_extract_value(result, 0, "add_result").unwrap(); + let has_overflowed = bd.build_extract_value(result, 1, "has_overflowed").unwrap(); + + let condition = builder.build_int_compare( + IntPredicate::EQ, + has_overflowed.into_int_value(), + context.bool_type().const_zero(), + "has_not_overflowed", + ); + + let then_block = context.append_basic_block(parent, "then_block"); + let throw_block = context.append_basic_block(parent, "throw_block"); + + builder.build_conditional_branch(condition, then_block, throw_block); + + builder.position_at_end(throw_block); + + throw_exception(env, "integer addition overflowed!"); + + builder.position_at_end(then_block); + + add_result + } + NumAddWrap => bd.build_int_add(lhs, rhs, "add_int_wrap").into(), + NumAddChecked => bd.build_int_add(lhs, rhs, "add_int_checked").into(), NumSub => bd.build_int_sub(lhs, rhs, "sub_int").into(), NumMul => bd.build_int_mul(lhs, rhs, "mul_int").into(), NumGt => bd.build_int_compare(SGT, lhs, rhs, "int_gt").into(), @@ -2475,6 +2517,8 @@ fn build_float_binop<'a, 'ctx, 'env>( match op { NumAdd => bd.build_float_add(lhs, rhs, "add_float").into(), + NumAddWrap => unreachable!("wrapping addition is not defined on floats"), + NumAddChecked => bd.build_float_add(lhs, rhs, "add_float_checked").into(), NumSub => bd.build_float_sub(lhs, rhs, "sub_float").into(), NumMul => bd.build_float_mul(lhs, rhs, "mul_float").into(), NumGt => bd.build_float_compare(OGT, lhs, rhs, "float_gt").into(), diff --git a/compiler/gen/tests/gen_num.rs b/compiler/gen/tests/gen_num.rs index 1c9e878e1a..8c52a87bab 100644 --- a/compiler/gen/tests/gen_num.rs +++ b/compiler/gen/tests/gen_num.rs @@ -685,4 +685,18 @@ mod gen_num { fn pow_int() { assert_evals_to!("Num.powInt 2 3", 8, i64); } + + #[test] + #[should_panic(expected = r#"Roc failed with message: "integer addition overflowed!"#)] + fn int_overflow() { + assert_evals_to!( + indoc!( + r#" + 9_223_372_036_854_775_807 + 1 + "# + ), + 0, + i64 + ); + } } diff --git a/compiler/module/src/low_level.rs b/compiler/module/src/low_level.rs index e347d134c5..483f890f9d 100644 --- a/compiler/module/src/low_level.rs +++ b/compiler/module/src/low_level.rs @@ -20,6 +20,8 @@ pub enum LowLevel { ListKeepIf, ListWalkRight, NumAdd, + NumAddWrap, + NumAddChecked, NumSub, NumMul, NumGt, diff --git a/compiler/module/src/symbol.rs b/compiler/module/src/symbol.rs index e61883ff15..8566b1156e 100644 --- a/compiler/module/src/symbol.rs +++ b/compiler/module/src/symbol.rs @@ -644,6 +644,8 @@ define_builtins! { 39 NUM_CEILING: "ceiling" 40 NUM_POW_INT: "powInt" 41 NUM_FLOOR: "floor" + 42 NUM_ADD_WRAP: "addWrap" + 43 NUM_ADD_CHECKED: "addChecked" } 2 BOOL: "Bool" => { 0 BOOL_BOOL: "Bool" imported // the Bool.Bool type alias diff --git a/compiler/mono/src/borrow.rs b/compiler/mono/src/borrow.rs index ac02b9d457..4ec5a56aee 100644 --- a/compiler/mono/src/borrow.rs +++ b/compiler/mono/src/borrow.rs @@ -521,10 +521,9 @@ pub fn lowlevel_borrow_signature(arena: &Bump, op: LowLevel) -> &[bool] { ListKeepIf => arena.alloc_slice_copy(&[owned, irrelevant]), ListWalkRight => arena.alloc_slice_copy(&[borrowed, irrelevant, owned]), - Eq | NotEq | And | Or | NumAdd | NumSub | NumMul | NumGt | NumGte | NumLt | NumLte - | NumCompare | NumDivUnchecked | NumRemUnchecked | NumPow | NumPowInt => { - arena.alloc_slice_copy(&[irrelevant, irrelevant]) - } + Eq | NotEq | And | Or | NumAdd | NumAddWrap | NumAddChecked | NumSub | NumMul | NumGt + | NumGte | NumLt | NumLte | NumCompare | NumDivUnchecked | NumRemUnchecked | NumPow + | NumPowInt => arena.alloc_slice_copy(&[irrelevant, irrelevant]), NumAbs | NumNeg | NumSin | NumCos | NumSqrtUnchecked | NumRound | NumCeiling | NumFloor | NumToFloat | Not => arena.alloc_slice_copy(&[irrelevant]), From 95177eee5a749750de67b798c95f9e9d26c5064a Mon Sep 17 00:00:00 2001 From: Folkert Date: Sun, 20 Sep 2020 20:21:45 +0200 Subject: [PATCH 02/10] integer addition operations --- compiler/can/src/builtins.rs | 103 +++++++++++++++++++++ compiler/gen/src/llvm/build.rs | 2 +- compiler/gen/tests/gen_num.rs | 53 +++++++++++ compiler/reporting/tests/test_reporting.rs | 27 ++++++ 4 files changed, 184 insertions(+), 1 deletion(-) diff --git a/compiler/can/src/builtins.rs b/compiler/can/src/builtins.rs index 065664ee6c..f5aa06ef8d 100644 --- a/compiler/can/src/builtins.rs +++ b/compiler/can/src/builtins.rs @@ -68,6 +68,8 @@ pub fn builtin_defs(var_store: &mut VarStore) -> MutMap { Symbol::LIST_KEEP_IF => list_keep_if, Symbol::LIST_WALK_RIGHT => list_walk_right, Symbol::NUM_ADD => num_add, + Symbol::NUM_ADD_CHECKED => num_add_checked, + Symbol::NUM_ADD_WRAP => num_add_wrap, Symbol::NUM_SUB => num_sub, Symbol::NUM_MUL => num_mul, Symbol::NUM_GT => num_gt, @@ -238,6 +240,107 @@ fn num_add(symbol: Symbol, var_store: &mut VarStore) -> Def { num_binop(symbol, var_store, LowLevel::NumAdd) } +/// Num.add : Num a, Num a -> Num a +fn num_add_wrap(symbol: Symbol, var_store: &mut VarStore) -> Def { + num_binop(symbol, var_store, LowLevel::NumAddWrap) +} + +/// Num.add : Num a, Num a -> Num a +fn num_add_checked(symbol: Symbol, var_store: &mut VarStore) -> Def { + let bool_var = var_store.fresh(); + let num_var_1 = var_store.fresh(); + let num_var_2 = var_store.fresh(); + let num_var_3 = var_store.fresh(); + let ret_var = var_store.fresh(); + let record_var = var_store.fresh(); + + // let arg_3 = RunLowLevel NumAddChecked arg_1 arg_2 + // + // if arg_3.b then + // # overflow + // Err IntOverflow + // else + // # all is well + // Ok arg_3.a + + let cont = If { + branch_var: ret_var, + cond_var: bool_var, + branches: vec![( + // if-condition + no_region( + // Num.neq denominator 0 + Access { + record_var, + ext_var: var_store.fresh(), + field: "b".into(), + field_var: var_store.fresh(), + loc_expr: Box::new(no_region(Var(Symbol::ARG_3))), + }, + ), + // overflow! + no_region(tag( + "Err", + vec![tag("DivByZero", Vec::new(), var_store)], + var_store, + )), + )], + final_else: Box::new( + // all is well + no_region( + // Ok (Float.#divUnchecked numerator denominator) + tag( + "Ok", + vec![ + // Num.#divUnchecked numerator denominator + Access { + record_var, + ext_var: var_store.fresh(), + field: "a".into(), + field_var: num_var_3, + loc_expr: Box::new(no_region(Var(Symbol::ARG_3))), + }, + ], + var_store, + ), + ), + ), + }; + + let def = crate::def::Def { + loc_pattern: no_region(Pattern::Identifier(Symbol::ARG_3)), + loc_expr: no_region( + // Num.neq denominator 0 + RunLowLevel { + op: LowLevel::NumAddChecked, + args: vec![ + (num_var_1, Var(Symbol::ARG_1)), + (num_var_2, Var(Symbol::ARG_2)), + ], + ret_var: record_var, + }, + ), + expr_var: record_var, + pattern_vars: SendMap::default(), + annotation: None, + }; + + let body = LetNonRec( + Box::new(def), + Box::new(no_region(cont)), + ret_var, + SendMap::default(), + ); + + defn( + symbol, + vec![(num_var_1, Symbol::ARG_1), (num_var_2, Symbol::ARG_2)], + var_store, + body, + ret_var, + ) +} + /// Num.sub : Num a, Num a -> Num a fn num_sub(symbol: Symbol, var_store: &mut VarStore) -> Def { num_binop(symbol, var_store, LowLevel::NumSub) diff --git a/compiler/gen/src/llvm/build.rs b/compiler/gen/src/llvm/build.rs index 18bc2bcc93..5b8156a6c9 100644 --- a/compiler/gen/src/llvm/build.rs +++ b/compiler/gen/src/llvm/build.rs @@ -2467,7 +2467,7 @@ fn build_int_binop<'a, 'ctx, 'env>( add_result } NumAddWrap => bd.build_int_add(lhs, rhs, "add_int_wrap").into(), - NumAddChecked => bd.build_int_add(lhs, rhs, "add_int_checked").into(), + NumAddChecked => env.call_intrinsic(LLVM_SADD_WITH_OVERFLOW_I64, &[lhs.into(), rhs.into()]), NumSub => bd.build_int_sub(lhs, rhs, "sub_int").into(), NumMul => bd.build_int_mul(lhs, rhs, "mul_int").into(), NumGt => bd.build_int_compare(SGT, lhs, rhs, "int_gt").into(), diff --git a/compiler/gen/tests/gen_num.rs b/compiler/gen/tests/gen_num.rs index 8c52a87bab..64f79896a0 100644 --- a/compiler/gen/tests/gen_num.rs +++ b/compiler/gen/tests/gen_num.rs @@ -699,4 +699,57 @@ mod gen_num { i64 ); } + + #[test] + fn add_checked() { + assert_evals_to!( + indoc!( + r#" + when Num.addChecked 1 2 is + Ok 3 -> True + _ -> False + "# + ), + true, + bool + ); + + assert_evals_to!( + indoc!( + r#" + when Num.addChecked 9_223_372_036_854_775_807 1 is + Err IntOverflow -> True + _ -> False + "# + ), + true, + bool + ); + } + + #[test] + fn add_wrap() { + assert_evals_to!( + indoc!( + r#" + Num.addWrap 9_223_372_036_854_775_807 1 + "# + ), + std::i64::MIN, + i64 + ); + } + + #[test] + fn float_overflow() { + assert_evals_to!( + indoc!( + r#" + 1.7976931348623157E+308 + 1 + "# + ), + 0.0, + f64 + ); + } } diff --git a/compiler/reporting/tests/test_reporting.rs b/compiler/reporting/tests/test_reporting.rs index 900e4f980e..6cd693bfe2 100644 --- a/compiler/reporting/tests/test_reporting.rs +++ b/compiler/reporting/tests/test_reporting.rs @@ -3847,4 +3847,31 @@ mod test_reporting { ), ) } + + #[test] + fn foobar() { + report_problem_as( + indoc!( + r#" + Num.addChecked 1 2 + "# + ), + indoc!( + r#" + ── REDUNDANT PATTERN ─────────────────────────────────────────────────────────── + + The 3rd pattern is redundant: + + 1│ when Foo 1 2 3 is + 2│ Foo _ 1 _ -> 1 + 3│ _ -> 2 + 4│ _ -> 3 + ^ + + Any value of this shape will be handled by a previous pattern, so this + one should be removed. + "# + ), + ) + } } From ac8e3928e3ff52add73997b42daef2caa8aecc2c Mon Sep 17 00:00:00 2001 From: Folkert Date: Mon, 21 Sep 2020 23:30:11 +0200 Subject: [PATCH 03/10] remove unneeded test --- compiler/reporting/tests/test_reporting.rs | 27 ---------------------- 1 file changed, 27 deletions(-) diff --git a/compiler/reporting/tests/test_reporting.rs b/compiler/reporting/tests/test_reporting.rs index 6cd693bfe2..900e4f980e 100644 --- a/compiler/reporting/tests/test_reporting.rs +++ b/compiler/reporting/tests/test_reporting.rs @@ -3847,31 +3847,4 @@ mod test_reporting { ), ) } - - #[test] - fn foobar() { - report_problem_as( - indoc!( - r#" - Num.addChecked 1 2 - "# - ), - indoc!( - r#" - ── REDUNDANT PATTERN ─────────────────────────────────────────────────────────── - - The 3rd pattern is redundant: - - 1│ when Foo 1 2 3 is - 2│ Foo _ 1 _ -> 1 - 3│ _ -> 2 - 4│ _ -> 3 - ^ - - Any value of this shape will be handled by a previous pattern, so this - one should be removed. - "# - ), - ) - } } From 455b73e8bd16c53bfb4be2c4a3d063763e8d6be8 Mon Sep 17 00:00:00 2001 From: Folkert Date: Mon, 21 Sep 2020 23:31:11 +0200 Subject: [PATCH 04/10] add isFinite to builtins bytecode --- compiler/gen/src/llvm/builtins.bc | Bin 2112 -> 2284 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/compiler/gen/src/llvm/builtins.bc b/compiler/gen/src/llvm/builtins.bc index c56125a48e68a62dc464e0e3af822d538423c6df..746943b3936f8246b5c4065b21dc0ac0c664cfc2 100644 GIT binary patch delta 596 zcmX>g@J4Wg2jk6&p4p826Spd;m^d>Ovn&W;;h1FP!g5i@F@S}6S7U=ElYs=|3?>GK zKnVs0iHTnW6)v!sZ)h)?&~CwCCcwnd!N|bi36uh=U=VIfXcFL<9LIRu*m;$wB?E&1 zP!J@+z#!(3)a-EaJp)674oHvhfC@JI^!Oz7cMb3ex7d}(t8(;_C0QwB{71e7_v zCNr_FVoaO7mbJY;O!3dIM1xDt9Q#~!S`5BEJHXQ5@?*<>j!7&tScDja9$x;M%YQij zzFQ$f(}T;Uxjfto9CmN@n^YLt13ZKUe6MW~xc>O^KP3;L1Y!0{NEk@Aca7%PyrZT zlQ*)t2757RXn;f*7zz~_7+8R`5ubok3&UF`i30rwrp?R@-CVZ+?rsu&7k~C~(ScH% zJKT4t|Cg2jRXE?Vm?4#ifk7LnM~Hb!0?6H<;Fg_i%Pwum3}h_;Vh}I`VqhdO_yMsp z5OV`D2rK}T5Q2GfC%d_ZFp$Fq#4sA@F&5`tn=6$mTG!^hDbFfcVVqL`;Hu)@Td%c_DpIxyAmz+8Fx#+YQe7$yn zrNQOLmi-))SRz=27=#{P{+i2oIR3d=Aw$!H%cZ&8+zK3aZ}pp07}*0ngav$$Z4kKr z`0_s`51|Ah_Ci~`*xSn#|7_P3_#b;YO!3c8RvyQM`3D48wma&;tN^(g2o@?ZFbFa* zOx9<2u4iB<1oD`HqDFiIN-YdenIt~wF?4fT@40O)^2_>uZIMTQnA+)v+=nI0SU;MF z&R|UCVPH@NsuyCOq5(3H35bP(SQ3bRfEXke0mL8xqCxT lfCLD@6oA-FlLa}<1wgD=pjEO!8few?$pswZlP7XS0sze{a8v*Q From fb4a796e07484e943514a754a3d032ede5c0ba1d Mon Sep 17 00:00:00 2001 From: Folkert Date: Mon, 21 Sep 2020 23:38:10 +0200 Subject: [PATCH 05/10] add different variants of addition --- cli/src/lib.rs | 6 ++ compiler/builtins/bitcode/src/lib.rs | 9 +++ compiler/builtins/src/std.rs | 6 +- compiler/builtins/src/unique.rs | 6 +- compiler/can/src/builtins.rs | 4 +- compiler/gen/src/llvm/build.rs | 102 ++++++++++++++++++--------- compiler/gen/tests/gen_num.rs | 53 +++++++++++--- compiler/module/src/low_level.rs | 1 + compiler/mono/src/borrow.rs | 2 +- 9 files changed, 136 insertions(+), 53 deletions(-) diff --git a/cli/src/lib.rs b/cli/src/lib.rs index ce97bc90f3..b84b6ed76a 100644 --- a/cli/src/lib.rs +++ b/cli/src/lib.rs @@ -210,6 +210,12 @@ fn build_file( "host.rs", "-o", binary_path.as_path().to_str().unwrap(), + // ensure we don't make a position-independent executable + "-C", + "link-arg=-no-pie", + // explicitly link in the c++ stdlib, for exceptions + "-C", + "link-arg=-lc++", ]) .current_dir(cwd) .spawn() diff --git a/compiler/builtins/bitcode/src/lib.rs b/compiler/builtins/bitcode/src/lib.rs index 2316d1265b..7e9031bfef 100644 --- a/compiler/builtins/bitcode/src/lib.rs +++ b/compiler/builtins/bitcode/src/lib.rs @@ -36,3 +36,12 @@ pub fn pow_int_(mut base: i64, mut exp: i64) -> i64 { acc } + +/// Adapted from Rust's core::num module, by the Rust core team, +/// licensed under the Apache License, version 2.0 - https://www.apache.org/licenses/LICENSE-2.0 +/// +/// Thank you, Rust core team! +#[no_mangle] +pub fn is_finite_(num: f64) -> bool { + f64::is_finite(num) +} diff --git a/compiler/builtins/src/std.rs b/compiler/builtins/src/std.rs index acdae6f60a..1c481a0244 100644 --- a/compiler/builtins/src/std.rs +++ b/compiler/builtins/src/std.rs @@ -188,8 +188,8 @@ pub fn types() -> MutMap { ); // addChecked or (+) : Num a, Num a -> Result (Num a) [ IntOverflow ]* - let int_overflow = SolvedType::TagUnion( - vec![(TagName::Global("IntOverflow".into()), vec![])], + let overflow = SolvedType::TagUnion( + vec![(TagName::Global("Overflow".into()), vec![])], Box::new(SolvedType::Wildcard), ); @@ -197,7 +197,7 @@ pub fn types() -> MutMap { Symbol::NUM_ADD_CHECKED, SolvedType::Func( vec![num_type(flex(TVAR1)), num_type(flex(TVAR1))], - Box::new(result_type(num_type(flex(TVAR1)), int_overflow)), + Box::new(result_type(num_type(flex(TVAR1)), overflow)), ), ); diff --git a/compiler/builtins/src/unique.rs b/compiler/builtins/src/unique.rs index a5dfb09b6f..e175ee6bf7 100644 --- a/compiler/builtins/src/unique.rs +++ b/compiler/builtins/src/unique.rs @@ -275,8 +275,8 @@ pub fn types() -> MutMap { }); // addChecked or (+) : Num a, Num a -> Result (Num a) [ IntOverflow ]* - let int_overflow = SolvedType::TagUnion( - vec![(TagName::Global("IntOverflow".into()), vec![])], + let overflow = SolvedType::TagUnion( + vec![(TagName::Global("Overflow".into()), vec![])], Box::new(SolvedType::Wildcard), ); @@ -284,7 +284,7 @@ pub fn types() -> MutMap { let_tvars! { u, v, w, num, result, star }; unique_function( vec![num_type(u, num), num_type(v, num)], - result_type(result, num_type(w, num), lift(star, int_overflow)), + result_type(result, num_type(w, num), lift(star, overflow)), ) }); diff --git a/compiler/can/src/builtins.rs b/compiler/can/src/builtins.rs index f5aa06ef8d..d8fba0d7ba 100644 --- a/compiler/can/src/builtins.rs +++ b/compiler/can/src/builtins.rs @@ -258,7 +258,7 @@ fn num_add_checked(symbol: Symbol, var_store: &mut VarStore) -> Def { // // if arg_3.b then // # overflow - // Err IntOverflow + // Err Overflow // else // # all is well // Ok arg_3.a @@ -281,7 +281,7 @@ fn num_add_checked(symbol: Symbol, var_store: &mut VarStore) -> Def { // overflow! no_region(tag( "Err", - vec![tag("DivByZero", Vec::new(), var_store)], + vec![tag("Overflow", Vec::new(), var_store)], var_store, )), )], diff --git a/compiler/gen/src/llvm/build.rs b/compiler/gen/src/llvm/build.rs index 5b8156a6c9..8386d5b1cc 100644 --- a/compiler/gen/src/llvm/build.rs +++ b/compiler/gen/src/llvm/build.rs @@ -990,8 +990,8 @@ pub fn build_exp_expr<'a, 'ctx, 'env>( } } - // TODO verify that this is required (better safe than sorry) if filler > 0 { + // TODO verify that this is required (better safe than sorry) field_types.push(env.context.i8_type().array_type(filler).into()); } @@ -1381,15 +1381,6 @@ pub fn build_exp_stmt<'a, 'ctx, 'env>( builder.build_conditional_branch(value, then_block, else_block); - // build then block - builder.position_at_end(then_block); - let then_val = build_exp_stmt(env, layout_ids, scope, parent, pass_stmt); - if then_block.get_terminator().is_none() { - builder.build_unconditional_branch(cont_block); - let then_block = builder.get_insert_block().unwrap(); - blocks.push((&then_val, then_block)); - } - // build else block builder.position_at_end(else_block); let else_val = build_exp_stmt(env, layout_ids, scope, parent, fail_stmt); @@ -1399,6 +1390,15 @@ pub fn build_exp_stmt<'a, 'ctx, 'env>( blocks.push((&else_val, else_block)); } + // build then block + builder.position_at_end(then_block); + let then_val = build_exp_stmt(env, layout_ids, scope, parent, pass_stmt); + if then_block.get_terminator().is_none() { + builder.build_unconditional_branch(cont_block); + let then_block = builder.get_insert_block().unwrap(); + blocks.push((&then_val, then_block)); + } + // emit merge block if blocks.is_empty() { // SAFETY there are no other references to this block in this case @@ -2115,7 +2115,7 @@ fn run_low_level<'a, 'ctx, 'env>( list_join(env, inplace, parent, list, outer_list_layout) } NumAbs | NumNeg | NumRound | NumSqrtUnchecked | NumSin | NumCos | NumCeiling | NumFloor - | NumToFloat => { + | NumToFloat | NumIsFinite => { debug_assert_eq!(args.len(), 1); let (arg, arg_layout) = load_symbol_and_layout(env, scope, &args[0]); @@ -2250,6 +2250,7 @@ fn run_low_level<'a, 'ctx, 'env>( ), Float128 | Float64 | Float32 | Float16 => build_float_binop( env, + parent, lhs_arg.into_float_value(), lhs_layout, rhs_arg.into_float_value(), @@ -2504,6 +2505,7 @@ fn call_bitcode_fn<'a, 'ctx, 'env>( fn build_float_binop<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, + parent: FunctionValue<'ctx>, lhs: FloatValue<'ctx>, _lhs_layout: &Layout<'a>, rhs: FloatValue<'ctx>, @@ -2516,9 +2518,60 @@ fn build_float_binop<'a, 'ctx, 'env>( let bd = env.builder; match op { - NumAdd => bd.build_float_add(lhs, rhs, "add_float").into(), + NumAdd => { + let builder = env.builder; + let context = env.context; + + let result = bd.build_float_add(lhs, rhs, "add_float"); + + let is_finite = + call_bitcode_fn(NumIsFinite, env, &[result.into()], "is_finite_").into_int_value(); + + let then_block = context.append_basic_block(parent, "then_block"); + let throw_block = context.append_basic_block(parent, "throw_block"); + + builder.build_conditional_branch(is_finite, then_block, throw_block); + + builder.position_at_end(throw_block); + + throw_exception(env, "float addition overflowed!"); + + builder.position_at_end(then_block); + + result.into() + } + NumAddChecked => { + let builder = env.builder; + let context = env.context; + + let result = bd.build_float_add(lhs, rhs, "add_float"); + + let is_finite = + call_bitcode_fn(NumIsFinite, env, &[result.into()], "is_finite_").into_int_value(); + let is_infinite = bd.build_not(is_finite, "negate"); + + let struct_type = context.struct_type( + &[context.f64_type().into(), context.bool_type().into()], + false, + ); + + let struct_value = { + let v1 = struct_type.const_zero(); + + let v2 = builder + .build_insert_value(v1, result, 0, "set_result") + .unwrap(); + + let v3 = builder + .build_insert_value(v2, is_infinite, 1, "set_is_infinite") + .unwrap(); + + v3.into_struct_value() + }; + + struct_value.into() + } NumAddWrap => unreachable!("wrapping addition is not defined on floats"), - NumAddChecked => bd.build_float_add(lhs, rhs, "add_float_checked").into(), NumSub => bd.build_float_sub(lhs, rhs, "sub_float").into(), NumMul => bd.build_float_mul(lhs, rhs, "mul_float").into(), NumGt => bd.build_float_compare(OGT, lhs, rhs, "float_gt").into(), @@ -2622,6 +2675,7 @@ fn build_float_unary_op<'a, 'ctx, 'env>( env.context.i64_type(), "num_floor", ), + NumIsFinite => call_bitcode_fn(NumIsFinite, env, &[arg.into()], "is_finite_"), _ => { unreachable!("Unrecognized int unary operation: {:?}", op); } @@ -2632,7 +2686,6 @@ fn define_global_str<'a, 'ctx, 'env>( env: &Env<'a, 'ctx, 'env>, message: &str, ) -> inkwell::values::GlobalValue<'ctx> { - let context = env.context; let module = env.module; // hash the name so we don't re-define existing messages @@ -2644,29 +2697,12 @@ fn define_global_str<'a, 'ctx, 'env>( message.hash(&mut hasher); let hash = hasher.finish(); - format!("message_{}", hash) + format!("_Error_message_{}", hash) }; match module.get_global(&name) { Some(current) => current, - None => { - let i8_type = context.i8_type(); - - // define the error message as a global constant - let message_global = - module.add_global(i8_type.array_type(message.len() as u32), None, &name); - - let mut message_bytes = Vec::with_capacity_in(message.len(), env.arena); - - for c in message.chars() { - message_bytes.push(i8_type.const_int(c as u64, false)); - } - - let const_array = i8_type.const_array(&message_bytes); - message_global.set_initializer(&const_array); - - message_global - } + None => unsafe { env.builder.build_global_string(message, name.as_str()) }, } } diff --git a/compiler/gen/tests/gen_num.rs b/compiler/gen/tests/gen_num.rs index 64f79896a0..b468030030 100644 --- a/compiler/gen/tests/gen_num.rs +++ b/compiler/gen/tests/gen_num.rs @@ -701,34 +701,34 @@ mod gen_num { } #[test] - fn add_checked() { + fn int_add_checked() { assert_evals_to!( indoc!( r#" when Num.addChecked 1 2 is - Ok 3 -> True - _ -> False + Ok v -> v + _ -> -1 "# ), - true, - bool + 3, + i64 ); assert_evals_to!( indoc!( r#" when Num.addChecked 9_223_372_036_854_775_807 1 is - Err IntOverflow -> True - _ -> False + Err Overflow -> -1 + Ok v -> v "# ), - true, - bool + -1, + i64 ); } #[test] - fn add_wrap() { + fn int_add_wrap() { assert_evals_to!( indoc!( r#" @@ -741,11 +741,42 @@ mod gen_num { } #[test] + fn float_add_checked_pass() { + assert_evals_to!( + indoc!( + r#" + when Num.addChecked 1.0 0.0 is + Ok v -> v + Err Overflow -> -1.0 + "# + ), + 1.0, + f64 + ); + } + + #[test] + fn float_add_checked_fail() { + assert_evals_to!( + indoc!( + r#" + when Num.addChecked 1.7976931348623157e308 1.7976931348623157e308 is + Err Overflow -> -1 + Ok v -> v + "# + ), + -1.0, + f64 + ); + } + + #[test] + #[should_panic(expected = r#"Roc failed with message: "float addition overflowed!"#)] fn float_overflow() { assert_evals_to!( indoc!( r#" - 1.7976931348623157E+308 + 1 + 1.7976931348623157e308 + 1.7976931348623157e308 "# ), 0.0, diff --git a/compiler/module/src/low_level.rs b/compiler/module/src/low_level.rs index 483f890f9d..2076921ba8 100644 --- a/compiler/module/src/low_level.rs +++ b/compiler/module/src/low_level.rs @@ -42,6 +42,7 @@ pub enum LowLevel { NumCeiling, NumPowInt, NumFloor, + NumIsFinite, Eq, NotEq, And, diff --git a/compiler/mono/src/borrow.rs b/compiler/mono/src/borrow.rs index 4ec5a56aee..efc849f2d2 100644 --- a/compiler/mono/src/borrow.rs +++ b/compiler/mono/src/borrow.rs @@ -526,6 +526,6 @@ pub fn lowlevel_borrow_signature(arena: &Bump, op: LowLevel) -> &[bool] { | NumPowInt => arena.alloc_slice_copy(&[irrelevant, irrelevant]), NumAbs | NumNeg | NumSin | NumCos | NumSqrtUnchecked | NumRound | NumCeiling | NumFloor - | NumToFloat | Not => arena.alloc_slice_copy(&[irrelevant]), + | NumToFloat | Not | NumIsFinite => arena.alloc_slice_copy(&[irrelevant]), } } From b42a49035afbf11e5856236cc069eb6187d34600 Mon Sep 17 00:00:00 2001 From: Folkert Date: Mon, 21 Sep 2020 23:52:04 +0200 Subject: [PATCH 06/10] fix some comments --- compiler/can/src/builtins.rs | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/compiler/can/src/builtins.rs b/compiler/can/src/builtins.rs index d8fba0d7ba..de7074f154 100644 --- a/compiler/can/src/builtins.rs +++ b/compiler/can/src/builtins.rs @@ -269,7 +269,7 @@ fn num_add_checked(symbol: Symbol, var_store: &mut VarStore) -> Def { branches: vec![( // if-condition no_region( - // Num.neq denominator 0 + // arg_3.b Access { record_var, ext_var: var_store.fresh(), @@ -288,11 +288,11 @@ fn num_add_checked(symbol: Symbol, var_store: &mut VarStore) -> Def { final_else: Box::new( // all is well no_region( - // Ok (Float.#divUnchecked numerator denominator) + // Ok arg_3.a tag( "Ok", vec![ - // Num.#divUnchecked numerator denominator + // arg_3.a Access { record_var, ext_var: var_store.fresh(), @@ -307,19 +307,17 @@ fn num_add_checked(symbol: Symbol, var_store: &mut VarStore) -> Def { ), }; + // arg_3 = RunLowLevel NumAddChecked arg_1 arg_2 let def = crate::def::Def { loc_pattern: no_region(Pattern::Identifier(Symbol::ARG_3)), - loc_expr: no_region( - // Num.neq denominator 0 - RunLowLevel { - op: LowLevel::NumAddChecked, - args: vec![ - (num_var_1, Var(Symbol::ARG_1)), - (num_var_2, Var(Symbol::ARG_2)), - ], - ret_var: record_var, - }, - ), + loc_expr: no_region(RunLowLevel { + op: LowLevel::NumAddChecked, + args: vec![ + (num_var_1, Var(Symbol::ARG_1)), + (num_var_2, Var(Symbol::ARG_2)), + ], + ret_var: record_var, + }), expr_var: record_var, pattern_vars: SendMap::default(), annotation: None, From 515f7175363a48df56863129fd84157fe82915d2 Mon Sep 17 00:00:00 2001 From: Folkert Date: Mon, 21 Sep 2020 23:55:05 +0200 Subject: [PATCH 07/10] don't explicitly add filler bytes leaving the memory undefined is fine --- compiler/gen/src/llvm/build.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/compiler/gen/src/llvm/build.rs b/compiler/gen/src/llvm/build.rs index 8386d5b1cc..4408f35a4c 100644 --- a/compiler/gen/src/llvm/build.rs +++ b/compiler/gen/src/llvm/build.rs @@ -947,8 +947,6 @@ pub fn build_exp_expr<'a, 'ctx, 'env>( debug_assert!(*union_size > 1); let ptr_size = env.ptr_bytes; - let mut filler = tag_layout.stack_size(ptr_size); - let ctx = env.context; let builder = env.builder; @@ -985,16 +983,9 @@ pub fn build_exp_expr<'a, 'ctx, 'env>( } else { field_vals.push(val); } - - filler -= field_size; } } - if filler > 0 { - // TODO verify that this is required (better safe than sorry) - field_types.push(env.context.i8_type().array_type(filler).into()); - } - // Create the struct_type let struct_type = ctx.struct_type(field_types.into_bump_slice(), false); let mut struct_val = struct_type.const_zero().into(); From 9ac5533f8051ab9a5b514cd6111cea95c746c6a8 Mon Sep 17 00:00:00 2001 From: Folkert Date: Mon, 21 Sep 2020 23:57:05 +0200 Subject: [PATCH 08/10] final touchups --- compiler/gen/src/llvm/build.rs | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/compiler/gen/src/llvm/build.rs b/compiler/gen/src/llvm/build.rs index 4408f35a4c..f5be0c775b 100644 --- a/compiler/gen/src/llvm/build.rs +++ b/compiler/gen/src/llvm/build.rs @@ -1372,15 +1372,6 @@ pub fn build_exp_stmt<'a, 'ctx, 'env>( builder.build_conditional_branch(value, then_block, else_block); - // build else block - builder.position_at_end(else_block); - let else_val = build_exp_stmt(env, layout_ids, scope, parent, fail_stmt); - if else_block.get_terminator().is_none() { - let else_block = builder.get_insert_block().unwrap(); - builder.build_unconditional_branch(cont_block); - blocks.push((&else_val, else_block)); - } - // build then block builder.position_at_end(then_block); let then_val = build_exp_stmt(env, layout_ids, scope, parent, pass_stmt); @@ -1390,6 +1381,15 @@ pub fn build_exp_stmt<'a, 'ctx, 'env>( blocks.push((&then_val, then_block)); } + // build else block + builder.position_at_end(else_block); + let else_val = build_exp_stmt(env, layout_ids, scope, parent, fail_stmt); + if else_block.get_terminator().is_none() { + let else_block = builder.get_insert_block().unwrap(); + builder.build_unconditional_branch(cont_block); + blocks.push((&else_val, else_block)); + } + // emit merge block if blocks.is_empty() { // SAFETY there are no other references to this block in this case @@ -2427,7 +2427,6 @@ fn build_int_binop<'a, 'ctx, 'env>( match op { NumAdd => { - // TODO let builder = env.builder; let context = env.context; From 9381207f6158347c8dd59f2db1245512944cf7dc Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Mon, 21 Sep 2020 21:26:56 -0400 Subject: [PATCH 09/10] Clarify some comments --- compiler/builtins/src/std.rs | 4 ++-- compiler/builtins/src/unique.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/builtins/src/std.rs b/compiler/builtins/src/std.rs index 1c481a0244..a2d3088482 100644 --- a/compiler/builtins/src/std.rs +++ b/compiler/builtins/src/std.rs @@ -187,7 +187,7 @@ pub fn types() -> MutMap { ), ); - // addChecked or (+) : Num a, Num a -> Result (Num a) [ IntOverflow ]* + // addChecked : Num a, Num a -> Result (Num a) [ IntOverflow ]* let overflow = SolvedType::TagUnion( vec![(TagName::Global("Overflow".into()), vec![])], Box::new(SolvedType::Wildcard), @@ -201,7 +201,7 @@ pub fn types() -> MutMap { ), ); - // addWrap or (+) : Int, Int -> Int + // addWrap : Int, Int -> Int add_type( Symbol::NUM_ADD_WRAP, SolvedType::Func(vec![int_type(), int_type()], Box::new(int_type())), diff --git a/compiler/builtins/src/unique.rs b/compiler/builtins/src/unique.rs index e175ee6bf7..4c13ab1716 100644 --- a/compiler/builtins/src/unique.rs +++ b/compiler/builtins/src/unique.rs @@ -274,7 +274,7 @@ pub fn types() -> MutMap { unique_function(vec![num_type(u, num), num_type(v, num)], num_type(w, num)) }); - // addChecked or (+) : Num a, Num a -> Result (Num a) [ IntOverflow ]* + // addChecked : Num a, Num a -> Result (Num a) [ IntOverflow ]* let overflow = SolvedType::TagUnion( vec![(TagName::Global("Overflow".into()), vec![])], Box::new(SolvedType::Wildcard), @@ -288,7 +288,7 @@ pub fn types() -> MutMap { ) }); - // addWrap or (+) : Int, Int -> Int + // addWrap : Int, Int -> Int add_type(Symbol::NUM_ADD_WRAP, { let_tvars! { u, v, w }; unique_function(vec![int_type(u), int_type(v)], int_type(w)) From 94b17609657bff028fb17a36ecec2e899b919255 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Mon, 21 Sep 2020 21:27:04 -0400 Subject: [PATCH 10/10] Remove redundant variable --- compiler/gen/src/llvm/build.rs | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/compiler/gen/src/llvm/build.rs b/compiler/gen/src/llvm/build.rs index f5be0c775b..c05bb4fe6c 100644 --- a/compiler/gen/src/llvm/build.rs +++ b/compiler/gen/src/llvm/build.rs @@ -2427,9 +2427,7 @@ fn build_int_binop<'a, 'ctx, 'env>( match op { NumAdd => { - let builder = env.builder; let context = env.context; - let result = env .call_intrinsic(LLVM_SADD_WITH_OVERFLOW_I64, &[lhs.into(), rhs.into()]) .into_struct_value(); @@ -2437,7 +2435,7 @@ fn build_int_binop<'a, 'ctx, 'env>( let add_result = bd.build_extract_value(result, 0, "add_result").unwrap(); let has_overflowed = bd.build_extract_value(result, 1, "has_overflowed").unwrap(); - let condition = builder.build_int_compare( + let condition = bd.build_int_compare( IntPredicate::EQ, has_overflowed.into_int_value(), context.bool_type().const_zero(), @@ -2447,13 +2445,13 @@ fn build_int_binop<'a, 'ctx, 'env>( let then_block = context.append_basic_block(parent, "then_block"); let throw_block = context.append_basic_block(parent, "throw_block"); - builder.build_conditional_branch(condition, then_block, throw_block); + bd.build_conditional_branch(condition, then_block, throw_block); - builder.position_at_end(throw_block); + bd.position_at_end(throw_block); throw_exception(env, "integer addition overflowed!"); - builder.position_at_end(then_block); + bd.position_at_end(then_block); add_result } @@ -2531,7 +2529,6 @@ fn build_float_binop<'a, 'ctx, 'env>( result.into() } NumAddChecked => { - let builder = env.builder; let context = env.context; let result = bd.build_float_add(lhs, rhs, "add_float"); @@ -2547,12 +2544,8 @@ fn build_float_binop<'a, 'ctx, 'env>( let struct_value = { let v1 = struct_type.const_zero(); - - let v2 = builder - .build_insert_value(v1, result, 0, "set_result") - .unwrap(); - - let v3 = builder + let v2 = bd.build_insert_value(v1, result, 0, "set_result").unwrap(); + let v3 = bd .build_insert_value(v2, is_infinite, 1, "set_is_infinite") .unwrap();