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

Add erroring and widening arithmetic operators #14320

Open
Validark opened this issue Jan 14, 2023 · 13 comments
Open

Add erroring and widening arithmetic operators #14320

Validark opened this issue Jan 14, 2023 · 13 comments
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@Validark
Copy link
Contributor

Validark commented Jan 14, 2023

Problem

Currently in Zig, the way to express a program which properly handles all potential errors, in this case integer overflows, is not the easiest, laziest thing to do. To use addition as the canonical example, most people reach for the + operator for nearly every case where they want to do addition (with +% and +| being used when called for by particular needs). This means the programmer is not expressing which additions can overflow and which cannot, because both are being expressed the same way.

Gibbon1's comment on Hackernews, although indicative of a lack of understanding that programs can be properly expressed in Zig, does indicate that this is a real problem.

See comment

abainbridge:

From the Zig language reference: Zig has many instances of undefined behavior. If undefined behavior is detected at compile-time, Zig emits a compile error and refuses to continue. Most undefined behavior that cannot be detected at compile-time can be detected at runtime. In these cases, Zig has safety checks. [...] When a safety check fails, Zig crashes with a stack trace

    Gibbon1:

    Zig crashes with a stack trace

    I hope the people behind Zig understand that in critical applications that's totally unacceptable.


Here is a YouTube video touring the facilities provided by various languages for integer overflow, and unfortunately for Zig, it is mainly in the same boat as C/C++, with std.math.add (potentially erroring addition) and std.math.mulWide (casting to a bigger integer size) being relegated to a single comment in the comments section (at the time of writing). I believe I have also seen other sentiments and comments over time of a similar nature. Many people think the way Zig deals with overflow is by using ReleaseSafe and throwing up its hands in defeat when it happens or using ReleaseFast and just letting wraparound behavior do ungodly things to your program invariants should an overflow occur (like allowing buffer overruns).

In short, std.math.add, @addWithOverflow, and casting integers to a bigger size are not nearly as ubiquitous as they probably should be, because they have a lot more friction than just using +. Zig strives to apply friction to make it harder to do things you likely should not be doing and make it easy to do things the right way, but in the status quo, I believe it accidentally encourages people to write suboptimal software that does not adequately express their intent when it comes to overflowable integer operations.

Solution

To remedy this, I propose adding two new classes of operators, which for now can be thought of as syntactic sugar for the standard library functions like std.math.add (erroring behavior) and std.math.mulWide (widening behavior).

operator name what it does when to use
+! erroring addition adds two integers, evaluating to an error.Overflow error upon overflow. when an overflow may occur and it is desirable to throw in that case
+_ widening addition adds two integers after widening with semantics similar to #7967 when an integer should be produced that can represent the correct value under all circumstances

This should complement the existing operators:

operator name what it does when to use
+ unoverflowable addition adds two integers and asserts that no overflow occurs, i.e. a + b is equivalent to (a +! b) catch unreachable when it is impossible for an overflow to occur, e.g. when an operation is a repeat and was already done and checked previously or when program invariants guarantee that overflow is impossible
+% wrapping addition adds two integers with twos-complement wrapping behavior when overflow occurs when wrapping overflow behavior is desired
+| saturating addition adds two integers with saturating behavior when overflow occurs when the maximum/minimum representable value is desired upon overflow

There should be similar operators for the other operators than can produce an error.Overflow. I.e. -!, *!, -_, *_. The erroring operators also conveniently eliminate the need for @addWithOverflow, @subWithOverflow, @mulWithOverflow, simplifying the language a little.

Example usage

var x = a +! b; // produces error.Overflow!PeerType(a, b)

// with try:
var x = try (a +! b);

// with catch:
var x = (a +! b) catch |err| {
    // err is error.Overflow
    return err;
};

// using catch to give a default value
var x = (a +! b) catch 0;

// with if:
if (a +! b) |x| {
    _ = x;
} else {}

I also think it would be a welcome feature to allow (these?) errors to propagate across operations, kind of like NaN.

// that way, one can mix and match operators
var x = a +! b *! c -! d *! e;

// one could even mix these with the non-erroring operators and it should work as expected
var x = a +! b - c; // the programmer knows that a+b>c

// this means only a single try/catch/error check is needed for a given sequence of operations
var x = try ((a *! b) - (a +! b) ^ 1);

Here is an implementation of the new operators, with the semantics I think are most desirable.

const std = @import("std");

pub fn erroring_int_type(comptime a: type, comptime b: type) type {
    const c: switch (@typeInfo(a)) {
        .ErrorUnion => |y| y.payload,
        else => a,
    } = undefined;

    const d: switch (@typeInfo(b)) {
        .ErrorUnion => |y| y.payload,
        else => b,
    } = undefined;

    const int_kind = @TypeOf(c, d);
    var error_set: type = error{Overflow};

    for (.{ a, b }) |x| {
        switch (@typeInfo(x)) {
            .ErrorUnion => |e| {
                error_set = error_set || e.error_set;
            },
            else => {},
        }
    }

    return error_set!int_kind;
}

fn log2(comptime _x: comptime_int) comptime_int {
    comptime var count = 0;
    comptime var x = _x;
    inline while (x != 0) : (x >>= 1) count += 1;
    return count;
}

fn isPowerOfTwo(comptime v: comptime_int) bool {
    return (v & (v - 1)) == 0;
}

const OP_KIND = enum { add, sub, mul };

pub fn widened_int_type(comptime a_range: [2]comptime_int, comptime b_range: [2]comptime_int, comptime widenType: OP_KIND) type {
    // each range argument is in the form [min, max]
    var min: comptime_int = undefined;
    var max: comptime_int = undefined;

    switch (widenType) {
        .add => {
            min = a_range[0] + b_range[0];
            max = a_range[1] + b_range[1];
        },
        .sub => {
            min = a_range[0] - b_range[1];
            max = a_range[1] - b_range[0];
        },
        .mul => {
            // I originally tried to be clever here, but eventually I realized the logic I needed was more complex
            // than just doing a couple of @min/@max calls.
            const _0 = a_range[0] * b_range[0];
            const _1 = a_range[0] * b_range[1];
            const _2 = a_range[1] * b_range[0];
            const _3 = a_range[1] * b_range[1];
            min = @min(@min(_0, _1), @min(_2, _3));
            max = @max(@max(_0, _1), @max(_2, _3));
        },
    }

    const sign: std.builtin.Signedness = if ((widenType == .sub and a_range[0] == a_range[1] and !isPowerOfTwo(a_range[0] + 1)) or min < 0) .signed else .unsigned;
    comptime var bits = @boolToInt(sign == .signed);

    min = if (min < 0) min + 1 else min;
    max = if (max < 0) max + 1 else max;

    inline while (min != 0 or max != 0) {
        bits += 1;
        min = if (min == -1) 0 else min >> 1;
        max = if (max == -1) 0 else max >> 1;
    }

    return std.meta.Int(sign, bits);
}

pub fn widened_int_type_with_error(comptime a: type, comptime b: type, comptime a_range: [2]comptime_int, comptime b_range: [2]comptime_int, comptime widenType: OP_KIND) type {
    const int_type = widened_int_type(a_range, b_range, widenType);
    var error_set: ?type = null;
    for (.{ a, b }) |x| {
        switch (@typeInfo(x)) {
            .ErrorUnion => |e| {
                if (error_set) |_| {
                    error_set = error_set || e.error_set;
                } else {
                    error_set = e.error_set;
                }
            },
            else => {},
        }
    }

    if (error_set) |e| {
        return e!int_type;
    } else {
        return int_type;
    }
}

pub fn @"+!"(a: anytype, b: anytype) erroring_int_type(@TypeOf(a), @TypeOf(b)) {
    const c = switch (@typeInfo(@TypeOf(a))) {
        .ErrorUnion => try a,
        else => a,
    };

    const d = switch (@typeInfo(@TypeOf(b))) {
        .ErrorUnion => try b,
        else => b,
    };

    return std.math.add(@TypeOf(c, d), c, d);
}

pub fn @"-!"(a: anytype, b: anytype) erroring_int_type(@TypeOf(a), @TypeOf(b)) {
    const c = switch (@typeInfo(@TypeOf(a))) {
        .ErrorUnion => try a,
        else => a,
    };

    const d = switch (@typeInfo(@TypeOf(b))) {
        .ErrorUnion => try b,
        else => b,
    };

    return std.math.sub(@TypeOf(c, d), c, d);
}

pub fn @"*!"(a: anytype, b: anytype) erroring_int_type(@TypeOf(a), @TypeOf(b)) {
    const c = switch (@typeInfo(@TypeOf(a))) {
        .ErrorUnion => try a,
        else => a,
    };

    const d = switch (@typeInfo(@TypeOf(b))) {
        .ErrorUnion => try b,
        else => b,
    };

    return std.math.mul(@TypeOf(c, d), c, d);
}

pub fn int_range(comptime x: type) [2]comptime_int {
    const a = switch (@typeInfo(x)) {
        .ErrorUnion => |y| y.payload,
        else => x,
    };
    return .{ std.math.minInt(a), std.math.maxInt(a) };
}

fn widenOp(comptime op: OP_KIND, a: anytype, b: anytype) widened_int_type_with_error(@TypeOf(a), @TypeOf(b), switch (@typeInfo(@TypeOf(a))) {
    .ComptimeInt => .{ a, a },
    else => int_range(@TypeOf(a)),
}, switch (@typeInfo(@TypeOf(b))) {
    .ComptimeInt => .{ b, b },
    else => int_range(@TypeOf(b)),
}, op) {
    const int_type = widened_int_type(switch (@typeInfo(@TypeOf(a))) {
        .ComptimeInt => .{ a, a },
        else => int_range(@TypeOf(a)),
    }, switch (@typeInfo(@TypeOf(b))) {
        .ComptimeInt => .{ b, b },
        else => int_range(@TypeOf(b)),
    }, op);

    const bits = @typeInfo(int_type).Int.bits;
    const signedness = @typeInfo(int_type).Int.signedness;

    const c = switch (@typeInfo(@TypeOf(a))) {
        .ErrorUnion => try a,
        else => a,
    };

    const d = switch (@typeInfo(@TypeOf(b))) {
        .ErrorUnion => try b,
        else => b,
    };

    const e = if (switch (@typeInfo(@TypeOf(c))) {
        .Int => true,
        else => false,
    } and signedness != @typeInfo(@TypeOf(c)).Int.signedness)
        @bitCast(int_type, @as(std.meta.Int(@typeInfo(@TypeOf(c)).Int.signedness, bits), c))
    else
        @as(int_type, c);

    const f = switch (@typeInfo(@TypeOf(d))) {
        .Int => if (signedness != @typeInfo(@TypeOf(d)).Int.signedness)
            @bitCast(int_type, @as(std.meta.Int(@typeInfo(@TypeOf(d)).Int.signedness, bits), d))
        else
            @as(int_type, d),

        // i8 +_ 128 -> u8,  u8 -_ 128 -> i8
        .ComptimeInt => if (comptime d > 0 and isPowerOfTwo(d) and log2(d) == bits)
            @bitCast(int_type, @as(std.meta.Int(.signed, bits), -d))
        else
            @as(int_type, d),
        else => unreachable,
    };

    return switch (op) {
        .add => e +% f,
        .sub => e -% f,
        .mul => e *% f,
    };
}

fn @"+_"(a: anytype, b: anytype) widened_int_type_with_error(@TypeOf(a), @TypeOf(b), switch (@typeInfo(@TypeOf(a))) {
    .ComptimeInt => .{ a, a },
    else => int_range(@TypeOf(a)),
}, switch (@typeInfo(@TypeOf(b))) {
    .ComptimeInt => .{ b, b },
    else => int_range(@TypeOf(b)),
}, .add) {
    return widenOp(.add, a, b);
}

fn @"-_"(a: anytype, b: anytype) widened_int_type_with_error(@TypeOf(a), @TypeOf(b), switch (@typeInfo(@TypeOf(a))) {
    .ComptimeInt => .{ a, a },
    else => int_range(@TypeOf(a)),
}, switch (@typeInfo(@TypeOf(b))) {
    .ComptimeInt => .{ b, b },
    else => int_range(@TypeOf(b)),
}, .sub) {
    return widenOp(.sub, a, b);
}

fn @"*_"(a: anytype, b: anytype) widened_int_type_with_error(@TypeOf(a), @TypeOf(b), switch (@typeInfo(@TypeOf(a))) {
    .ComptimeInt => .{ a, a },
    else => int_range(@TypeOf(a)),
}, switch (@typeInfo(@TypeOf(b))) {
    .ComptimeInt => .{ b, b },
    else => int_range(@TypeOf(b)),
}, .mul) {
    return widenOp(.mul, a, b);
}

Note that the widening operator actually adapts according to what the range of potential variables are. That means when dealing with comptime ints we can be more precise:

u8 *_ 257 -> u16
u8 *_ 258 -> u17
u8 +_ 1 -> u9
u8 +_ 65280 -> u16
u8 +_ 65281 -> u17
i8 +_ 1 -> i9
i8 +_ 128 -> u8
u8 -_ 128 -> i8
127 -_ u8 -> i8
127 -_ i8 -> u8

Assignment operators

+!= makes sense as an operator so long as it is operating on an error union. It would be necessary to check the value using an if statement after a +!= operation, but it would work.

+_= does not make sense as an operator, as it would require changing the destination variable type, but in Zig variables cannot change their type dynamically.

Discussions

There could also be <<!, >>! which could replace @shlWithOverflow, as well as @shrExact and @shlExact when paired with catch unreachable. However, having these might imply that a << b is equivalent to (a <<! b) catch unreachable, which in status quo it isn't. It's worth consideration, but probably not a good fit for Zig at the moment.

Note that the / operator only works on unsigned values in Zig, so overflowing via std.math.minInt(Type) / -1 is not possible. However, there is still safety checked/undefined behavior for divisions by zero, so /! could check evaluate to error.DivisionByZero when applicable, although it is less compelling of use-case to support.

I think the ! and _ operators should not work for floats. E.g. f32 +_ f32 should not work, and I don't even know what the ! operators would do for floats anyway.

Parsing

+! et al. should mostly integrate into Zig just fine, because a+!b has never worked because the arithmetic operators do not work on booleans.

+_ could conflict with existing Zig programs in the case of a+_b. Thus, the +_ operator must require a space afterwards in order to be considered a valid widening addition. Ambiguous + operators without a space following it where the next operand is a variable that starts with an underscore should be banned. E.g. a+_b should be banned because it could mean a +_ b or a + _b, and a+_1 should be banned because it could mean a +_ 1 or a + _1.

Prior art

Others have had this idea: #7821 (comment)


Update: Removed rounding up to power of 2.
Side note: I also believe the error should be changed to error.IntegerOverflow, so as to not be confused with stack overflow or any other overflow you can think of.

@matu3ba
Copy link
Contributor

matu3ba commented Jan 14, 2023

Only looking at this proposal:

  1. I would suggest to use a symbol more related to the widening operator https://duckduckgo.com/l/?uddg=https%3A%2F%2Fwww.cs.cmu.edu%2F~aldrich%2Fcourses%2F15%2D819O%2D13sp%2Fresources%2Fwidening%2Dcollecting.pdf&rut=fe99615cb5c27ef982bbbaf66949dac29212e923ec22827997a38a1e8605bb60, so u. The alternative would be the reseversed delta, but that would be extremely unergonomic.
    _ means to me bottom or lower interval, which does not capture the semantics very well.

  2. Implicit widening of the result location should be forbidden, so usage of +_ must have the result location being annotated or one will have annoying to track down performance bottlenecks as it may be unclear, if the variable fits into a register. Reason: var x = a +! b *! c -! d *! e; makes the result of widening not obvious on first look.

This feels consistent, but not very ergonomic to write or read.

Also I am curious: Do you know use cases, where non-global handling (not at statement level) of overflow and widening would be strictly necessary?

  • Adding ! to every operation feels clunky to me, if I cant distinguish where the error happened anyway.
  • Adding widening sounds to ensure no overflow can happen sounds to me like it would be also much more readable at the statement level.

Statement level means var x: u128 = widen/fail (a + b - d*c + g*f - z).


Counter-proposal (wip thought process):

// return error on failed operation
var x: u128 = try @errorop(a + b - d*c + g*f - z)
var x: u128 = @errorop(a + b - d*c + g*f - z) catch unreachable;
// widen
var x = @widen(a + b - d*c + g*f - z); // error: widen requires type on result location
var x: u128 = @widen(a + b - d*c + g*f - z);
// mixing @errorop with other errors must be forbidden. arithmetic errors of local context 
must always be handled local

Shortcomings with this counter-proposal:

  • try @errorop(..) still feels clunky to write and refactor with the brackets
  • errorop is a bad name
  • This would be an exception to the usual Zig approach of handling every error locally and could set a bad precedent.

@matu3ba
Copy link
Contributor

matu3ba commented Jan 14, 2023

I feel more like widening and error operation for arithmetic-only (no other errors possible in that statement) belong to the assignment, because the alternative is less readable, less copy-pastable and arithmetic is too common of an operation.

The typical use case for "add overflow checks to everything" is to me to have effortless + relative fast sanitation of a limited number of inputs not known to be safe to use, because the developer did not check input bounds of the arithmetic.

Or do I misunderstand the use case for widening and error handling in arithmetic-only statements?

@Validark
Copy link
Contributor Author

Validark commented Jan 15, 2023

  1. I would suggest to use a symbol more related to the widening operator, so u. The alternative would be the reseversed delta, but that would be extremely unergonomic.
    _ means to me bottom or lower interval, which does not capture the semantics very well.

I see your point, but I went with _ as an ascii stand-in for , , or , so it actually is kind of a similar idea to the -looking symbol used in math.

  1. Implicit widening of the result location should be forbidden, so usage of +_ must have the result location being annotated or one will have annoying to track down performance bottlenecks as it may be unclear, if the variable fits into a register. Reason: var x = a +! b *! c -! d *! e; makes the result of widening not obvious on first look.

Well, no widening operator was used in that expression, so that would just give you the status quo integer type unioned with an error. If you meant to use the widening operators in all those places, I would agree the result of that widening would not be obvious, and I think that would be a very contrived and bad piece of code to write. The point of these operators is to make it easier to express (good and safe) intent, not to throw a bunch of syntax together and guess what it means. I'd expect that widening operators will typically be used in expressions with only one or two widening operators in them. I'd expect things like these to be way more common:

x +_ @boolToInt(b)
@boolToInt(b1) +_ @boolToInt(b2)
a *_ b

Also I am curious: Do you know use cases, where non-global handling (not at statement level) of overflow and widening would be strictly necessary?

I am not sure which way to interpret this question, so I will try to answer the different things you might mean. For one, Zig already handles expressions independently of the assignment statement they occur in. E.g. u32 = u1 + u1 will give you an XOR (equivalent) operation in ReleaseFast, even if you wanted a u32 between 0 and 2 inclusive.

If you are asking about the need for being able to express different arithmetic semantics within a single expression, I gave a theoretical example of a situation where you know that adding up some numbers will give an answer larger than another number.

var x = a +! b - c;
// the programmer knows that a+b>c

If you are asking about whether it would ever make sense to catch just part of an expression rather than the entire thing, I'd say it depends on whether you prefer to break those situations into multiple statements or keep them in one.

  • Adding ! to every operation feels clunky to me, if I cant distinguish where the error happened anyway.

Well you can distinguish where it happened, if you write it in such a way that you can by breaking up into multiple statements or catching directly on the relevant subexpressions. If you write it in a way where you can't distinguish where it happened, that should mean that you don't care where it happened.

  • Adding widening sounds to ensure no overflow can happen sounds to me like it would be also much more readable at the statement level.

Statement level means var x: u128 = widen/fail (a + b - d*c + g*f - z).

The problem with adding built-ins like @errorop or @widen in my view is that

  1. They look like function calls, not wrappers that modify operator semantics contained within AND the assignment expression they appear in.
  2. We are kind of back to the issue with @addWithOverflow and std.math.add, they just are not cemented into to the foundation of the language as much as the +/+%/+| operators are.

The typical use case for "add overflow checks to everything" is to me to have effortless + relative fast sanitation of a limited number of inputs not known to be safe to use, because the developer did not check input bounds of the arithmetic.

The last part of this statement is the problem, in my view. I want programmers to actually express in their program where overflow can occur and where it can't. E.g. imagine I calculate how much space to allocate, allocate that much space, and then write the important data into that buffer. When calculating how much space to allocate in the first pass, it might overflow. When calculating the index to write into that buffer, we know it can't overflow because we are repeating the calculations from the first pass:

var required_space: error{Overflow}!u64 = 0;
for (data) |b| { // imagine some real logic
    required_space +!= 1;
    // we might overflow if we don't know how big `data` is
   // imagine we're streaming `data` or the calculation for space usage is superlinear
}

var buffer = try allocator.alloc(try required_space);
var index: u64 = 0;

for (data) |b| {
    buffer[index] = b;
    index += 1;
    // we know this can't overflow because we are repeating
    // the calculations from the previous, already checked loop.
}

@matu3ba
Copy link
Contributor

matu3ba commented Jan 15, 2023

will typically be used in expressions with only one or two widening operators in them. I'd expect things like these to be way more common

Unfortunately, we need to take into consideration all edge cases or we can create unnecessary + terrible footguns.

I think allowing error handling together with trapping in tedious and long expressions like a + b - d*c + g*f - z or worse is a terrible idea, because a typo is easy to miss in a code review and then you have a potential vulnerability in sanitation code (where developers will likely rely on this functionality to "just work").
In theory it is nice and shiny, but in practice somebody will use it for sanitation (of a more cold path) as "without the typo it just works".

Likewise stands my argument for widening: Each nesting of a multiplication multiplies the necessary bit size by 2, which obliterates performance very quickly if not forcing to be catched via explicit type bounds. Mixing trapping and widening code is here also a problem.

I do agree with you on the reasoning for @errorop or @widen. They feel unnecessary unreadable.

The last part of this statement is the problem, in my view. I want programmers to actually express in their program where overflow can occur and where it can't.

Agreed. I would phrase the proposal according to exactly this use case instead of "Add erroring and widening arithmetic operators".

  • As of now, the Zig default is "arithmetic is fast + unsafe".
  • I do believe mixing "unsafe + safe parts" should be very explicit and readable or it can create CVEs as people will use them for sanitation and stuff like single-letter typos are often missed in code reviews
  • Probably it is best if they can not be mixed in a statement, so all "safe arithmetic" is annotated ?
  • Safe arithmetic is significantly slower (affects register sizes or branch prediction/pollutes instruction fetcher + binary size with hints what branches are cold) ?
  • Slow stuff has longer names in Zig
  • Should assignments have annotations for this and what would have reasonable length and uniqueness?

@wooster0
Copy link

After thinking about this for a while, I'm starting to like this.
I do think it is far more complicated at the moment to handle overflow than it should be. At the moment it's often easiest to just think "oh whatever, when is this ever going to overflow?" even though in Zig we handle edge cases.

I like the idea of making all the different operator types equally hard/easy to write.

I think +_ etc. are quite logical (the underscore is wide) and I think it's fine to add two more operator types.

An argument against +! is that it may be unexpected for an operator to return an error union. I think it's fine though and eventually maybe float operations will return error.NaN when desirable. I think that's option A of #11234. So that'd be consistent with that too.

@rohlem
Copy link
Contributor

rohlem commented Jan 16, 2023

The one thing I don't understand about this proposal is the auto-over-widening rounding up to a power of two.
If we didn't do that, those new operators would scale much better.

a b a *_ b minimally widened proposed over-widened
0xFF (max u8) 0x102 (u9) 0x1 00FE u17 u32
0x1 FFFF (max u17) 0xFF 0x1FD FF01 u25 u32
0x1FF FFFF (max u25) 0xFF 0x1 FDFF FF01 u33 (-> u32 + overflow flag bit) u64
0x1 FFFF FFFF (max u33) 0xFF 0x1FD FFFF FF01 u41 u64
0xFFFF FFFF FFFF FFFF 0xFF 0xFE FFFF FFFF FFFF FF01 u72 u128

I understand that most architectures won't provide specific arithmetic instructions for, say, u18, but to me that is well into the area of optimization problem,
and as such I'd prefer to leave it to the compiler. It knows how big which registers are, and their current allocation.
And it can reorder, inline, and recombine operations on local variables as much as it wants.

With the proposed over-widened result types, if I need/want slimmer ranges, I'd have to

  • re-count the actual bits an operation can result in (which is error prone), and then
  • add an explicit @intCast, which means "I guarantee I made no errors".

With slimmer result types, you'd only have to fiddle with types in response to the generated assembly -
and then it would only be growing types, which is always safe.

@Validark
Copy link
Contributor Author

will typically be used in expressions with only one or two widening operators in them. I'd expect things like these to be way more common

Unfortunately, we need to take into consideration all edge cases or we can create unnecessary + terrible footguns.

I think allowing error handling together with trapping in tedious and long expressions like a + b - d*c + g*f - z or worse is a terrible idea, because a typo is easy to miss in a code review and then you have a potential vulnerability in sanitation code (where developers will likely rely on this functionality to "just work"). In theory it is nice and shiny, but in practice somebody will use it for sanitation (of a more cold path) as "without the typo it just works".

I'm not fully understanding what specific situations you may be thinking of, but I don't think this is a bad feature just because someone could abuse it. I think in order for something to be a footgun you have to be trying to get it right but some other subtlety thwarts your efforts. E.g. @boolToInt(b1) + @boolToInt(b2) is a footgun because the programmer was thinking that b1 and b2 should each be converted to 1 or 0 and then added together, however, this will do an XOR because the type of these two is u1 so in the case of 1 + 1 it overflows to 0 in ReleaseFast. Whereas, casting 123 to a pointer and dereferencing it is not a footgun because the programmer has to do that totally on purpose.

With +! and +_, the programmer is thinking about what kind of add they want, so the only way to mess that up is if they are incorrect about the invariants in their program.

I also am not sure I agree with the statement "without the typo it just works". For one, when you have an expression that produces an error vs one that doesn't, that will be handled very differently by the rest of the code and so that will require multiple lines to change (or at least some try or catch expression will need to be removed).

If you are just critiquing the possibility of someone properly specifying which operations can overflow for most of the operations but specifying one operation incorrectly, then to that I would say that that could happen in status quo anyway. In fact, the easiest thing to do is to just not specify which operations can overflow at all. So from that perspective, any stray ! characters that get tacked on to arithmetic operators is a win for correctness.

Likewise stands my argument for widening: Each nesting of a multiplication multiplies the necessary bit size by 2, which obliterates performance very quickly if not forcing to be catched via explicit type bounds. Mixing trapping and widening code is here also a problem.

The mere presence of these operators in the language will encourage programmers to ask themselves whether each arithmetic operation might overflow and what should happen in each case. It will change the way programmers think: it will make them think more carefully about arithmetic operations in general. So I really am not afraid that someone might do var x = a *_ b *_ c *_ d *_ e, because these operators increase intentionality. And if something like the previous statement made it past code review, I don't know what you want to save you. Maybe have a linter that tells you if you are using an integer bigger than 64 or 128 bits and only allow it with a comment that says you know what you're doing? And, on most keyboards, you have to hold shift to type ! or _, and they are often on opposite ends as well, so I am not afraid of accidentally typing one of these operators.

  • As of now, the Zig default is "arithmetic is fast + unsafe".
  • Safe arithmetic is significantly slower (affects register sizes or branch prediction/pollutes instruction fetcher + binary size with hints what branches are cold) ?
  • Slow stuff has longer names in Zig

Yes, safe arithmetic is slower on most current ISAs, but that is an implementation detail. Don't get me wrong, I care deeply about speed, and power consumption too. On better ISA's like the Mill, which hopefully takes over the world some day, safe arithmetic is equally as fast as wrapping arithmetic. On ISA's like x86_64, it requires a jo (jump on overflow) after each potentially erroring arithmetic operation. Due to branch prediction, it shouldn't hurt speed too badly if we are talking about an operation which will almost never, ever overflow. But yes, it does take space in the binary and instruction/prediction caches on most ISA's we have to deal with.

To that, I would say that correctness is more important than speed. One of Zig's fundamental beliefs is that allocation can fail, even though allocation succeeds 99.9999% of the time (made up statistic but probably pretty close). Zig would be faster if it didn't check whether allocation failed and didn't pass around allocator pointers. But would it be better?

To me, one of the most attractive features of Zig is the ability to ensure all edge-cases are properly handled. Zig should encourage handling arithmetic operations properly too.

@Validark
Copy link
Contributor Author

The one thing I don't understand about this proposal is the auto-over-widening rounding up to a power of two. If we didn't do that, those new operators would scale much better.

You are probably right. I was not sure what behavior would be expected but now that I think about it, it is probably more Zig-like to produce integer types that are not rounded up to a power of two. And like you said, the compiler should be able to figure out that a +_ operation should not need an AND operation with it to make sure it didn't overflow, because it should be able to assert overflow did not happen.

@Validark
Copy link
Contributor Author

Validark commented Jan 17, 2023

@rohlem Do you believe it would make sense to have /! that errors on divide by 0 and /_ which narrows when the divisor is a comptime_int? It is somewhat orthogonal, but does work differently then the others. I do feel like a narrowing division operator has a place in Zig, but I'm not sure how it should look.

@InKryption
Copy link
Contributor

If we wanted to be able to narrow through division with comptime_int, you wouldn't need a whole new operator, you'd just need to make it so that that's what happens when you divide with a comptime_int.

@rohlem
Copy link
Contributor

rohlem commented Jan 17, 2023

I like and support @InKryption's suggestion above, I don't see a reason why we shouldn't always narrow the result type when dividing by comptime_int.
Note if #3806 were accepted (though I don't deem it likely), we could also auto-narrow with a divisor type comptime-known to be in a range,
f.e. divide by [4, 12] => always remove 2 bits.
For now, though, since we can't narrow with a runtime divisor, I don't think a special variant would be very useful.

@Validark I do think /! and %! would both be useful. Note that division can technically trigger overflow, f.e. @as(i1, -1)/-1.
(I think we should have /% for this reason too, though that's arguably has quite low piority.)

One special case that an actual widening division would be useful would be if we had distinct float types for finite-only and full IEEE numbers.
Then we could allow division of a finite value by 0 to yield infinity only when using /_, so that you stay in the finite domain when using / on finite floats.
(I actually wrote up a proposal in a comment to #11234 interacting with this, which was mostly ignored.
I might end up re-posting it as a standalone proposal if that issue gets closed without a clear stance on my version.)

@Validark
Copy link
Contributor Author

@Validark I do think /! and %! would both be useful. Note that division can technically trigger overflow, f.e. @as(i1, -1)/-1.
(I think we should have /% for this reason too, though that's arguably has quite low piority.)

In Zig, the division operator does not work with signed integers unless they are known to be positive at compile-time, because you have to choose whether you want it to round towards 0 or -inf (@divTrunc or @divFloor). So your example is not possible to execute at runtime.

@Validark
Copy link
Contributor Author

Update: I removed over-widening (rounding up to a power of 2) from the proposal.

@andrewrk andrewrk added the proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. label Jan 23, 2023
@andrewrk andrewrk added this to the 0.12.0 milestone Jan 23, 2023
@andrewrk andrewrk modified the milestones: 0.13.0, 0.12.0 Jul 9, 2023
@andrewrk andrewrk modified the milestones: 0.14.0, 0.15.0 Feb 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet
Development

No branches or pull requests

6 participants