oscar-system / GAP.jl

GAP packages for Julia integration
https://oscar-system.github.io/GAP.jl/
GNU Lesser General Public License v3.0
64 stars 21 forks source link

GAP.julia_to_gap(big(2)^62) versus GAP.julia_to_gap(2^62) #688

Open fingolfin opened 3 years ago

fingolfin commented 3 years ago

I just run into this:

julia> GAP.julia_to_gap(big(2)^62)
GAP: 4611686018427387904

julia> GAP.julia_to_gap(2^62)
4611686018427387904

Without reflecting it deeper, I would expect the two to return the same thing. Maybe I am wrong, need to think more about it.

See also PR #434 and thinks that links to, and probably a bunch of other issues and PRs sigh

fingolfin commented 2 weeks ago

Looking at the code, we have

julia_to_gap(x::Int64) = x

but at the same time also this:

function julia_to_gap(x::Integer)
    # if it fits into a GAP immediate integer, convert x to Int64
    x in -1<<60:(1<<60-1) && return Int64(x)
    # for the general case, fall back to BigInt
    return julia_to_gap(BigInt(x))
end

...

function julia_to_gap(x::UInt)
    x < (1<<60) && return Int64(x)
    return ccall((:ObjInt_UInt, libgap), GapObj, (UInt64, ), x)
end

## BigInts are converted via a ccall
function julia_to_gap(x::BigInt)
    x in -1<<60:(1<<60-1) && return Int64(x)
    return GC.@preserve x ccall((:MakeObjInt, libgap), GapObj, (Ptr{UInt64}, Cint), x.d, x.size)
end

Thus once the value 2^62 is converted to an Int and once to a GapObj.

This can be fixed by replace the checks involving 1<<60 to compare against typemin(Int) resp. typemax(Int)