JuliaLang / julia

The Julia Programming Language
https://julialang.org/
MIT License
45.54k stars 5.47k forks source link

`checked_rem` and `checked_mod` don't follow the same promotion rules as regular rem and mod #54485

Open NHDaly opened 4 months ago

NHDaly commented 4 months ago
julia> typeof(Base.checked_rem(0x11, 2))
Int64

julia> typeof(Base.rem(0x11, 2))
UInt64

julia> typeof(Base.checked_rem(11, 0x2))
Int64

julia> typeof(Base.rem(11, 0x2))
Int64
julia> typeof(Base.checked_mod(0x11, 2))
Int64

julia> typeof(Base.mod(0x11, 2))
Int64

julia> typeof(Base.checked_mod(11, 0x2))
Int64

julia> typeof(Base.mod(11, 0x2))
UInt64

It looks like rem and mod have custom promotion rules, to keep the signed-ness of one of their arguments, but the checked_* operations just directly call promote(x,y) on the arguments.

Probably the checked* operations should be updated to have the same promotion rules as their normal, non-checked operators.

EDIT: Also, this unusual promotion behavior for rem and mod is not documented in the docstrings. Can we please fix that as part of this issue?

BioTurboNick commented 4 months ago

The promotion rules for div/rem/mod are similar to powers, and the promotion rules for it also aren't in the docstring.

julia> 0x11 + 1, 0x11 * 1, 0x11 ^ 1, 0x11 ÷ 1,           0x11 % 1
        (18,     17,       0x11,     0x0000000000000011, 0x0000000000000000)

Though also inconsistent that div and rem move to UInt64 while ^ sticks to the bit width of the first argument.

BioTurboNick commented 4 months ago

wat

julia> 0xFF ÷ Int8(-1)
0x01

Since Julia doesn't widen results of operations beyond their input argument types, the consistent option would be for unsigned dividends with negative divisors to throw a DivideError, just like when typemin(Int) ÷ -1 does. Integer division in Julia is documented to be safe from overflow, so this should probably be fixed.

IMO then integer division result should just be the type of the dividend, rather than the current signedness of the dividend and largest width of either argument.

For rem, can use the same rule, with the exception that there's no need to throw an error on "overflow" (following the pattern of typemin(Int) % -1 not erroring, and all results are positive.

For mod, the promotion rule should possibly be the typical promotion widening, because a result has to fit within the range of the divisor's type to be fully represented, but probably don't want it to be any narrower than the dividend.

BioTurboNick commented 4 months ago

Out of curiosity I looked at what C# does. It's also messy, but does prevent overflow.

In C#, all integer division/remainder results are widened to contain all valid results of the input types (e.g. UInt32 ÷ Int8 = Int64), with a minimum of Int32. Only if both are unsigned and at least one argument is 32-bit or larger will it return an unsigned result. If it can't widen further, the operation is not valid.