Closed eschnett closed 2 years ago
I suspect this came from a desire to minimize the amount of code generated for div
. I agree an OverflowError makes more sense, but our div
is already painfully slow so we'd have to carefully check the performance and code size impact.
Closing as won'tfix for performance reasons.
Wouldn't it have been reasonable for div(typemin(Int), -1)
to return -typemin(Int) = typemax(Int) + 1 = typemin(Int)
instead of throwing? That return value is still meaningful in modular arithmetic, since what we require from div(x,y)
is that:
y * div(x,y) + rem(x,y) == x
which would still hold if div(typemin(Int), -1)
returned typemin(Int)
.
Is the main reason for not doing this that it would further slow down div
?
Figured I'd post a response here too, from Slack - LLVM defines division by zero and typemin(Int) by -1 to be undefined behavior, so Julia couldn't just rely on an LLVM intrinsic to implement this.
That said, in implementing this for OverflowContexts.jl, the branching required doesn't seem to hit performance much because integer division is so slow anyway.
But, the existing Julia API is that division is checked by default, and there is probably code relying on this behavior, so that would be a breaking change and introduce silently wrong arithemetic into existing code if it were to be done. Very bad :-)
From more chatting Slack: it is in fact documented that this case will throw - and so, I suppose, technically breaking to change it: https://docs.julialang.org/en/v1/manual/integers-and-floating-point-numbers/#Division-errors.
On the other hand, it is odd that we use non-throwing, modular arithmetic for addition, subtraction, and multiplication of Int
s, but not for division (aside from division by zero; that makes sense).
Currently,
div(typemin(Int), -1
throws aDivideError
. Should it throw anOverflowError
instead? Arguably, there's nothing wrong per se in dividing by-1
; it's just that the typeInt
can't hold the result.