RFC: Refactor the package to make it self-generated and more #210
Conversation
Co-Authored-By: Martijn Visser <[email protected]>
let's make Clang.jl self-generated.
Co-Authored-By: Simon <[email protected]>
1. remove old cindex wrappers 2. add a new low-level module LibClang that is one-to-one mapped to libclang APIs and exports everything
|
I'll try to give this a read-through this weekend, very cool! |
Codecov Report
@@ Coverage Diff @@
## master #210 +/- ##
=========================================
Coverage ? 24.11%
=========================================
Files ? 23
Lines ? 1518
Branches ? 0
=========================================
Hits ? 366
Misses ? 1152
Partials ? 0
Continue to review full report at Codecov.
|
|
the CI failure on Linux might due to the compatibility of the LLVM binaries. I've released two windows binaries and it works well on my VM. |
|
This is great, thanks for the hard work. |
Gnimuc
changed the title
| error("llvm-config returned non-existent libdir path: '$(llvm_libdir)") | ||
| end | ||
| # Download binaries from hosted location | ||
| bin_prefix = "https://github.com/ihnorton/Clang.jl/releases/download/Julia1.0-compatible" |
There was a problem hiding this comment.
Cool! This looks like we actually want to have a "LLVMBuilder.jl" that has a hard-bound on the Julia version that you can depend on through the package-system, so that each version of Julia get's the right version of LLVMBuilder (and of course we should provide compatible releases (my apologies)). Also it means we should become a bit more conservative in what LLVMBuilder provides and maybe have a second mode for the more experimental features.
cc @staticfloat
There was a problem hiding this comment.
It saddens me that LLVM is so incompatible.
Let's create a JuliaCompatibleLLVM in Yggdrasil, register JuliaCompatibleLLVM.jl, hook up the appropriate BinaryProvider junk, and keep JuliaCompatibleLLVM as close to base Julia's LLVM as possible. We can keep LLVMBuilder with all the bells and whistles that others may want.
|
@Gnimuc I started taking a look through this. Wow! It is a lot of work and some really nice changes. Thank you! |
|
To start off at a high level, this change immediately stood out out:
Not sure either -- did you have considerations for performance that led in this direction? At first glance, it feels a little bit unwieldy for a high-level API (IIUC, have to pull out the enum, wrap in a |
|
It looks like this PR downgraded the package too much. 😅 For backward compatibility, it's also not a good idea to remove CL-prefixed types. But it seems very easy to get them back -- regenerate those types from the CX enums and define some conversion methods. I'll give it a test at this weekend. |
|
@ihnorton I've brought those CL-types back and it's indeed much faster than dispatching on |
|
gently bump. hi @ihnorton, is there any chance to get this PR merged before Julia-1.1 release? Since the LLVM & Clang used in Julia-1.1 may not be compatible with those in used Julia-1.0, we need to cap this package to Julia-1.0 firstly, tag a v0.8 release and then provide Julia-1.1 support and tag a v0.9 release. Or should we just jump over Julia-1.0 and directly target Julia-1.1? |
- see PR JuliaInterop/Clang.jl#210 - and discussion at JuliaInterop/Clang.jl#214
This sounds good! The compatibility of generated bindings is more important than what version Clang.jl will run on, so I think it is fine to target Julia 1.1. |
|
@Gnimuc this looks great. I looked through the generated files in scipopt/SCIP.jl#76 as the large test, and the output is very nice. I'm not sure what is up with the rebase conflict, but please go ahead and tag then merge when you are ready 👍 |
without this additional check, a common record followed by another anonymous typedef may mistakenly trigger a name substitution, for example:
```c
enum nk_tree_type {NK_TREE_NODE, NK_TREE_TAB};
typedef void*(*nk_plugin_alloc)(nk_handle, void *old, nk_size);
```
|
I also get rebase conflicts, so close this and submit a new PR. |
I'm investigating how to rebuild the package directly on the auto-generated libclang bindings. Almost done. Please give it a review at your convenience ;)
Change Summary
Clang.jldirectly on top of the auto-generated libclang C bindingsLibClangis one-to-one mapped to the libclang C APIs. it depends on two new files:CEnum.jlandctypes.jl.CEnum.jlis for better C-compatible enum handling.ctypes.jlcontains those C stdlib types that are missing corresponding Julia aliases. when generating wrappers, we should also ship these two files to users.cindex's base functions are now rebuild on top of LibClang. almost all of the old APIs are renamed, please take a look atindex.jl,trans_unit.jl,cursor.jl,type.jl,token.jlandshow.jlfor details.modulecindex's CL-prefixed types are removed, since methods are now directly dynamic-dispatched onVal{CEnum}value types(not sure whether this is a good idea or not).wrap_c.jland add an AbstractContext type for extensibilitywrap!for certain cursor declsand dispatched on their own context.
init+runare rebuild on top ofDefaultContext. also, oldcindexandwrap_cmodules can be loaded withClang.Deprecated.cindex; using Clang.Deprecated.wrap_c;LLVMBuilderare still not compatible with Julia-1.0, so I built a compatible version with an older version ofBinaryBuilderlocally and uploaded on the Github releases of this repo.pwd()==usr/lib#39 since with BB2 we install those headers in the same place on all platformsunrecognized command line option '-rdynamic') #145(obsolete), close runtests.jl Segmentation fault- clang_getTranslationUnitCursor at /usr/bin/../lib/x86_64-linux-gnu/libclang-3.6.so.1 #142(looks like an installation problem), close Failed to find libwrapclang #75(obsolete), close failure to build on OS X (v0.0.0 and master) #49(obsolete)https://github.com/ihnorton/Clang.jl/blob/1c5f425dc4cdd469f0c66a3c667592edf42dd4e2/src/wrap_c.jl#L712-L716