[PATCH] D37383: [AST] Add TableGen for StmtDataCollectors

2017-09-06 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL312634: [AST] Add TableGen for StmtDataCollectors (authored by krobelus). Changed prior to commit: https://reviews.llvm.org/D37383?vs=113547=114000#toc Repository: rL LLVM

[PATCH] D37383: [AST] Add TableGen for StmtDataCollectors

2017-09-06 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. @johannes The blocking reviewer is because it touches clone detection code :) Fine with me, I have some comments on things but nothing that affects this review. LGTM!

[PATCH] D37383: [AST] Add TableGen for StmtDataCollectors

2017-09-06 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes added a comment. @teemperor ok for you? did phabricator make you a blocking reviewer because of the affected code, or did I do that somehow? https://reviews.llvm.org/D37383 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D37383: [AST] Add TableGen for StmtDataCollectors

2017-09-04 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman added a comment. LGTM with one inline comment below: Comment at: utils/TableGen/TableGen.cpp:151 clEnumValN(GenOptDocs, "gen-opt-docs", "Generate option documentation"), +clEnumValN(GenDataCollectors, "gen-clang-data-collectors", "Generate data

[PATCH] D37383: [AST] Add TableGen for StmtDataCollectors

2017-09-04 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman accepted this revision. arphaman added a comment. `def`s could work, but I think that we should stick with TableGen. As you've said, we'd be able to introduce some sort of structure instead of just code in the future which will be better (Ideally we'd try to do it now, but given the

[PATCH] D37383: [AST] Add TableGen for StmtDataCollectors

2017-09-01 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes added a comment. In https://reviews.llvm.org/D37383#858905, @teemperor wrote: > @arphaman I suggested tablegen in https://reviews.llvm.org/D36664 because I > remembered we had some CMake sanity check about not having an *.inc in our > include dir: >

[PATCH] D37383: [AST] Add TableGen for StmtDataCollectors

2017-09-01 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman added a comment. In https://reviews.llvm.org/D37383#858905, @teemperor wrote: > @arphaman I suggested tablegen in https://reviews.llvm.org/D36664 because I > remembered we had some CMake sanity check about not having an *.inc in our > include dir: >

[PATCH] D37383: [AST] Add TableGen for StmtDataCollectors

2017-09-01 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. @arphaman I suggested tablegen in https://reviews.llvm.org/D36664 because I remembered we had some CMake sanity check about not having an *.inc in our include dir: https://github.com/llvm-mirror/clang/blob/master/CMakeLists.txt#L266 Not sure if it actually fires for

[PATCH] D37383: [AST] Add TableGen for StmtDataCollectors

2017-09-01 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman added a comment. TableGen is great for structured data, but we shouldn't use it just to embed code in it. What was the issue you've had with the build when the inc file was in include? https://reviews.llvm.org/D37383 ___ cfe-commits

[PATCH] D37383: [AST] Add TableGen for StmtDataCollectors

2017-09-01 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision. Herald added a subscriber: mgorny. Herald added 1 blocking reviewer(s): teemperor. This adds an option "-gen-clang-data-collectors" to the Clang TableGen that is used to generate StmtDataCollectors.inc. https://reviews.llvm.org/D37383 Files: