julia: added a new way to load GAPTypes

[sebasguts]

by getting module and object from a variable

Description

This fixes the bug here together with a patch for GAP.jl submitted here

Does not need to appear in release notes.

[coveralls]

Coverage decreased (-0.0008%) to 84.442% when pulling 41f78e6 on sebasguts:sebasguts/add_jl_gaptypes_fallback_loader into fcd1f51 on gap-system:master.

[fingolfin]

So, how about we instead get the GAP.jl module, from the already existing global var __JULIAGAPMODULE. Then, if it exists, it will also have the GAPTypes module as a member, and from that we can get GapObj, no?

[fingolfin]

Probably also wouldn't hurt to verify we are really getting a module, via jl_is_module

[fingolfin]

Have you tested this? is there really an exception here. From the quick glance at the Julia code, my impression was that jl_get_global returns NULL if the binding does not exist. Also, jl_exception_occurred w/o an explicit exception handler only works for a very, very small set of APIs explicitly designated for this purpose, and I am pretty sure jl_get_global is not one of them

[fingolfin]

Thanks for working on this.

Ideally, we should finish this ASAP and then backport it to 4.11

[sebasguts]

I think I addressed your comments, and added a lot of checks. I will close the corresponding GAP.jl now.

[fingolfin]

Suggested change

	UInt gapjl_already_loaded = 1;
	UInt gapjl_already_loaded = 0;

[fingolfin]

Suggested change

gapjl_already_loaded = 0;

[fingolfin]

I am still surprised that the jl_boundp check is needed. Here is the code for it:

JL_DLLEXPORT int jl_boundp(jl_module_t *m, jl_sym_t *var)
{
    jl_binding_t *b = jl_get_binding(m, var);
    return b && (b->value != NULL);
}

... and here is the code for jl_get_global:

JL_DLLEXPORT jl_value_t *jl_get_global(jl_module_t *m, jl_sym_t *var)
{
    jl_binding_t *b = jl_get_binding(m, var);
    if (b == NULL) return NULL;
    if (b->deprecated) jl_binding_deprecation_warning(m, b);
    return b->value;
}

Based on that, I would assume that jl_get_global returns an error, and does not produce an exception. This is further supported by how the Julia kernel itself uses it, e.g. to quote form Julia's init.c:

...
        jl_value_t *f = jl_get_global(jl_base_module, jl_symbol("_atexit"));
        if (f != NULL) {
...

[fingolfin]

Actually, I just verified it (by running the code): it returns NULL, and does not return an exception.

I will update this PR with an alternative implementation for further discussion.

[fingolfin]

@sebasguts I refactored your code, please have a look. Also, perhaps @rbehrends could review this PR as well?

[rbehrends]

I have looked at the PR and the companion PR for GAP.jl. They look sensible to me, but I can't claim that I fully understand all of the module import logic (yet).

[sebasguts]

I think this should be merged. However, since this is my PR, I cannot approve it.

[fingolfin]

Backported to stable-4.11 in commits 1d383b7 and 5a72f72