JuliaInterop / Cxx.jl

The Julia C++ Interface
Other
755 stars 107 forks source link

fix usage in precompiled modules: eliminate sourcebuffers #474

Closed stevengj closed 4 years ago

stevengj commented 4 years ago

Fixes #449.

The cxx"..." string macros were caching their code in a global sourcebuffers array, and then passing an index into this array with SourceBuf{id} to a @generated function so that id is visible at compile-time (being encoded a type). However, because this sourcebuffers array is generated when Cxx is precompiled, it is incompatible with using Cxx in other precompiled modules (which cannot modify sourcebuffers in a way that persists beyond precompilation).

Instead, this PR eliminates the sourcebuffers array and simply passes all of the source data as parameters in a SourceBuf type, translating the code to a Symbol. This serves the same purpose of storing the data in a fashion that is persistent and visible in the type-domain, but without causing problems for precompiled modules.

Keno commented 4 years ago

This seems fine to be (Cxx predates precompilation, so it didn't use to be an issue). That said, I don't think it solves the problem entirely. Cxx has a lot of state that does not get serialized (which headers were included, global C++ state, LLVM state). I had at one point hoped to get around to fixing that, but it's a decent size project and was never quite a priority.

stevengj commented 4 years ago

@Keno, I agree that there are a number of other improvements that are possible along these lines. However, so far I've been able to use Cxx reasonably well in a precompiled module with this patch as long as I put some things into __init__ as needed.

(When using Cxx in a module, I'm also worried about composability with other modules using Cxx, e.g. not using a global compiler instance.)

stevengj commented 4 years ago

Okay to merge?

Keno commented 4 years ago

Totally fine with me, but @Gnimuc is the current maintainer, so I'd be more comfortable if he hit the button.

Gnimuc commented 4 years ago

Merged. All CIs are green.

BTW, @Keno do you have time to take a look at #465. I'm currently stuck in the following error:

/Cxx.cpp:1:1: error: 'data' following the 'template' keyword does not refer to a template
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/string:1242:23: note: declared as a non-template here
    const value_type* data() const _NOEXCEPT  {return _VSTD::__to_raw_pointer(__get_pointer());}
                      ^
/Cxx.cpp:1:1: error: 'size' following the 'template' keyword does not refer to a template
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/string:946:41: note: declared as a non-template here
    _LIBCPP_INLINE_VISIBILITY size_type size() const _NOEXCEPT