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

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

Open
NHDaly opened this issue May 15, 2024 · 3 comments
Labels
domain:maths Mathematical functions kind:bug Indicates an unexpected problem or unintended behavior

Comments

@NHDaly
Copy link
Member

NHDaly commented May 15, 2024

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?

@NHDaly NHDaly added kind:bug Indicates an unexpected problem or unintended behavior domain:maths Mathematical functions labels May 15, 2024
@NHDaly NHDaly changed the title checked_rem and checked_mod don't follow the same promotion rules as regular rem and mod checked_rem and checked_mod don't follow the same promotion rules as regular rem and mod May 15, 2024
@BioTurboNick
Copy link
Contributor

BioTurboNick commented May 29, 2024

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
Copy link
Contributor

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
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:maths Mathematical functions kind:bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

No branches or pull requests

2 participants