JeffreySarnoff / SaferIntegers.jl

These integer types use checked arithmetic, otherwise they are as system types.
MIT License
59 stars 9 forks source link

New method ambiguities for `rem(::SafeUnsigned, ::Base.BitSigned)` and others #21

Closed sostock closed 3 years ago

sostock commented 3 years ago

Version 3 introduced new method ambiguities:

julia> rem(SafeUInt(1), 1)
ERROR: MethodError: rem(::SafeUInt64, ::Int64) is ambiguous. Candidates:
  rem(x::T1, y::T2) where {T1<:SafeInteger, T2<:Integer} in SaferIntegers at /home/sebastian/.julia/packages/SaferIntegers/eaDK3/src/arith_ops.jl:64
  rem(x::Unsigned, y::Union{Int128, Int16, Int32, Int64, Int8}) in Base at int.jl:205
Possible fix, define
  rem(::T1, ::T2) where {T1<:SafeUnsigned, T2<:Union{Int128, Int16, Int32, Int64, Int8}}
Stacktrace:
[...]

The same ambiguities exist for mod, div, cld, fld, divrem.

Test.detect_ambiguities reveals that there are quite a number of ambiguities:

julia> Test.detect_ambiguities(SaferIntegers, Base)
110-element Vector{Tuple{Method, Method}}:
[...]
JeffreySarnoff commented 3 years ago

thank you -- @mkitti do you see the difficulty and its resolution?

mkitti commented 3 years ago

The first thing is to expand the test suite. I will try to tackle this.

JeffreySarnoff commented 3 years ago

The branch #fixambiguities (using some of what you did in https://github.com/JeffreySarnoff/SaferIntegers.jl/pull/22) and taking it further just to be safe before we have a more concise approach -- is working without ambiguities. I am going to merge it just so the released version works. The other changes you recommend remain very important and I am eager to get this repo there.

JeffreySarnoff commented 3 years ago

merged