Change the way we do Int64 conversion to/from GAP

Previously, any Int64 too big to fit into a GAP immediate integer was put into
a T_JULIA wrapper when passed to GAP, and unwrapped later. At the same time,
`julia_to_gap` relied completely (and incorrectly) on the C kernel functions
`julia_gap` and `gap_julia` to take care of *all* Int64 values.

The result was that passing such an integer as argument to a GAP function did
not work, not even if one explicitly used `julia_to_gap`.

To fix this we teach the C kernel function `gap_julia` to convert "large"
Int64 values to GAP big integers; this ensures that the many adapter functions
we install for Int64 arguments work.

This means that we violate the principle of roundtrip type fidelity (Int64 -> GAP large
int -> MPtr), but that is a very minor concern.

For really efficient conversions of e.g. Array{Int64,1} to GAP plists, we need
dedicated conversion functions which directly copy over bits. These can be
added transparently in the future.

Finally, we add some minor optimization to julia_to_gap for UInt64, Int128, UInt128

pkg/GAPJulia/JuliaInterface/gap/convert.gd

@@ -142,7 +142,7 @@
 #!  <List>
 #!  <Item>
 #!    <C>Int64</C> to immediate integer when it fits,
-#!    otherwise to &Julia; object wrapper,
+#!    otherwise to a &GAP; large integer,
 #!  </Item>
 #!  <Item>
 #!    <C>GapFFE</C> to immediate FFE,

pkg/GAPJulia/JuliaInterface/src/convert.c

@@ -54,7 +54,7 @@ Obj gap_julia(jl_value_t * julia_obj)
         if (INT_INTOBJ_MIN <= v && v <= INT_INTOBJ_MAX) {
             return INTOBJ_INT(v);
         }
-        return NewJuliaObj(julia_obj);
+        return ObjInt_Int8(v);
     }
     if (is_gapobj(julia_obj)) {
         return (Obj)julia_obj;

src/ccalls.jl

@@ -75,14 +75,6 @@ function NewPlist(length :: Int64)
     return o
 end
 
-function MakeObjInt(x::BigInt)
-    o = ccall( :MakeObjInt,
-               Ptr{Cvoid},
-               (Ptr{UInt64},Cint),
-               x.d, x.size )
-    return RAW_GAP_TO_JULIA( o )
-end
-
 function NEW_MACFLOAT(x::Float64)
     o = ccall( :NEW_MACFLOAT,
                Any,

src/julia_to_gap.jl

@@ -11,28 +11,40 @@ of input data, by converting egal data to identical objects in GAP.
 """
 
 
-## Default
+## Default for actual GAP objects is to do nothing
 julia_to_gap(x::GapObj) = x
 julia_to_gap(x::FFE) = x
 julia_to_gap(x::Bool) = x
 
+## Integers: general case first deal with things that fit into immediate
+## integers, then falls back to converting to BigInt and calling into the GAP
+## kernel API.
+## TODO: we could provide more efficient conversion for UInt64, Int128, UInt128
+## which avoids the conversion to BigInt, if we wanted to.
+function julia_to_gap(x::Integer)
+    # if it fits into a GAP immediate integer, convert x to Int64
+    if x in -1<<60:(1<<60-1)
+        return Int64(x)
+    end
+    # for the general case, fall back to BigInt
+    return julia_to_gap(BigInt(x))
+end
 
-## Integers
-julia_to_gap(x::Int128) = MakeObjInt(BigInt(x)) # FIXME: inefficient hack
+## Small integers types always fit into GAP immediate integers, and thus are
+## represented by Int64 on the Julia side.
 julia_to_gap(x::Int64)  = x
 julia_to_gap(x::Int32)  = Int64(x)
 julia_to_gap(x::Int16)  = Int64(x)
 julia_to_gap(x::Int8)   = Int64(x)
-
-## Unsigned Integers
-julia_to_gap(x::UInt128) = MakeObjInt(BigInt(x)) # FIXME: inefficient hack
-julia_to_gap(x::UInt64)  = MakeObjInt(BigInt(x)) # FIXME: inefficient hack
-julia_to_gap(x::UInt32)  = Int64(x)
-julia_to_gap(x::UInt16)  = Int64(x)
-julia_to_gap(x::UInt8)   = Int64(x)
-
-## BigInts
-julia_to_gap(x::BigInt) = MakeObjInt(x)
+julia_to_gap(x::UInt32) = Int64(x)
+julia_to_gap(x::UInt16) = Int64(x)
+julia_to_gap(x::UInt8)  = Int64(x)
+
+## BigInts are converted via a ccall
+function julia_to_gap(x::BigInt)
+    o = ccall(:MakeObjInt, Ptr{Cvoid}, (Ptr{UInt64},Cint), x.d, x.size)
+    return RAW_GAP_TO_JULIA(o)
+end
 
 ## Rationals
 function julia_to_gap(x::Rational{T}) where T <: Integer

test/basics.jl

@@ -56,7 +56,7 @@ end
     # passed and then extracted again
     @test f( (1,2,3) )[1] == (1,2,3)
 
-    @test GAP.Globals.IdFunc(2^62) isa Int64
+    @test !(GAP.Globals.IdFunc(2^62) isa Int64)
 
     x = GAP.Globals.Indeterminate(GAP.Globals.Rationals)
     f = x^4+1

test/conversion.jl

@@ -185,6 +185,21 @@ end
     @test GAP.julia_to_gap(Int16(1))  == 1
     @test GAP.julia_to_gap(Int8(1))   == 1
 
+    ## Int64 corner cases
+    @test GAP.julia_to_gap(-2^60  )  === -2^60
+    @test GAP.julia_to_gap( 2^60-1)  ===  2^60 - 1
+
+    @test GAP.Globals.IsInt(GAP.julia_to_gap(-2^60-1))
+    @test GAP.Globals.IsInt(GAP.julia_to_gap( 2^60))
+
+    @test GAP.Globals.IsInt(GAP.julia_to_gap(-2^63-1))
+    @test GAP.Globals.IsInt(GAP.julia_to_gap(-2^63))
+    @test GAP.Globals.IsInt(GAP.julia_to_gap( 2^63-1))
+    @test GAP.Globals.IsInt(GAP.julia_to_gap( 2^63))
+
+    # see issue https://github.com/oscar-system/GAP.jl/issues/332
+    @test 2^60 * GAP.Globals.Factorial(20) == GAP.EvalString("2^60 * Factorial(20)")
+
     ## Unsigned integers
     @test GAP.julia_to_gap(UInt128(1)) == 1
     @test GAP.julia_to_gap(UInt64(1))  == 1