Mitgation of script injection risk in CI [was Add support for a method_extras option]

[JamesWrigley]

This allows the user to specify files in the config file that will be inserted into the generated wrappers for specific types. It makes customizing the wrappers a bit easier, since otherwise the only option would be to take the generated wrappers, modify them, and either commit the changes or save the changes as a patch to be applied.

In the future this might need to be extended to add support for extra globals in JlGlobals.cxx.

[JamesWrigley]

I also attempted to fix the CI in 7cbbdaf, it was attempting to execute my backtick-quoted JlGlobals.cxx in the PR body as a command 😅

[grasph]

Hello James,

Thanks for the PR.

Another way to reach the same goal is to define the method as a global function that takes the class instance as its first parameter in a header file and include the header file in the input list.

You can find an example in ex002-ROOT, here Templates.h line 18. This lines add a GetTTree(::TTree, ...) Julia method, the same as a t.add_method(...) call with t the TTree wrapper will do. Actually if t.add_method(...) is called with a named function (in opposition to a lambda function), then it defines two methods, one with the first parameter passed as a reference and the other as a pointer. So to be complete TTree* GetTTree(TDirectoryFile* f, const char *namecycle) would also need to be defined.

In the example of your unit test, you use a lambda function and supports only reference. The extra header file will be:

inline int get(Foo&){
    return 42;
}

Support for pointer (CxxPtr) will be added with this extra line:

inline int get(Foo* p) { return get(*p); }

For your use case, this solution is in my opinion cleaner than inserting custom code in the generated code.

Note: if the need to define two versions of the methods is too inconvenient, this can improved. I have already ideas how to avoid it. I can explain them if wanted.

That said, there might be use cases that cannot be handled with wrapit, and for which insertion of custom code would be needed. Embedding custom code in function is tricky since you can have conflicts between the generated code, which can evolve with wrapit releases, and custom code. To handle such case, I think we should use hook functions. In you example, the code generated for add_methods() would include a call to add_methods_end_hook(t), with add_methods_end_hook a templated function the user can specialize.

Nevertheless, before adding hooks everywhere, I believe we should first have concrete use cases that cannot be handled without a hook.

Let me know if the provided solution, without hook or embedding code, suits your needs. I will add to the doc directory a FAQ including the description of this method.

Philippe.

[JamesWrigley]

Thanks for the reply, that's actually an excellent idea :) I didn't think of that. I'm trying it and it seems to be mostly successful, the only thing I'm struggling with is having a method that returns a jlcxx::Array. This example function in my header file:

inline jlcxx::Array<float> foo() {
    jlcxx::Array<float> tests{ };
    return tests;
}

Creates this wrapper in JlGlobals.cxx:

    t.method("foo", static_cast<int (*)() >(&foo));

For some reason the return type is being inferred as an int and that causes a compilation error, if I manually change the wrapper to be:

    t.method("foo", static_cast<jlcxx::Array<float> (*)() >(&foo));

Then compilation succeeds and the function can be called from Julia. Do you know what's up with that?

[JamesWrigley]

Ok, turns out it was because I needed to add the CxxWrap and julia.h include directories for clang. It's still not working because now I'm getting undefined reference errors, but I don't think that's related to wrapit.

I deleted the commit to add methods_extras, I think the other commits could be merged.