From 55cce99368e815d4303887a83bc409e992b6c6c2 Mon Sep 17 00:00:00 2001 From: Max Horn Date: Mon, 5 Nov 2018 21:36:01 +0100 Subject: [PATCH] Use julia_gap and gap_julia in more places ... for consistency in the auto-conversion behaviour of JuliaInterface versus LibGAP.jl, and for ease of use. There is more work to be done here, though, and we need to properly document the rules for when and where which conversion is applied, and why. --- JuliaExperimental/tst/realcyc.tst | 2 +- JuliaInterface/gap/JuliaInterface.gd | 4 +-- JuliaInterface/gap/JuliaInterface.gi | 14 ++++++---- JuliaInterface/src/JuliaInterface.c | 25 +++++++++-------- JuliaInterface/src/calls.c | 9 ++---- JuliaInterface/tst/calls.tst | 42 ++++++++++++++-------------- JuliaInterface/tst/utils.tst | 2 +- 7 files changed, 48 insertions(+), 50 deletions(-) diff --git a/JuliaExperimental/tst/realcyc.tst b/JuliaExperimental/tst/realcyc.tst index 5603312a0..7d28dc7a8 100644 --- a/JuliaExperimental/tst/realcyc.tst +++ b/JuliaExperimental/tst/realcyc.tst @@ -37,7 +37,7 @@ false ## gap> Julia.GAPRealCycModule.test_this_module(); - +true ## gap> STOP_TEST( "realcyc.tst", 1 ); diff --git a/JuliaInterface/gap/JuliaInterface.gd b/JuliaInterface/gap/JuliaInterface.gd index d2d0e0ca9..a4b34d6d7 100644 --- a/JuliaInterface/gap/JuliaInterface.gd +++ b/JuliaInterface/gap/JuliaInterface.gd @@ -44,7 +44,7 @@ DeclareOperation( "ConvertedToJulia", [ IsObject ] ); #! @Description #! Retuns an object that is a sensible conversion from the Julia object julia_object. #! If no conversion exists, fail is returned. -DeclareOperation( "ConvertedFromJulia", [ IsJuliaObject ] ); +DeclareOperation( "ConvertedFromJulia", [ IsObject ] ); #! @Arguments julia_object #! @Returns an object @@ -139,5 +139,3 @@ DeclareGlobalFunction( "JuliaTypeInfo" ); #! If an error occured then ok has the value false, #! and value is the error message as a &GAP; string. DeclareGlobalFunction( "CallJuliaFunctionWithCatch" ); - - diff --git a/JuliaInterface/gap/JuliaInterface.gi b/JuliaInterface/gap/JuliaInterface.gi index 5e3291251..63ccbc425 100644 --- a/JuliaInterface/gap/JuliaInterface.gi +++ b/JuliaInterface/gap/JuliaInterface.gi @@ -28,6 +28,10 @@ InstallMethod( ConvertedFromJulia, [ IsJuliaObject ], _ConvertedFromJulia ); +InstallMethod( ConvertedFromJulia, + [ IsObject ], + IdFunc ); + InstallMethod( ConvertedToJulia, [ IsObject ], function( obj ) @@ -91,9 +95,9 @@ InstallMethod( \., Error( rnam, " is not bound in Julia" ); fi; - if ConvertedFromJulia( _JULIA_ISA( global_variable, _JULIA_FUNCTION_TYPE ) ) then + if _JULIA_ISA( global_variable, _JULIA_FUNCTION_TYPE ) then global_variable := _JuliaFunction( global_variable, true ); - elif ConvertedFromJulia( _JULIA_ISA( global_variable, _JULIA_MODULE_TYPE ) ) then + elif _JULIA_ISA( global_variable, _JULIA_MODULE_TYPE ) then global_variable := _WrapJuliaModule( rnam, global_variable ); fi; @@ -195,7 +199,7 @@ InstallGlobalFunction( ImportJuliaModuleIntoGAP, is_module_present := JuliaEvalString( Concatenation( "isdefined( Main, :", name, ")" ) ); if no_import = false then - if not ConvertedFromJulia( is_module_present ) then + if not is_module_present then ## Local modules cannot be imported callstring:= Concatenation( "import ", name ); JuliaEvalString( callstring ); @@ -218,7 +222,7 @@ InstallGlobalFunction( JuliaImportPackage, function( pkgname ) fi; callstring := Concatenation( "try import ", pkgname, "; return true; catch e; return e; end" ); - if ConvertedFromJulia( JuliaEvalString( callstring ) ) = true then + if JuliaEvalString( callstring ) = true then return true; else Info( InfoWarning, 1, @@ -238,7 +242,7 @@ InstallGlobalFunction( CallJuliaFunctionWithCatch, local res; res:= Julia.GAPUtils.call_with_catch( juliafunc, arguments ); - if ConvertedFromJulia( res[1] ) = true then + if res[1] then return rec( ok:= true, value:= res[2] ); else return rec( ok:= false, value:= ConvertedFromJulia( res[2] ) ); diff --git a/JuliaInterface/src/JuliaInterface.c b/JuliaInterface/src/JuliaInterface.c index aad04da70..9af0909fa 100644 --- a/JuliaInterface/src/JuliaInterface.c +++ b/JuliaInterface/src/JuliaInterface.c @@ -5,6 +5,7 @@ #include "JuliaInterface.h" #include "calls.h" +#include "convert.h" #include // GAP headers @@ -215,7 +216,7 @@ static Obj FuncJuliaEvalString(Obj self, Obj string) // JL_TRY/JL_CATCH. jl_value_t * result = jl_eval_string(copy); JULIAINTERFACE_EXCEPTION_HANDLER - return NewJuliaObj(result); + return gap_julia(result); } // Converts the julia value pointer into a GAP object @@ -288,6 +289,8 @@ Obj _ConvertedFromJulia_internal(jl_value_t * julia_obj) continue; } jl_value_t * current_jl_element = jl_arrayref(array_ptr, i); + // TODO: replace NewJuliaObj here by gap_julia, or + // by a recursive call to _ConvertedFromJulia_internal? current_element = NewJuliaObj(current_jl_element); SET_ELM_PLIST(return_list, i + 1, current_element); CHANGED_BAG(return_list); @@ -455,8 +458,10 @@ static Obj Func_ConvertedFromJulia_record_dict(Obj self, Obj dict) real_index++; current_value = jl_arrayref(dict_values, i); current_index = jl_arrayref(dict_indices, i); + // FIXME: should we use gap_julia here instead of NewJuliaObj? SET_ELM_PLIST(values_gap, real_index, NewJuliaObj(current_value)); CHANGED_BAG(values_gap); + // FIXME: should we use gap_julia here instead of NewJuliaObj? SET_ELM_PLIST(indices_gap, real_index, NewJuliaObj(current_index)); CHANGED_BAG(indices_gap); } @@ -529,18 +534,14 @@ static Obj FuncJuliaModule(Obj self, Obj name) return NewJuliaObj((jl_value_t *)julia_module); } -// Sets the value of the julia identifier to the julia value the julia -// object GAP object points to. -// This function is for debugging purposes -static Obj FuncJuliaSetVal(Obj self, Obj name, Obj julia_val) +// Sets the value of the julia identifier to the . +// This function is for debugging purposes. +static Obj FuncJuliaSetVal(Obj self, Obj name, Obj val) { if (!IsStringConv(name)) { ErrorMayQuit("JuliaSetVal: must be a string", 0, 0); } - if (!IS_JULIA_OBJ(julia_val)) { - ErrorMayQuit("JuliaSetVal: must be a Julia object", 0, 0); - } - jl_value_t * julia_obj = GET_JULIA_OBJ(julia_val); + jl_value_t * julia_obj = julia_gap(val); jl_sym_t * julia_symbol = jl_symbol(CSTR_STRING(name)); jl_set_global(jl_main_module, julia_symbol, julia_obj); return 0; @@ -560,7 +561,7 @@ static Obj Func_JuliaGetGlobalVariable(Obj self, Obj name) return Fail; } jl_value_t * value = jl_get_global(jl_main_module, symbol); - return NewJuliaObj(value); + return gap_julia(value); } // Returns the julia object GAP object that holds a pointer to the value @@ -591,7 +592,7 @@ static Obj Func_JuliaGetGlobalVariableByModule(Obj self, Obj name, Obj module) return Fail; } jl_value_t * value = jl_get_global(m, symbol); - return NewJuliaObj(value); + return gap_julia(value); } // Returns the julia object GAP object that holds a pointer to the value @@ -617,7 +618,7 @@ static Obj FuncJuliaGetFieldOfObject(Obj self, Obj super_obj, Obj field_name) jl_value_t * field_value = jl_get_field(extracted_superobj, CSTR_STRING(field_name)); JULIAINTERFACE_EXCEPTION_HANDLER - return NewJuliaObj(field_value); + return gap_julia(field_value); } diff --git a/JuliaInterface/src/calls.c b/JuliaInterface/src/calls.c index 21aae5e81..599a2336b 100644 --- a/JuliaInterface/src/calls.c +++ b/JuliaInterface/src/calls.c @@ -101,10 +101,7 @@ static ALWAYS_INLINE Obj DoCallJuliaFunc(Obj func, } else { for (int i = 0; i < narg; i++) { - if (IS_INTOBJ(a[i])) - a[i] = (Obj)jl_box_int64(INT_INTOBJ(a[i])); - else if (IS_FFE(a[i])) - ErrorQuit("TODO: implement conversion for T_FFE", 0, 0); + a[i] = (Obj)julia_gap(a[i]); } } jl_function_t * f = GET_JULIA_FUNC(func); @@ -129,9 +126,7 @@ static ALWAYS_INLINE Obj DoCallJuliaFunc(Obj func, // and its variants are part of the jlapi, so don't have to be wrapped in // JL_TRY/JL_CATCH. JULIAINTERFACE_EXCEPTION_HANDLER - if (IsGapObj(result)) - return (Obj)result; - return NewJuliaObj(result); + return gap_julia(result); } static Obj DoCallJuliaFunc0ArgConv(Obj func) diff --git a/JuliaInterface/tst/calls.tst b/JuliaInterface/tst/calls.tst index 7a2827577..c6e3a257d 100644 --- a/JuliaInterface/tst/calls.tst +++ b/JuliaInterface/tst/calls.tst @@ -176,31 +176,31 @@ gap> fwN(); # gap> fwN(true); - + # gap> fwN(true,2); - + # gap> fwN(true,2,3); - + # gap> fwN(true,2,3,4); - + # gap> fwN(true,2,3,4,5); - + # gap> fwN(true,2,3,4,5,6); - + # gap> fwN(true,2,3,4,5,6,7); - + # non-variadic functions gap> f0wN := _JuliaFunction("f0", false);; @@ -218,31 +218,31 @@ gap> f0wN(); # gap> f1wN(true); - + # gap> f2wN(true,2); - + # gap> f3wN(true,2,3); - + # gap> f4wN(true,2,3,4); - + # gap> f5wN(true,2,3,4,5); - + # gap> f6wN(true,2,3,4,5,6); - + # gap> f7wN(true,2,3,4,5,6,7); - + # # calls via wrapped C function pointers @@ -280,17 +280,17 @@ Error, Only 0-6 arguments are supported gap> g0(); gap> g1(true); - +true gap> g2(true,2); - +2 gap> g3(true,2,3); - +3 gap> g4(true,2,3,4); - +4 gap> g5(true,2,3,4,5); - +5 gap> g6(true,2,3,4,5,6); - +6 # gap> g0C(); @@ -344,7 +344,7 @@ Error, TypeError: in h6, in cfunction, expected Ptr{Nothing}, got Nothing # gap> _NewJuliaCFunc(fail, fail); Error, NewJuliaCFunc: must be a Julia object -gap> _NewJuliaCFunc(JuliaEvalString("1"), fail); +gap> _NewJuliaCFunc(JuliaEvalString("'a'"), fail); Error, NewJuliaCFunc: must be plain list # diff --git a/JuliaInterface/tst/utils.tst b/JuliaInterface/tst/utils.tst index db5aa4654..b3537ab78 100644 --- a/JuliaInterface/tst/utils.tst +++ b/JuliaInterface/tst/utils.tst @@ -32,7 +32,7 @@ gap> JuliaSetVal(fail, 1); Error, JuliaSetVal: must be a string gap> JuliaSetVal("foo", JuliaEvalString("1")); gap> JuliaGetGlobalVariable("foo"); - +1 ## gap> JuliaTuple([]);