Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update zig @panic calls to roc_panic for numeric errors #6062

Merged
merged 17 commits into from
Dec 5, 2023
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 14 additions & 13 deletions crates/compiler/builtins/bitcode/src/dec.zig
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,6 @@ pub const RocDec = extern struct {

if (answer.has_overflowed) {
roc_panic("Decimal addition overflowed!", 0);
unreachable;
} else {
return answer.value;
}
Expand Down Expand Up @@ -283,7 +282,6 @@ pub const RocDec = extern struct {

if (answer.has_overflowed) {
roc_panic("Decimal subtraction overflowed!", 0);
unreachable;
} else {
return answer.value;
}
Expand Down Expand Up @@ -347,7 +345,6 @@ pub const RocDec = extern struct {

if (answer.has_overflowed) {
roc_panic("Decimal multiplication overflowed!", 0);
unreachable;
} else {
return answer.value;
}
Expand All @@ -369,7 +366,7 @@ pub const RocDec = extern struct {

// (n / 0) is an error
if (denominator_i128 == 0) {
@panic("TODO runtime exception for dividing by 0!");
roc_panic("Decimal division by 0!", 0);
}

// If they're both negative, or if neither is negative, the final answer
Expand Down Expand Up @@ -397,7 +394,7 @@ pub const RocDec = extern struct {
if (denominator_i128 == one_point_zero_i128) {
return self;
} else {
@panic("TODO runtime exception for overflow when dividing!");
roc_panic("Decimal division overflow in numerator!", 0);
}
};
const numerator_u128 = @as(u128, @intCast(numerator_abs_i128));
Expand All @@ -410,7 +407,7 @@ pub const RocDec = extern struct {
if (numerator_i128 == one_point_zero_i128) {
return other;
} else {
@panic("TODO runtime exception for overflow when dividing!");
roc_panic("Decimal division overflow in denominator!", 0);
}
};
const denominator_u128 = @as(u128, @intCast(denominator_abs_i128));
Expand All @@ -422,7 +419,7 @@ pub const RocDec = extern struct {
if (answer.hi == 0 and answer.lo <= math.maxInt(i128)) {
unsigned_answer = @as(i128, @intCast(answer.lo));
} else {
@panic("TODO runtime exception for overflow when dividing!");
roc_panic("Decimal division overflow!", 0);
}

return RocDec{ .num = if (is_answer_negative) -unsigned_answer else unsigned_answer };
Expand Down Expand Up @@ -632,7 +629,7 @@ fn mul_and_decimalize(a: u128, b: u128) i128 {
const d = answer[0];

if (overflowed == 1) {
@panic("TODO runtime exception for overflow!");
roc_panic("Decimal multiplication overflow!", 0);
}

// Final 512bit value is d, c, b, a
Expand Down Expand Up @@ -1208,7 +1205,7 @@ pub fn fromF64C(arg: f64) callconv(.C) i128 {
if (@call(.always_inline, RocDec.fromF64, .{arg})) |dec| {
return dec.num;
} else {
@panic("TODO runtime exception failing convert f64 to RocDec");
roc_panic("Decimal conversion from f64 failed!", 0);
}
}

Expand All @@ -1217,7 +1214,7 @@ pub fn fromF32C(arg_f32: f32) callconv(.C) i128 {
if (@call(.always_inline, RocDec.fromF64, .{arg_f64})) |dec| {
return dec.num;
} else {
@panic("TODO runtime exception failing convert f64 to RocDec");
roc_panic("Decimal conversion from f32!", 0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
roc_panic("Decimal conversion from f32!", 0);
roc_panic("Decimal conversion from f32 failed!", 0);

}
}

Expand All @@ -1232,7 +1229,7 @@ pub fn exportFromInt(comptime T: type, comptime name: []const u8) void {

const answer = @mulWithOverflow(this, RocDec.one_point_zero_i128);
if (answer[1] == 1) {
@panic("TODO runtime exception failing convert integer to RocDec");
roc_panic("Decimal conversion from Integer failed!", 0);
} else {
return answer[0];
}
Expand All @@ -1258,11 +1255,15 @@ pub fn neqC(arg1: RocDec, arg2: RocDec) callconv(.C) bool {
}

pub fn negateC(arg: RocDec) callconv(.C) i128 {
return if (@call(.always_inline, RocDec.negate, .{arg})) |dec| dec.num else @panic("TODO overflow for negating RocDec");
return if (@call(.always_inline, RocDec.negate, .{arg})) |dec| dec.num else {
roc_panic("Decimal negation overflow!", 0);
};
}

pub fn absC(arg: RocDec) callconv(.C) i128 {
const result = @call(.always_inline, RocDec.abs, .{arg}) catch @panic("TODO overflow for calling absolute value on RocDec");
const result = @call(.always_inline, RocDec.abs, .{arg}) catch {
roc_panic("Decimal absolute value overflow!", 0);
};
return result.num;
}

Expand Down
13 changes: 6 additions & 7 deletions crates/compiler/builtins/bitcode/src/num.zig
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,9 @@ pub fn exportCeiling(comptime F: type, comptime T: type, comptime name: []const
pub fn exportDivCeil(comptime T: type, comptime name: []const u8) void {
comptime var f = struct {
fn func(a: T, b: T) callconv(.C) T {
return math.divCeil(T, a, b) catch @panic("TODO runtime exception for dividing by 0!");
return math.divCeil(T, a, b) catch {
roc_panic("Integer division by 0!", 0);
};
}
}.func;
@export(f, .{ .name = name ++ @typeName(T), .linkage = .Strong });
Expand Down Expand Up @@ -379,8 +381,7 @@ pub fn exportAddOrPanic(comptime T: type, comptime name: []const u8) void {
fn func(self: T, other: T) callconv(.C) T {
const result = addWithOverflow(T, self, other);
if (result.has_overflowed) {
roc_panic("integer addition overflowed!", 0);
unreachable;
roc_panic("Integer addition overflowed!", 0);
} else {
return result.value;
}
Expand Down Expand Up @@ -437,8 +438,7 @@ pub fn exportSubOrPanic(comptime T: type, comptime name: []const u8) void {
fn func(self: T, other: T) callconv(.C) T {
const result = subWithOverflow(T, self, other);
if (result.has_overflowed) {
roc_panic("integer subtraction overflowed!", 0);
unreachable;
roc_panic("Integer subtraction overflowed!", 0);
} else {
return result.value;
}
Expand Down Expand Up @@ -622,8 +622,7 @@ pub fn exportMulOrPanic(comptime T: type, comptime W: type, comptime name: []con
fn func(self: T, other: T) callconv(.C) T {
const result = @call(.always_inline, mulWithOverflow, .{ T, W, self, other });
if (result.has_overflowed) {
roc_panic("integer multiplication overflowed!", 0);
unreachable;
roc_panic("Integer multiplication overflowed!", 0);
} else {
return result.value;
}
Expand Down
6 changes: 3 additions & 3 deletions crates/compiler/builtins/bitcode/src/panic.zig
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@ const std = @import("std");
const RocStr = @import("str.zig").RocStr;

// Signals to the host that the program has panicked
extern fn roc_panic(msg: *const RocStr, tag_id: u32) callconv(.C) void;
extern fn roc_panic(msg: *const RocStr, tag_id: u32) callconv(.C) noreturn;

pub fn panic_help(msg: []const u8, tag_id: u32) void {
pub fn panic_help(msg: []const u8, tag_id: u32) noreturn {
var str = RocStr.init(msg.ptr, msg.len);
roc_panic(&str, tag_id);
}

// must export this explicitly because right now it is not used from zig code
pub fn panic(msg: *const RocStr, alignment: u32) callconv(.C) void {
pub fn panic(msg: *const RocStr, alignment: u32) callconv(.C) noreturn {
return roc_panic(msg, alignment);
}
Comment on lines +5 to 15
Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was having a weird error in CI were valgrind was crashing but was not happening locally

My gut feeling is that us adding unreachable after a roc_panic call is whats causing the issue.

Looks like unreachable is undefined behavior when compiled with RelaseFast https://ziglang.org/documentation/master/#try which i think we do

This return type lets you do

if (some_cond) {
    roc_panic(...)
} else {
    return foo
}

without zig type errors

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So glad this worked. Thanks for the find

16 changes: 8 additions & 8 deletions crates/compiler/gen_llvm/src/llvm/lowlevel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1535,7 +1535,7 @@ fn build_int_binop<'ctx>(
)
.into_struct_value();

throw_on_overflow(env, parent, result, "integer addition overflowed!")
throw_on_overflow(env, parent, result, "Integer addition overflowed!")
}
NumAddWrap => bd.new_build_int_add(lhs, rhs, "add_int_wrap").into(),
NumAddChecked => {
Expand Down Expand Up @@ -1566,7 +1566,7 @@ fn build_int_binop<'ctx>(
)
.into_struct_value();

throw_on_overflow(env, parent, result, "integer subtraction overflowed!")
throw_on_overflow(env, parent, result, "Integer subtraction overflowed!")
}
NumSubWrap => bd.new_build_int_sub(lhs, rhs, "sub_int").into(),
NumSubChecked => {
Expand Down Expand Up @@ -1597,7 +1597,7 @@ fn build_int_binop<'ctx>(
)
.into_struct_value();

throw_on_overflow(env, parent, result, "integer multiplication overflowed!")
throw_on_overflow(env, parent, result, "Integer multiplication overflowed!")
}
NumMulWrap => bd.new_build_int_mul(lhs, rhs, "mul_int").into(),
NumMulSaturated => call_bitcode_fn(
Expand Down Expand Up @@ -2350,23 +2350,23 @@ fn build_dec_binop<'a, 'ctx>(
bitcode::DEC_ADD_WITH_OVERFLOW,
lhs,
rhs,
"decimal addition overflowed",
"Decimal addition overflowed",
),
NumSub => build_dec_binop_throw_on_overflow(
env,
parent,
bitcode::DEC_SUB_WITH_OVERFLOW,
lhs,
rhs,
"decimal subtraction overflowed",
"Decimal subtraction overflowed",
),
NumMul => build_dec_binop_throw_on_overflow(
env,
parent,
bitcode::DEC_MUL_WITH_OVERFLOW,
lhs,
rhs,
"decimal multiplication overflowed",
"Decimal multiplication overflowed",
),
NumDivFrac => dec_binop_with_unchecked(env, bitcode::DEC_DIV, lhs, rhs),

Expand Down Expand Up @@ -2659,7 +2659,7 @@ fn int_neg_raise_on_overflow<'ctx>(
throw_internal_exception(
env,
parent,
"integer negation overflowed because its argument is the minimum value",
"Integer negation overflowed because its argument is the minimum value",
);

builder.position_at_end(else_block);
Expand Down Expand Up @@ -2690,7 +2690,7 @@ fn int_abs_raise_on_overflow<'ctx>(
throw_internal_exception(
env,
parent,
"integer absolute overflowed because its argument is the minimum value",
"Integer absolute overflowed because its argument is the minimum value",
);

builder.position_at_end(else_block);
Expand Down
4 changes: 2 additions & 2 deletions crates/compiler/gen_wasm/src/low_level.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1382,7 +1382,7 @@ impl<'a> LowLevelCall<'a> {
}
NumAbs => {
const PANIC_MSG: &str =
"integer absolute overflowed because its argument is the minimum value";
"Integer absolute overflowed because its argument is the minimum value";

self.load_args(backend);

Expand Down Expand Up @@ -1446,7 +1446,7 @@ impl<'a> LowLevelCall<'a> {
}
NumNeg => {
const PANIC_MSG: &str =
"integer negation overflowed because its argument is the minimum value";
"Integer negation overflowed because its argument is the minimum value";

self.load_args(backend);
match CodeGenNumType::from(self.ret_layout) {
Expand Down
2 changes: 1 addition & 1 deletion crates/compiler/test_gen/src/gen_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2934,7 +2934,7 @@ fn list_map_with_index() {

#[test]
#[cfg(any(feature = "gen-llvm", feature = "gen-wasm"))]
#[should_panic(expected = r#"Roc failed with message: "integer addition overflowed!"#)]
#[should_panic(expected = r#"Roc failed with message: "Integer addition overflowed!"#)]
fn cleanup_because_exception() {
assert_evals_to!(
indoc!(
Expand Down
38 changes: 29 additions & 9 deletions crates/compiler/test_gen/src/gen_num.rs
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,7 @@ fn various_sized_abs() {
#[test]
#[cfg(any(feature = "gen-llvm", feature = "gen-wasm"))]
#[should_panic(
expected = r#"Roc failed with message: "integer absolute overflowed because its argument is the minimum value"#
expected = r#"Roc failed with message: "Integer absolute overflowed because its argument is the minimum value"#
)]
fn abs_min_int_overflow() {
assert_evals_to!(
Expand Down Expand Up @@ -799,6 +799,26 @@ fn gen_div_checked_by_zero_dec() {
);
}

#[test]
#[cfg(any(feature = "gen-llvm", feature = "gen-wasm"))]
#[should_panic(expected = r#"Roc failed with message: "Decimal division by 0!"#)]
fn gen_div_dec_by_zero() {
assert_evals_to!("1dec / 0", RocDec::from_str_to_i128_unsafe("-1"), i128);
}

#[test]
#[cfg(any(feature = "gen-llvm", feature = "gen-wasm"))]
#[should_panic(expected = r#"Roc failed with message: "Integer division by 0!"#)]
fn gen_div_ceil_by_zero() {
assert_evals_to!(
r#"
Num.divCeil 5 0 == 0
"#,
false,
bool
);
}

#[test]
#[cfg(any(feature = "gen-llvm", feature = "gen-dev", feature = "gen-wasm"))]
fn gen_int_eq() {
Expand Down Expand Up @@ -1581,7 +1601,7 @@ fn int_negate() {
#[test]
#[cfg(any(feature = "gen-llvm", feature = "gen-wasm"))]
#[should_panic(
expected = r#"Roc failed with message: "integer negation overflowed because its argument is the minimum value"#
expected = r#"Roc failed with message: "Integer negation overflowed because its argument is the minimum value"#
)]
fn neg_min_int_overflow() {
assert_evals_to!(
Expand Down Expand Up @@ -1809,7 +1829,7 @@ fn atan() {

#[test]
#[cfg(any(feature = "gen-llvm", feature = "gen-wasm"))]
#[should_panic(expected = r#"Roc failed with message: "integer addition overflowed!"#)]
#[should_panic(expected = r#"Roc failed with message: "Integer addition overflowed!"#)]
fn int_add_overflow() {
assert_evals_to!(
indoc!(
Expand Down Expand Up @@ -1884,7 +1904,7 @@ fn float_add_overflow() {

#[test]
#[cfg(any(feature = "gen-llvm", feature = "gen-wasm"))]
#[should_panic(expected = r#"Roc failed with message: "integer subtraction overflowed!"#)]
#[should_panic(expected = r#"Roc failed with message: "Integer subtraction overflowed!"#)]
fn int_sub_overflow() {
assert_evals_to!("-9_223_372_036_854_775_808 - 1", 0, i64);
}
Expand Down Expand Up @@ -1969,7 +1989,7 @@ fn float_sub_checked() {

#[test]
#[cfg(any(feature = "gen-llvm", feature = "gen-wasm"))]
#[should_panic(expected = r#"Roc failed with message: "integer multiplication overflowed!"#)]
#[should_panic(expected = r#"Roc failed with message: "Integer multiplication overflowed!"#)]
fn int_positive_mul_overflow() {
assert_evals_to!(
indoc!(
Expand All @@ -1984,7 +2004,7 @@ fn int_positive_mul_overflow() {

#[test]
#[cfg(any(feature = "gen-llvm", feature = "gen-wasm"))]
#[should_panic(expected = r#"Roc failed with message: "integer multiplication overflowed!"#)]
#[should_panic(expected = r#"Roc failed with message: "Integer multiplication overflowed!"#)]
fn int_negative_mul_overflow() {
assert_evals_to!(
indoc!(
Expand Down Expand Up @@ -3907,21 +3927,21 @@ fn num_abs_diff_float() {

#[test]
#[cfg(any(feature = "gen-llvm", feature = "gen-wasm"))]
#[should_panic(expected = r#"Roc failed with message: "integer subtraction overflowed!"#)]
#[should_panic(expected = r#"Roc failed with message: "Integer subtraction overflowed!"#)]
fn num_abs_max_overflow() {
assert_evals_to!(r#"Num.absDiff Num.maxI64 -1"#, 0, i64);
}

#[test]
#[cfg(any(feature = "gen-llvm", feature = "gen-wasm"))]
#[should_panic(expected = r#"Roc failed with message: "integer subtraction overflowed!"#)]
#[should_panic(expected = r#"Roc failed with message: "Integer subtraction overflowed!"#)]
fn num_abs_int_min_overflow() {
assert_evals_to!(r#"Num.absDiff Num.minI64 0"#, 0, i64);
}

#[test]
#[cfg(feature = "gen-llvm")]
#[should_panic(expected = r#"Roc failed with message: "integer subtraction overflowed!"#)]
#[should_panic(expected = r#"Roc failed with message: "Integer subtraction overflowed!"#)]
fn num_abs_large_bits_min_overflow() {
assert_evals_to!(r#"Num.absDiff Num.minI128 0"#, 0, i128);
}
Expand Down
2 changes: 1 addition & 1 deletion crates/lang_srv/debug_server.sh
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/bin/bash
#!/usr/bin/bash

SCRIPT_DIR=$(cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd)
${SCRIPT_DIR}/../../target/debug/roc_ls "$@" 2> /tmp/roc_ls.err
Loading
Loading