search

Manu343726 / siplasplas

A library for C++ reflection and introspection
https://manu343726.github.io/siplasplas
MIT License
195 stars 27 forks source link

DRLParser fails while building the master branch #77

Open sztomi opened 7 years ago

sztomi commented 7 years ago

After setting the flag to download clang and having installed ncurses5-compat-libs, I'm observing an exception in DRLParser. Relevant parts of the output:

siplasplas reflection parser
============================

==> Scanning search directories:
  -> /home/sztomi/src/siplasplas/include/siplasplas/fswatch
  -> /home/sztomi/src/siplasplas/include/siplasplas/signals
  -> /home/sztomi/src/siplasplas/include/siplasplas/reflection/static
  -> /home/sztomi/src/siplasplas/include/siplasplas/typeerasure
  -> /home/sztomi/src/siplasplas/include/siplasplas/reflection/common
  -> /home/sztomi/src/siplasplas/include/siplasplas/utility
  -> /home/sztomi/src/siplasplas/include/siplasplas/logger
  -> /home/sztomi/src/siplasplas/include/siplasplas/constexpr

==> Processing 69 files:
[  1%] (outdated) include/siplasplas/constexpr/algorithm.hpp
[  2%] (outdated) include/siplasplas/constexpr/arrayview.hpp
[  4%] (outdated) include/siplasplas/constexpr/assert.hpp
Traceback (most recent call last):
  File "/home/sztomi/src/siplasplas/src/reflection/parser/DRLParser", line 238, in <module>
    App().run()
  File "/home/sztomi/src/siplasplas/src/reflection/parser/DRLParser", line 235, in run
    self.compiler.run()
  File "/home/sztomi/src/siplasplas/src/reflection/parser/DRLParser", line 86, in run
    self.process()
  File "/home/sztomi/src/siplasplas/src/reflection/parser/DRLParser", line 72, in process
    tu.process(self.verbose)
  File "/home/sztomi/src/siplasplas/src/reflection/parser/translationunitprocessor.py", line 49, in process
    self.translation_unit = TranslationUnit(self.clang_tu.cursor, self.filePath)
  File "/home/sztomi/src/siplasplas/src/reflection/parser/ast/translationunit.py", line 55, in __init__
    self.root = Namespace.global_namespace(self)
  File "/home/sztomi/src/siplasplas/src/reflection/parser/ast/namespaces.py", line 32, in global_namespace
    return Namespace.create_node(cursor = translation_unit.cursor, translation_unit = translation_unit, file = translation_unit.file)
  File "/home/sztomi/src/siplasplas/src/reflection/parser/ast/node.py", line 125, in create_node
    nodeClass.initialize_children(node)
  File "/home/sztomi/src/siplasplas/src/reflection/parser/ast/node.py", line 160, in initialize_children
    child = nodeClass.create_child(cursor = c, parent = node)
  File "/home/sztomi/src/siplasplas/src/reflection/parser/ast/node.py", line 115, in create_child
    return class_.create_node(**kwargs)
  File "/home/sztomi/src/siplasplas/src/reflection/parser/ast/node.py", line 125, in create_node
    nodeClass.initialize_children(node)
  File "/home/sztomi/src/siplasplas/src/reflection/parser/ast/node.py", line 160, in initialize_children
    child = nodeClass.create_child(cursor = c, parent = node)
  File "/home/sztomi/src/siplasplas/src/reflection/parser/ast/node.py", line 115, in create_child
    return class_.create_node(**kwargs)
  File "/home/sztomi/src/siplasplas/src/reflection/parser/ast/node.py", line 125, in create_node
    nodeClass.initialize_children(node)
  File "/home/sztomi/src/siplasplas/src/reflection/parser/ast/node.py", line 160, in initialize_children
    child = nodeClass.create_child(cursor = c, parent = node)
  File "/home/sztomi/src/siplasplas/src/reflection/parser/ast/node.py", line 115, in create_child
    return class_.create_node(**kwargs)
  File "/home/sztomi/src/siplasplas/src/reflection/parser/ast/node.py", line 125, in create_node
    nodeClass.initialize_children(node)
  File "/home/sztomi/src/siplasplas/src/reflection/parser/ast/node.py", line 160, in initialize_children
    child = nodeClass.create_child(cursor = c, parent = node)
  File "/home/sztomi/src/siplasplas/src/reflection/parser/ast/node.py", line 115, in create_child
    return class_.create_node(**kwargs)
  File "/home/sztomi/src/siplasplas/src/reflection/parser/ast/node.py", line 125, in create_node
    nodeClass.initialize_children(node)
  File "/home/sztomi/src/siplasplas/src/reflection/parser/ast/node.py", line 159, in initialize_children
    if c.kind in mapping:
  File "/usr/lib/python2.7/site-packages/clang/cindex.py", line 1271, in kind
    return CursorKind.from_id(self._kind_id)
  File "/usr/lib/python2.7/site-packages/clang/cindex.py", line 555, in from_id
    raise ValueError,'Unknown template argument kind %d' % id
ValueError: Unknown template argument kind 602
make[2]: *** [src/fswatch/CMakeFiles/siplasplas-fswatch_drlparser.dir/build.make:58: siplasplas-fswatch_drlparser] Error 1
make[1]: *** [CMakeFiles/Makefile2:1722: src/fswatch/CMakeFiles/siplasplas-fswatch_drlparser.dir/all] Error 2
make: *** [Makefile:161: all] Error 2

I ran:

mkdir build && cd build
cmake ..
make
sztomi commented 7 years ago

Adding

CursorKind.STATIC_ASSERT = CursorKind(602)

To /usr/lib/python2.7/site-packages/clang/cindex.py solves the problem, but obviously it's not a great idea. Maybe it would make sense to ship a patched cindex.py with siplasplas until the patch is upstream.

Manu343726 commented 7 years ago

Maybe the long-term solution to all this problems is to provide our own versioned versions of cindex.py, one for each tested clang version. This may include some hotfixes like yours. In general, a way to avoid relying in external unstable releases if possible.

Note: In the future, I would like to deprecate the python DRLParser and rewrite everything using C++ and the libclang C API

sztomi commented 7 years ago

I would be interested in contributing to that.

Manu343726 commented 7 years ago

Cool, feel free to send PRs to the master branch ;)

What I would do:

sztomi commented 7 years ago

I think it's enough to have just one cindex.py because in my experience the incompatibilities stem from certain things not being exposed (like the CursorKind above), and not things like changing APIs (after all, libclang aims to be a stable, backwards-compatible library). That implies that there is no need to change the scripts that import it if it's in the correct directory. By the way, I mainly meant contributing to the C++ rewrite of DRLparser, but I can help with this as well :)

Manu343726 commented 7 years ago

By the way, I mainly meant contributing to the C++ rewrite of DRLparser, but I can help with this as well

OH. That's even better! I haven't done before due to lack of time and, frankly, being a bit lazy... If we are going to continue this way, @foonathan 's standardese is a great reference.

Manu343726 commented 7 years ago

One of the things I have been thinking for months was to directly take Standardese backend and adapt the generation part to generate the siplasplas reflection metadata. This way we:

Manu343726 commented 7 years ago

I think it's enough to have just one cindex.py

So, we could maintain a cindex.py file compatible with all the clang versions we support (Something we must define and fix first)

sztomi commented 7 years ago

One of the things I have been thinking for months was to directly take Standardese backend

Good idea, I'll take a look.

Something we must define and fix first

I think the minimal version is the first clang version that supports the C++ standard that siplasplas wants to support. If it's C++14, then if I'm not mistaken it's 3.7(?)

Manu343726 commented 7 years ago

I think the minimal version is the first clang version that supports the C++ standard that siplasplas wants to support. If it's C++14, then if I'm not mistaken it's 3.7(?)

I have been trying to maintain C++11 compatibility at least in the static reflection API, so the DRLParser generated code and the reflection API should be C++11 compatible. (I think there are some minor issues, last time I tried it worked with a GCC 4.8 with some warning about generic lambdas).

sztomi commented 7 years ago

Since C++ standards are mostly backwards-compatible, it's not a bad idea to opt for a minimal version of clang that supports a newer standard (i.e. to be able to parse the widest set of possible inputs). The generated code should not depend on this at all. ANd libclang supports the -std flag.

sztomi commented 7 years ago

Do you want to rewrite all of DRLParser in C++ or only replace the actual parsing with Standardese? I think the latter would be fairly easy if we made a Cython wrapper around Standardese. The jinja template would probably have to be changed a bit. This could also be used as an intermediate step towards a full rewrite.

Manu343726 commented 7 years ago

I want to replace the whole DRLParser tool to get rid of python dependencies. Also, writing DRLPaser in C++ makes it easier to write a C++ API to invoke it programatically (Which I would like to have for runtime c++ compilation)

sztomi commented 7 years ago

I'm looking at a replacement for Jinja and found this: https://github.com/hughperkins/Jinja2CppLight

At a first glance, it misses macros that is feature that siplasplas templates seem to heavily depend on. Are there other features missing?

This is another approach: https://github.com/hughperkins/luacpptemplater which might be easier to implement macros with. Maybe @hughperkins has some input?

hughperkins commented 7 years ago

I use the luacpptemplater for runtime generation of opencl kernels from templates from c++, which works very well for me. The syntax is slightly different from jinja2, a bit uglier I would say, but it's a full scripting / macro language, whereas the jinjacpplight either is very lightweight (current form) and misses stuff, or else would become it's own entire scripting language, which seems redundant somehow?

On 7 December 2016 23:54:51 CET, "Tamás Szelei" notifications@github.com wrote:

I'm looking at a replacement for Jinja and found this: https://github.com/hughperkins/Jinja2CppLight

At a first glance, it misses macros that is feature that siplasplas templates seem to heavily depend on. Are there other features missing?

This is another approach: https://github.com/hughperkins/luacpptemplater which might be easier to implement macros with. Maybe @hughperkins has some input?

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/Manu343726/siplasplas/issues/77#issuecomment-265599999

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

sztomi commented 7 years ago

I can see where you are coming from. I agree that the lua option is fairly good for this, what do you think, @Manu343726 ? (There is also a lua package on conan). This is probably the direction with the least resistance, because changing the template syntax is way less work than adding the missing features to jinjacpplight.

Manu343726 commented 7 years ago

I looked at Jinja2CppLight previously when searching jinja2 replacements, but didn't know about luacpptemplater. I think it is perfect for our use case.

The only thing I'm worried about is conan.io support of this dependency, since I want to completely switch to conan for thirdparty management (See #78). But the project looks simple to port to conan ;)

sztomi commented 7 years ago

@hughperkins is it possible to define a lua function in the template as a "macro"? What I mean:

{% function something() %}
  hey, render this
{% endfunction %}

{% something() %}
{% something() %}

would output

  hey, render this
  hey, render this
hughperkins commented 7 years ago

is it possible to define a lua function in the template as a "macro"?

Interesting question. I added a test at https://github.com/hughperkins/luacpptemplater/commit/38cc75272d86c8311b91bb069620770a6b9d447b to check this point. This test passes.

Note that the lua templater code idea and core implementation came from John Nachtimwald https://john.nachtimwald.com/2014/08/06/using-lua-as-a-templating-engine/ . I'm not sure why I didnt point this out in the readme before, but I've added it now https://github.com/hughperkins/luacpptemplater/commit/9611b32ca7882c325c4290114cfb9746d67b8994

sztomi commented 7 years ago

That's great, thank you!

Manu343726 commented 7 years ago

@hughperkins thank you for the support! @sztomi I'm currently working on migrating dependencies to conan, I hope I can start with the DRLParser rewrite by next week. Anyway, thanks a lot for the feedback, I'm really looking forward to review your PRs ;)

sztomi commented 7 years ago

In the meantime I started to freshen up luacpptemplater a bit. I'll take a stab at making a conan package as well. https://github.com/sztomi/luacpptemplater