Tweak the conversion code (#1268)

* src/conversion.jl: order code differently

I never could remember whether handle_recursion belongs to
recursion_info_j or _g and had to look at call sites to find out.
Now it is clear (to me, at least) from the way the code is
arranged.

* Some more code re-ordering to make it more uniform

* More cleanup

Author: fingolfin

src/conversion.jl

@@ -44,6 +44,29 @@ const RecDict_j = IdDict{Tuple{Any, Type}, Any}
 
 const JuliaCacheDict = Union{Nothing,RecDict_j}
 
+# helper functions for recursion in conversion from GAP to Julia
+function recursion_info_j(::Type{T}, obj, recursive::Bool, recursion_dict::JuliaCacheDict) where {T}
+    if recursive && recursion_dict === nothing
+        return RecDict_j()
+    else
+        return recursion_dict
+    end
+end
+
+function handle_recursion(obj, ret_val, rec::Bool, rec_dict::Union{Nothing,IdDict})
+    if rec_dict !== nothing
+        # We assume that `obj` is not yet cached.
+        rec_dict[obj] = ret_val
+    end
+    return rec ? rec_dict : nothing
+end
+
+# Switch off recursion (hence avoid the creation of a dictionary)
+# if `isbitstype(T)` or `T <: GAP.Obj` holds (includes `GapObj`).
+function _needs_tracking_gap_to_julia(::Type{T}) where T
+  return !(isbitstype(T) || T <: GAP.Obj)
+end
+
 
 """
     RecDict_g = IdDict{Any,Any}
@@ -56,17 +79,6 @@ const RecDict_g = IdDict{Any,Any}
 
 const GapCacheDict = Union{Nothing,RecDict_g}
 
-
-# helper functions for recursion in conversion from GAP to Julia
-function recursion_info_j(::Type{T}, obj, recursive::Bool, recursion_dict::JuliaCacheDict) where {T}
-    if recursive && recursion_dict === nothing
-        return RecDict_j()
-    else
-        return recursion_dict
-    end
-end
-
-
 # helper functions for recursion (conversion from Julia to GAP)
 function recursion_info_g(::Type{T}, obj, ret_val, recursive::Bool, recursion_dict::GapCacheDict) where {T}
     rec = recursive && _needs_tracking_julia_to_gap(T)
@@ -84,21 +96,6 @@ function recursion_info_g(::Type{T}, obj, ret_val, recursive::Bool, recursion_di
 
 end
 
-function handle_recursion(obj, ret_val, rec::Bool, rec_dict::Union{Nothing,IdDict})
-    if rec_dict !== nothing
-        # We assume that `obj` is not yet cached.
-        rec_dict[obj] = ret_val
-    end
-    return rec ? rec_dict : nothing
-end
-
-
-# Switch off recursion (hence avoid the creation of a dictionary)
-# if `isbitstype(T)` or `T <: GAP.Obj` holds (includes `GapObj`).
-function _needs_tracking_gap_to_julia(::Type{T}) where T
-  return !(isbitstype(T) || T <: GAP.Obj)
-end
-
 
 # Helper to make use of `Val{true}` and `Val{false}` more efficient:
 # whenever we use patterns like `Val(recursive)` this throws a curve ball

src/gap_to_julia.jl

@@ -99,12 +99,11 @@ function gap_to_julia_internal(
     # if the desired result is already stored then return it
     recursive && recursion_dict !== nothing && haskey(recursion_dict, (obj, TT)) && return recursion_dict[(obj, TT)]
 
-    T = eltype(TT)
-    rec = recursive && _needs_tracking_gap_to_julia(T)
-
     len_list = length(obj)
     ret_val = TT(undef, len_list)::TT
 
+    T = eltype(TT)
+    rec = recursive && _needs_tracking_gap_to_julia(T)
     rec_dict = recursion_info_j(TT, obj, rec, recursion_dict)
     recursion_dict = handle_recursion((obj, TT), ret_val, rec, rec_dict)
 
@@ -147,13 +146,12 @@ function gap_to_julia_internal(
 
     recursive && recursion_dict !== nothing && haskey(recursion_dict, (obj, TT)) && return recursion_dict[(obj, TT)]
 
-    T = eltype(TT)
-    rec = recursive && _needs_tracking_gap_to_julia(T)
-
     elm = Wrappers.ELM_MAT
 #T not for holes!
     ret_val = TT(undef, nrows, ncols)::TT
 
+    T = eltype(TT)
+    rec = recursive && _needs_tracking_gap_to_julia(T)
     rec_dict = recursion_info_j(TT, obj, rec, recursion_dict)
     recursion_dict = handle_recursion((obj, TT), ret_val, rec, rec_dict)
 
@@ -188,18 +186,17 @@ function gap_to_julia_internal(
 
     recursive && recursion_dict !== nothing && haskey(recursion_dict, (obj, TT)) && return recursion_dict[(obj, TT)]
 
-    T = eltype(TT)
-    rec = recursive && _needs_tracking_gap_to_julia(T)
-
     ret_val = TT()
 
+    T = eltype(TT)
+    rec = recursive && _needs_tracking_gap_to_julia(T)
     rec_dict = recursion_info_j(TT, obj, rec, recursion_dict)
-    handle_recursion((obj, TT), ret_val, rec, rec_dict)
+    recursion_dict = handle_recursion((obj, TT), ret_val, rec, rec_dict)
 
     for i = 1:length(newobj)
         current_obj = ElmList(newobj, i)
         if (rec || !(current_obj isa T)) && !isbitstype(typeof(current_obj))
-            push!(ret_val, gap_to_julia_internal(T, current_obj, rec_dict, BoolVal(rec)))
+            push!(ret_val, gap_to_julia_internal(T, current_obj, recursion_dict, BoolVal(rec)))
         else
             push!(ret_val, current_obj)
         end
@@ -246,7 +243,6 @@ function gap_to_julia_internal(
 
       # Switch off recursion if none of the entry types needs recursion.
       rec = recursive && (_needs_tracking_gap_to_julia(S) || any(X ->_needs_tracking_gap_to_julia(X), parameters[1:(len-1)]))
-
       rec_dict = recursion_info_j(TT, obj, rec, recursion_dict)
 
       list = [
@@ -274,7 +270,7 @@ function gap_to_julia_internal(
     end
 
     ret_val = TT(list)
-    recursion_dict = handle_recursion((obj, TT), ret_val, rec, rec_dict)
+    handle_recursion((obj, TT), ret_val, rec, rec_dict)
     return ret_val
 end
 
@@ -303,7 +299,6 @@ function gap_to_julia_internal(
     ret_val = TT()
 
     rec = recursive && _needs_tracking_gap_to_julia(T)
-
     rec_dict = recursion_info_j(TT, obj, rec, recursion_dict)
     recursion_dict = handle_recursion((obj, TT), ret_val, rec, rec_dict)