Simplify and fix julia_to_gap, add and fix tests

LibGAP.jl/src/julia_to_gap.jl

@@ -72,29 +72,18 @@ function julia_to_gap(obj::Array{T,1}, recursive::Val{Recursive}=Val(false), rec
     ret_val = NewPlist(len)
     recursion_dict[obj] = ret_val
     for i in 1:len
-        current_obj = obj[i]
-        if haskey(recursion_dict,current_obj)
-            ret_val[i] = recursion_dict[current_obj]
-        else
-            if Recursive
-                current_converted = julia_to_gap(current_obj,recursive,recursion_dict)
-            else
-                current_converted = current_obj
-            end
-            recursion_dict[current_obj] = current_converted
-            ret_val[i] = current_converted
+        x = obj[i]
+        if Recursive
+            x = julia_to_gap(x, recursive, recursion_dict)
         end
+        ret_val[i] = x
     end
     return ret_val
 end
 
 ## Tuples
 function julia_to_gap(obj::Tuple, recursive::Val{Recursive}=Val(false), recursion_dict = IdDict()) where Recursive
-    size = length(obj)
-    array = Array{Any,1}(undef,size)
-    for i in 1:size
-        array[i] = obj[i]
-    end
+    array = collect(Any, obj)
     return julia_to_gap(array, recursive, recursion_dict)
 end
 
@@ -103,18 +92,16 @@ function julia_to_gap(obj::Dict{T,S}, recursive::Val{Recursive}=Val(false), recu
     if haskey(recursion_dict,obj)
         return recursion_dict[obj]
     end
-    nr_entries = obj.count
-    keys = Array{T,1}(undef,nr_entries)
-    entries = Array{S,1}(undef,nr_entries)
-    i = 1
+
+    recursion_dict[obj] = record = EvalString("rec()")
     for (x,y) in obj
-        keys[i] = x
-        entries[i] = y
-        i += 1
+        x = GAP.Globals.RNamObj(MakeString(string(x)))
+        if Recursive
+            y = julia_to_gap(y, recursive, recursion_dict)
+        end
+        GAP.Globals.ASS_REC(record, x, y)
     end
-    record = GAP.Globals.CreateRecFromKeyValuePairList(julia_to_gap(keys,Val(true)),julia_to_gap(entries, recursive, recursion_dict))
-    ## FIXME: This is too late!
-    recursion_dict[obj] = record
+
     return record
 end
 

LibGAP.jl/test/conversion.jl

@@ -93,13 +93,21 @@
     x = GAP.EvalString("(1,2,3)")
     @test GAP.gap_to_julia(x) == x
 
-    ## Recursive conversions
+    ## Conversions involving circular references
     xx = GAP.EvalString("l:=[1];x:=[l,l];")
     conv = GAP.gap_to_julia(xx)
     @test conv[1] === conv[2]
     conv = GAP.gap_to_julia(Tuple{Tuple{Int64},Tuple{Int64}},xx)
     @test conv[1] === conv[2]
 
+    xx = GAP.EvalString("[~]");
+    conv = GAP.gap_to_julia(xx)
+    @test conv === conv[1]
+
+    xx = GAP.EvalString("rec(a := 1, b := ~)");
+    conv = GAP.gap_to_julia(xx)
+    @test conv === conv[:b]
+
     ## Catch conversions to types that are not supported
     xx = GAP.julia_to_gap( "a" )
     @test_throws MethodError GAP.gap_to_julia( Dict{Int64,Int64}, xx )
@@ -167,19 +175,42 @@ end
 
     ## Dictionaries
     x = GAP.EvalString(" rec( foo := 1, bar := \"foo\" )" )
+    # ... recursive conversion
     y = Dict{Symbol,Any}( :foo => 1, :bar => "foo" )
     @test GAP.julia_to_gap(y,Val(true)) == x
+    # ... non-recursive conversion
     x = GAP.EvalString(" rec( foo := 1, bar := JuliaEvalString(\"\\\"foo\\\"\") )" )
     @test GAP.julia_to_gap(y) == x
 
-    ## Recursive conversions
+    ## Conversions with identical sub-objects
     l = [1]
     yy = [l,l]
-    xx = GAP.EvalString("l:=[1];x:=[l,l];")
-    conv = GAP.julia_to_gap(xx)
-    @test GAP.Globals.IsIdenticalObj(conv[1],conv[2])
+    # ... recursive conversion
+    conv = GAP.julia_to_gap(yy, Val(true))
+    @test isa(conv[1], GAP.MPtr)
+    @test conv[1] === conv[2]
+    # ... non-recursive conversion
+    conv = GAP.julia_to_gap(yy, Val(false))
+    @test isa(conv[1], Array{Int64,1})
+    @test conv[1] === conv[2]
 
-    xx = GAP.EvalString("[~]");
-    @test xx === xx[1]
+    ## converting a list with circular refs
+    yy = Array{Any,1}(undef,2)
+    yy[1] = yy;
+    yy[2] = yy;
+    # ... recursive conversion
+    conv = GAP.julia_to_gap(yy, Val(true));
+    @test conv[1] === conv
+    @test conv[1] === conv[2]
+    # ... non-recursive conversion
+    conv = GAP.julia_to_gap(yy, Val(false))
+    @test conv[1] !== conv
+    @test conv[1] === conv[2]
+
+    ## converting a dictionary with circular refs
+    d = Dict{String,Any}("a" => 1)
+    d["b"] = d
+    conv = GAP.julia_to_gap(d, Val(true));
+    @test conv === conv.b
 
 end