make decimal math ops correctly report overflow

compiler/builtins/bitcode/build.zig

@@ -39,7 +39,7 @@ pub fn build(b: *Builder) void {
     obj.linkSystemLibrary("c");
     obj.setOutputDir(".");
     obj.strip = true;
-    obj.bundle_compiler_rt = true;
+    // obj.bundle_compiler_rt = true;
     const obj_step = b.step("object", "Build object file for linking");
     obj_step.dependOn(&obj.step);
 

compiler/builtins/bitcode/src/dec.zig

@@ -1,9 +1,11 @@
 const std = @import("std");
 const str = @import("str.zig");
+const utils = @import("utils.zig");
 
 const math = std.math;
 const always_inline = std.builtin.CallOptions.Modifier.always_inline;
 const RocStr = str.RocStr;
+const WithOverflow = utils.WithOverflow;
 
 pub const RocDec = extern struct {
     num: i128,
@@ -215,29 +217,41 @@ pub const RocDec = extern struct {
         return if (negated) |n| .{ .num = n } else null;
     }
 
-    pub fn add(self: RocDec, other: RocDec) RocDec {
+    pub fn addWithOverflow(self: RocDec, other: RocDec) WithOverflow(RocDec) {
         var answer: i128 = undefined;
         const overflowed = @addWithOverflow(i128, self.num, other.num, &answer);
 
-        if (!overflowed) {
+        return .{ .value = RocDec{ .num = answer }, .has_overflowed = overflowed };
-            return RocDec{ .num = answer };
+    }
-        } else {
+
+    pub fn add(self: RocDec, other: RocDec) RocDec {
+        const answer = RocDec.addWithOverflow(self, other);
+
+        if (answer.has_overflowed) {
             @panic("TODO runtime exception for overflow!");
+        } else {
+            return answer.value;
         }
     }
 
-    pub fn sub(self: RocDec, other: RocDec) RocDec {
+    pub fn subWithOverflow(self: RocDec, other: RocDec) WithOverflow(RocDec) {
         var answer: i128 = undefined;
         const overflowed = @subWithOverflow(i128, self.num, other.num, &answer);
 
-        if (!overflowed) {
+        return .{ .value = RocDec{ .num = answer }, .has_overflowed = overflowed };
-            return RocDec{ .num = answer };
+    }
-        } else {
+
+    pub fn sub(self: RocDec, other: RocDec) RocDec {
+        const answer = RocDec.subWithOverflow(self, other);
+
+        if (answer.has_overflowed) {
             @panic("TODO runtime exception for overflow!");
+        } else {
+            return answer.value;
         }
     }
 
-    pub fn mul(self: RocDec, other: RocDec) RocDec {
+    pub fn mulWithOverflow(self: RocDec, other: RocDec) WithOverflow(RocDec) {
         const self_i128 = self.num;
         const other_i128 = other.num;
         // const answer = 0; //self_i256 * other_i256;
@@ -246,30 +260,40 @@ pub const RocDec = extern struct {
 
         const self_u128 = @intCast(u128, math.absInt(self_i128) catch {
             if (other_i128 == 0) {
-                return .{ .num = 0 };
+                return .{ .value = RocDec{ .num = 0 }, .has_overflowed = false };
             } else if (other_i128 == RocDec.one_point_zero.num) {
-                return self;
+                return .{ .value = self, .has_overflowed = false };
             } else {
-                @panic("TODO runtime exception for overflow!");
+                return .{ .value = undefined, .has_overflowed = true };
             }
         });
 
         const other_u128 = @intCast(u128, math.absInt(other_i128) catch {
             if (self_i128 == 0) {
-                return .{ .num = 0 };
+                return .{ .value = RocDec{ .num = 0 }, .has_overflowed = false };
             } else if (self_i128 == RocDec.one_point_zero.num) {
-                return other;
+                return .{ .value = other, .has_overflowed = false };
             } else {
-                @panic("TODO runtime exception for overflow!");
+                return .{ .value = undefined, .has_overflowed = true };
             }
         });
 
         const unsigned_answer: i128 = mul_and_decimalize(self_u128, other_u128);
 
         if (is_answer_negative) {
-            return .{ .num = -unsigned_answer };
+            return .{ .value = RocDec{ .num = -unsigned_answer }, .has_overflowed = false };
         } else {
-            return .{ .num = unsigned_answer };
+            return .{ .value = RocDec{ .num = unsigned_answer }, .has_overflowed = false };
+        }
+    }
+
+    pub fn mul(self: RocDec, other: RocDec) RocDec {
+        const answer = RocDec.mulWithOverflow(self, other);
+
+        if (answer.has_overflowed) {
+            @panic("TODO runtime exception for overflow!");
+        } else {
+            return answer.value;
         }
     }
 
@@ -1039,16 +1063,16 @@ pub fn negateC(arg: RocDec) callconv(.C) i128 {
     return if (@call(.{ .modifier = always_inline }, RocDec.negate, .{arg})) |dec| dec.num else @panic("TODO overflow for negating RocDec");
 }
 
-pub fn addC(arg1: RocDec, arg2: RocDec) callconv(.C) i128 {
+pub fn addC(arg1: RocDec, arg2: RocDec) callconv(.C) WithOverflow(RocDec) {
-    return @call(.{ .modifier = always_inline }, RocDec.add, .{ arg1, arg2 }).num;
+    return @call(.{ .modifier = always_inline }, RocDec.addWithOverflow, .{ arg1, arg2 });
 }
 
-pub fn subC(arg1: RocDec, arg2: RocDec) callconv(.C) i128 {
+pub fn subC(arg1: RocDec, arg2: RocDec) callconv(.C) WithOverflow(RocDec) {
-    return @call(.{ .modifier = always_inline }, RocDec.sub, .{ arg1, arg2 }).num;
+    return @call(.{ .modifier = always_inline }, RocDec.subWithOverflow, .{ arg1, arg2 });
 }
 
-pub fn mulC(arg1: RocDec, arg2: RocDec) callconv(.C) i128 {
+pub fn mulC(arg1: RocDec, arg2: RocDec) callconv(.C) WithOverflow(RocDec) {
-    return @call(.{ .modifier = always_inline }, RocDec.mul, .{ arg1, arg2 }).num;
+    return @call(.{ .modifier = always_inline }, RocDec.mulWithOverflow, .{ arg1, arg2 });
 }
 
 pub fn divC(arg1: RocDec, arg2: RocDec) callconv(.C) i128 {

compiler/builtins/bitcode/src/main.zig

@@ -10,9 +10,9 @@ comptime {
     exportDecFn(dec.eqC, "eq");
     exportDecFn(dec.neqC, "neq");
     exportDecFn(dec.negateC, "negate");
-    exportDecFn(dec.addC, "add");
+    exportDecFn(dec.addC, "add_with_overflow");
-    exportDecFn(dec.subC, "sub");
+    exportDecFn(dec.subC, "sub_with_overflow");
-    exportDecFn(dec.mulC, "mul");
+    exportDecFn(dec.mulC, "mul_with_overflow");
     exportDecFn(dec.divC, "div");
 }
 

compiler/builtins/bitcode/src/utils.zig

@@ -1,6 +1,10 @@
 const std = @import("std");
 const always_inline = std.builtin.CallOptions.Modifier.always_inline;
 
+pub fn WithOverflow(comptime T: type) type {
+    return extern struct { value: T, has_overflowed: bool };
+}
+
 // If allocation fails, this must cxa_throw - it must not return a null pointer!
 extern fn roc_alloc(size: usize, alignment: u32) callconv(.C) ?*c_void;
 

compiler/builtins/src/bitcode.rs

@@ -71,7 +71,7 @@ pub const DEC_FROM_F64: &str = "roc_builtins.dec.from_f64";
 pub const DEC_EQ: &str = "roc_builtins.dec.eq";
 pub const DEC_NEQ: &str = "roc_builtins.dec.neq";
 pub const DEC_NEGATE: &str = "roc_builtins.dec.negate";
-pub const DEC_ADD: &str = "roc_builtins.dec.add";
+pub const DEC_ADD_WITH_OVERFLOW: &str = "roc_builtins.dec.add_with_overflow";
-pub const DEC_SUB: &str = "roc_builtins.dec.sub";
+pub const DEC_SUB_WITH_OVERFLOW: &str = "roc_builtins.dec.sub_with_overflow";
-pub const DEC_MUL: &str = "roc_builtins.dec.mul";
+pub const DEC_MUL_WITH_OVERFLOW: &str = "roc_builtins.dec.mul_with_overflow";
 pub const DEC_DIV: &str = "roc_builtins.dec.div";

compiler/gen_llvm/src/llvm/build.rs

@@ -1,6 +1,6 @@
 use std::path::Path;
 
-use crate::llvm::bitcode::call_bitcode_fn;
+use crate::llvm::bitcode::{call_bitcode_fn, call_void_bitcode_fn};
 use crate::llvm::build_dict::{
     dict_contains, dict_difference, dict_empty, dict_get, dict_insert, dict_intersection,
     dict_keys, dict_len, dict_remove, dict_union, dict_values, dict_walk, set_from_list,
@@ -362,12 +362,8 @@ fn add_intrinsics<'ctx>(ctx: &'ctx Context, module: &Module<'ctx>) {
     // List of all supported LLVM intrinsics:
     //
     // https://releases.llvm.org/10.0.0/docs/LangRef.html#standard-c-library-intrinsics
-    let i1_type = ctx.bool_type();
     let f64_type = ctx.f64_type();
     let i64_type = ctx.i64_type();
-    let i32_type = ctx.i32_type();
-    let i16_type = ctx.i16_type();
-    let i8_type = ctx.i8_type();
 
     add_intrinsic(
         module,
@@ -416,60 +412,6 @@ fn add_intrinsics<'ctx>(ctx: &'ctx Context, module: &Module<'ctx>) {
         LLVM_FLOOR_F64,
         f64_type.fn_type(&[f64_type.into()], false),
     );
-
-    // add with overflow
-
-    add_intrinsic(module, LLVM_SADD_WITH_OVERFLOW_I8, {
-        let fields = [i8_type.into(), i1_type.into()];
-        ctx.struct_type(&fields, false)
-            .fn_type(&[i8_type.into(), i8_type.into()], false)
-    });
-
-    add_intrinsic(module, LLVM_SADD_WITH_OVERFLOW_I16, {
-        let fields = [i16_type.into(), i1_type.into()];
-        ctx.struct_type(&fields, false)
-            .fn_type(&[i16_type.into(), i16_type.into()], false)
-    });
-
-    add_intrinsic(module, LLVM_SADD_WITH_OVERFLOW_I32, {
-        let fields = [i32_type.into(), i1_type.into()];
-        ctx.struct_type(&fields, false)
-            .fn_type(&[i32_type.into(), i32_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)
-    });
-    // LLVM_SADD_WITH_OVERFLOW_I128 is exported in bitcode
-
-    // sub with overflow
-
-    add_intrinsic(module, LLVM_SSUB_WITH_OVERFLOW_I8, {
-        let fields = [i8_type.into(), i1_type.into()];
-        ctx.struct_type(&fields, false)
-            .fn_type(&[i8_type.into(), i8_type.into()], false)
-    });
-
-    add_intrinsic(module, LLVM_SSUB_WITH_OVERFLOW_I16, {
-        let fields = [i16_type.into(), i1_type.into()];
-        ctx.struct_type(&fields, false)
-            .fn_type(&[i16_type.into(), i16_type.into()], false)
-    });
-
-    add_intrinsic(module, LLVM_SSUB_WITH_OVERFLOW_I32, {
-        let fields = [i32_type.into(), i1_type.into()];
-        ctx.struct_type(&fields, false)
-            .fn_type(&[i32_type.into(), i32_type.into()], false)
-    });
-
-    add_intrinsic(module, LLVM_SSUB_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)
-    });
-    // LLVM_SSUB_WITH_OVERFLOW_I128 is exported in bitcode
 }
 
 static LLVM_MEMSET_I64: &str = "llvm.memset.p0i8.i64";
@@ -5251,6 +5193,39 @@ fn build_foreign_symbol<'a, 'ctx, 'env>(
     }
 }
 
+fn throw_on_overflow<'a, 'ctx, 'env>(
+    env: &Env<'a, 'ctx, 'env>,
+    parent: FunctionValue<'ctx>,
+    result: StructValue<'ctx>, // of the form { value: T, has_overflowed: bool }
+    message: &str,
+) -> BasicValueEnum<'ctx> {
+    let bd = env.builder;
+    let context = env.context;
+
+    let has_overflowed = bd.build_extract_value(result, 1, "has_overflowed").unwrap();
+
+    let condition = bd.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");
+
+    bd.build_conditional_branch(condition, then_block, throw_block);
+
+    bd.position_at_end(throw_block);
+
+    throw_exception(env, message);
+
+    bd.position_at_end(then_block);
+
+    bd.build_extract_value(result, 0, "operation_result")
+        .unwrap()
+}
+
 fn build_int_binop<'a, 'ctx, 'env>(
     env: &Env<'a, 'ctx, 'env>,
     parent: FunctionValue<'ctx>,
@@ -5267,8 +5242,6 @@ fn build_int_binop<'a, 'ctx, 'env>(
 
     match op {
         NumAdd => {
-            let context = env.context;
-
             let intrinsic = match lhs_layout {
                 Layout::Builtin(Builtin::Int8) => LLVM_SADD_WITH_OVERFLOW_I8,
                 Layout::Builtin(Builtin::Int16) => LLVM_SADD_WITH_OVERFLOW_I16,
@@ -5287,34 +5260,11 @@ fn build_int_binop<'a, 'ctx, 'env>(
                 .call_intrinsic(intrinsic, &[lhs.into(), rhs.into()])
                 .into_struct_value();
 
-            let add_result = bd.build_extract_value(result, 0, "add_result").unwrap();
+            throw_on_overflow(env, parent, result, "integer addition overflowed!")
-            let has_overflowed = bd.build_extract_value(result, 1, "has_overflowed").unwrap();
-
-            let condition = bd.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");
-
-            bd.build_conditional_branch(condition, then_block, throw_block);
-
-            bd.position_at_end(throw_block);
-
-            throw_exception(env, "integer addition overflowed!");
-
-            bd.position_at_end(then_block);
-
-            add_result
         }
         NumAddWrap => bd.build_int_add(lhs, rhs, "add_int_wrap").into(),
         NumAddChecked => env.call_intrinsic(LLVM_SADD_WITH_OVERFLOW_I64, &[lhs.into(), rhs.into()]),
         NumSub => {
-            let context = env.context;
-
             let intrinsic = match lhs_layout {
                 Layout::Builtin(Builtin::Int8) => LLVM_SSUB_WITH_OVERFLOW_I8,
                 Layout::Builtin(Builtin::Int16) => LLVM_SSUB_WITH_OVERFLOW_I16,
@@ -5333,59 +5283,16 @@ fn build_int_binop<'a, 'ctx, 'env>(
                 .call_intrinsic(intrinsic, &[lhs.into(), rhs.into()])
                 .into_struct_value();
 
-            let sub_result = bd.build_extract_value(result, 0, "sub_result").unwrap();
+            throw_on_overflow(env, parent, result, "integer subtraction overflowed!")
-            let has_overflowed = bd.build_extract_value(result, 1, "has_overflowed").unwrap();
-
-            let condition = bd.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");
-
-            bd.build_conditional_branch(condition, then_block, throw_block);
-
-            bd.position_at_end(throw_block);
-
-            throw_exception(env, "integer subtraction overflowed!");
-
-            bd.position_at_end(then_block);
-
-            sub_result
         }
         NumSubWrap => bd.build_int_sub(lhs, rhs, "sub_int").into(),
         NumSubChecked => env.call_intrinsic(LLVM_SSUB_WITH_OVERFLOW_I64, &[lhs.into(), rhs.into()]),
         NumMul => {
-            let context = env.context;
             let result = env
                 .call_intrinsic(LLVM_SMUL_WITH_OVERFLOW_I64, &[lhs.into(), rhs.into()])
                 .into_struct_value();
 
-            let mul_result = bd.build_extract_value(result, 0, "mul_result").unwrap();
+            throw_on_overflow(env, parent, result, "integer multiplication overflowed!")
-            let has_overflowed = bd.build_extract_value(result, 1, "has_overflowed").unwrap();
-
-            let condition = bd.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");
-
-            bd.build_conditional_branch(condition, then_block, throw_block);
-
-            bd.position_at_end(throw_block);
-
-            throw_exception(env, "integer multiplication overflowed!");
-
-            bd.position_at_end(then_block);
-
-            mul_result
         }
         NumMulWrap => bd.build_int_mul(lhs, rhs, "mul_int").into(),
         NumMulChecked => env.call_intrinsic(LLVM_SMUL_WITH_OVERFLOW_I64, &[lhs.into(), rhs.into()]),
@@ -5715,7 +5622,7 @@ fn build_float_binop<'a, 'ctx, 'env>(
 
 fn build_dec_binop<'a, 'ctx, 'env>(
     env: &Env<'a, 'ctx, 'env>,
-    _parent: FunctionValue<'ctx>,
+    parent: FunctionValue<'ctx>,
     lhs: BasicValueEnum<'ctx>,
     _lhs_layout: &Layout<'a>,
     rhs: BasicValueEnum<'ctx>,
@@ -5725,9 +5632,33 @@ fn build_dec_binop<'a, 'ctx, 'env>(
     use roc_module::low_level::LowLevel::*;
 
     match op {
-        NumAdd => call_bitcode_fn(env, &[lhs, rhs], &bitcode::DEC_ADD),
+        NumAddChecked => call_bitcode_fn(env, &[lhs, rhs], &bitcode::DEC_ADD_WITH_OVERFLOW),
-        NumSub => call_bitcode_fn(env, &[lhs, rhs], &bitcode::DEC_SUB),
+        NumSubChecked => call_bitcode_fn(env, &[lhs, rhs], &bitcode::DEC_SUB_WITH_OVERFLOW),
-        NumMul => call_bitcode_fn(env, &[lhs, rhs], &bitcode::DEC_MUL),
+        NumMulChecked => call_bitcode_fn(env, &[lhs, rhs], &bitcode::DEC_MUL_WITH_OVERFLOW),
+        NumAdd => build_dec_binop_throw_on_overflow(
+            env,
+            parent,
+            bitcode::DEC_ADD_WITH_OVERFLOW,
+            lhs,
+            rhs,
+            "decimal addition overflowed",
+        ),
+        NumSub => build_dec_binop_throw_on_overflow(
+            env,
+            parent,
+            bitcode::DEC_SUB_WITH_OVERFLOW,
+            lhs,
+            rhs,
+            "decimal subtraction overflowed",
+        ),
+        NumMul => build_dec_binop_throw_on_overflow(
+            env,
+            parent,
+            bitcode::DEC_MUL_WITH_OVERFLOW,
+            lhs,
+            rhs,
+            "decimal multiplication overflowed",
+        ),
         NumDivUnchecked => call_bitcode_fn(env, &[lhs, rhs], &bitcode::DEC_DIV),
         _ => {
             unreachable!("Unrecognized int binary operation: {:?}", op);
@@ -5735,6 +5666,29 @@ fn build_dec_binop<'a, 'ctx, 'env>(
     }
 }
 
+fn build_dec_binop_throw_on_overflow<'a, 'ctx, 'env>(
+    env: &Env<'a, 'ctx, 'env>,
+    parent: FunctionValue<'ctx>,
+    operation: &str,
+    lhs: BasicValueEnum<'ctx>,
+    rhs: BasicValueEnum<'ctx>,
+    message: &str,
+) -> BasicValueEnum<'ctx> {
+    let overflow_type = crate::llvm::convert::zig_with_overflow_roc_dec(env);
+
+    let result_ptr = env.builder.build_alloca(overflow_type, "result_ptr");
+    call_void_bitcode_fn(env, &[result_ptr.into(), lhs, rhs], operation);
+
+    let result = env
+        .builder
+        .build_load(result_ptr, "load_overflow")
+        .into_struct_value();
+
+    let value = throw_on_overflow(env, parent, result, message).into_struct_value();
+
+    env.builder.build_extract_value(value, 0, "num").unwrap()
+}
+
 fn int_type_signed_min(int_type: IntType) -> IntValue {
     let width = int_type.get_bit_width();
 

compiler/gen_llvm/src/llvm/convert.rs

@@ -222,3 +222,11 @@ pub fn zig_has_tag_id_type<'a, 'ctx, 'env>(
 ) -> StructType<'ctx> {
     env.module.get_struct_type("list.HasTagId").unwrap()
 }
+
+pub fn zig_with_overflow_roc_dec<'a, 'ctx, 'env>(
+    env: &crate::llvm::build::Env<'a, 'ctx, 'env>,
+) -> StructType<'ctx> {
+    env.module
+        .get_struct_type("utils.WithOverflow(dec.RocDec)")
+        .unwrap()
+}

compiler/test_gen/src/gen_num.rs

@@ -752,7 +752,7 @@ mod gen_num {
                     z : Dec
                     z = 3
 
-                    x - y - z
+                    (x - y) - z
                 "#
             ),
             RocDec::from_str_to_i128_unsafe(&"-3.9"),