[PATCH] D37383: [AST] Add TableGen for StmtDataCollectors
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
https://reviews.llvm.org/D37383
Files:
cfe/trunk/include/clang/AST/CMakeLists.txt
cfe/trunk/include/clang/AST/StmtDataCollectors.td
cfe/trunk/lib/AST/StmtDataCollectors.inc
cfe/trunk/lib/Analysis/CloneDetection.cpp
cfe/trunk/unittests/AST/DataCollectionTest.cpp
cfe/trunk/utils/TableGen/CMakeLists.txt
cfe/trunk/utils/TableGen/ClangDataCollectorsEmitter.cpp
cfe/trunk/utils/TableGen/TableGen.cpp
cfe/trunk/utils/TableGen/TableGenBackends.h
Index: cfe/trunk/include/clang/AST/CMakeLists.txt
===
--- cfe/trunk/include/clang/AST/CMakeLists.txt
+++ cfe/trunk/include/clang/AST/CMakeLists.txt
@@ -50,3 +50,6 @@
SOURCE CommentCommands.td
TARGET ClangCommentCommandList)
+clang_tablegen(StmtDataCollectors.inc -gen-clang-data-collectors
+ SOURCE StmtDataCollectors.td
+ TARGET StmtDataCollectors)
Index: cfe/trunk/include/clang/AST/StmtDataCollectors.td
===
--- cfe/trunk/include/clang/AST/StmtDataCollectors.td
+++ cfe/trunk/include/clang/AST/StmtDataCollectors.td
@@ -0,0 +1,242 @@
+class Stmt {
+ code Code = [{
+addData(S->getStmtClass());
+// This ensures that non-macro-generated code isn't identical to
+// macro-generated code.
+addData(data_collection::getMacroStack(S->getLocStart(), Context));
+addData(data_collection::getMacroStack(S->getLocEnd(), Context));
+ }];
+}
+
+class Expr {
+ code Code = [{
+addData(S->getType());
+ }];
+}
+
+//--- Builtin functionality --//
+class ArrayTypeTraitExpr {
+ code Code = [{
+addData(S->getTrait());
+ }];
+}
+class ExpressionTraitExpr {
+ code Code = [{
+addData(S->getTrait());
+ }];
+}
+class PredefinedExpr {
+ code Code = [{
+addData(S->getIdentType());
+ }];
+}
+class TypeTraitExpr {
+ code Code = [{
+addData(S->getTrait());
+for (unsigned i = 0; i < S->getNumArgs(); ++i)
+ addData(S->getArg(i)->getType());
+ }];
+}
+
+//--- Calls --//
+class CallExpr {
+ code Code = [{
+// Function pointers don't have a callee and we just skip hashing it.
+if (const FunctionDecl *D = S->getDirectCallee()) {
+ // If the function is a template specialization, we also need to handle
+ // the template arguments as they are not included in the qualified name.
+ if (auto Args = D->getTemplateSpecializationArgs()) {
+std::string ArgString;
+
+// Print all template arguments into ArgString
+llvm::raw_string_ostream OS(ArgString);
+for (unsigned i = 0; i < Args->size(); ++i) {
+ Args->get(i).print(Context.getLangOpts(), OS);
+ // Add a padding character so that 'foo()' != 'foo()'.
+ OS << '\n';
+}
+OS.flush();
+
+addData(ArgString);
+ }
+ addData(D->getQualifiedNameAsString());
+}
+ }];
+}
+
+//--- Value references ---//
+class DeclRefExpr {
+ code Code = [{
+addData(S->getDecl()->getQualifiedNameAsString());
+ }];
+}
+class MemberExpr {
+ code Code = [{
+addData(S->getMemberDecl()->getName());
+ }];
+}
+
+//--- Literals ---//
+class IntegerLiteral {
+ code Code = [{
+addData(llvm::hash_value(S->getValue()));
+ }];
+}
+class FloatingLiteral {
+ code Code = [{
+addData(llvm::hash_value(S->getValue()));
+ }];
+}
+class StringLiteral {
+ code Code = [{
+addData(S->getString());
+}];
+}
+class CXXBoolLiteralExpr {
+ code Code = [{
+addData(S->getValue());
+ }];
+}
+class CharacterLiteral {
+ code Code = [{
+addData(S->getValue());
+ }];
+}
+
+//--- Exceptions -//
+class CXXCatchStmt {
+ code Code = [{
+addData(S->getCaughtType());
+ }];
+}
+
+//--- C++ OOP Stmts --//
+class CXXDeleteExpr {
+ code Code = [{
+addData(S->isArrayFormAsWritten()); addData(S->isGlobalDelete());
+ }];
+}
+
+//--- Casts --//
+class ObjCBridgedCastExpr {
+ code Code = [{
+addData(S->getBridgeKind());
+ }];
+}
+
+//--- Miscellaneous Exprs //
+class BinaryOperator {
+ code Code = [{
+addData(S->getOpcode());
+ }];
+}
+class UnaryOperator {
+ code Code = [{
+addData(S->getOpcode());
+ }];
+}
+
+//--- Control flow ---//
+class GotoStmt {
+ code Code = [{
+
[PATCH] D37383: [AST] Add TableGen for StmtDataCollectors
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! https://reviews.llvm.org/D37383 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37383: [AST] Add TableGen for StmtDataCollectors
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 http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37383: [AST] Add TableGen for StmtDataCollectors
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 collectors for AST nodes"), clEnumValN(GenTestPragmaAttributeSupportedAttributes, Looks like an 80 column violation, please reformat this line. https://reviews.llvm.org/D37383 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37383: [AST] Add TableGen for StmtDataCollectors
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 fact that the GSoC is done the current approach is acceptable). I would also like to see this analyzed and tested better in the future to ensure a complete coverage of AST, and I think the use of TableGen can potentially help with that. https://reviews.llvm.org/D37383 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37383: [AST] Add TableGen for StmtDataCollectors
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:
> https://github.com/llvm-mirror/clang/blob/master/CMakeLists.txt#L266 Not sure
> if it actually fires for our case, but it looks like it does.
Yes, that was the issue why it didn't work under include/. I think the sanity
check could be removed, since in-source builds are forbidden anyway.
Or maybe it is still useful by telling us that .inc is probably an generated
file (most, but not all of them are generated, it seems), so another solution
would be to just change the file extension. Actually, `.def` might be the right
extension.
Shall we move to using TableGen only when we need it, and rename it to `.def`
for now?
A preprocessor-only solution for the two modes could look like this:
user:
#define DERIVED MyStmtVisitor
#define SINGLE_TU
#include "clang/AST/StmtDataCollectors.def"
StmtDataCollectors.def:
#define DEF_VISIT(CLASS, CODE)\
template void Visit##CLASS(const CLASS *S) {\
CODE;\
ConstStmtVisitor<##DERIVED>::Visit##CLASS(S)\
}
#ifdef SINGLE_TU
#define DEF_ADD_DATA(CLASS, COMMON, SINGLE, CROSS) DEF_VISIT(CLASS, COMMON;
SINGLE)
#else
#define DEF_ADD_DATA(CLASS, COMMON, SINGLE, CROSS) DEF_VISIT(CLASS, COMMON,
CROSS)
#endif
DEF_ADD_DATA(Stmt, .., .., ..)
...
However, I think TableGen has an advantage when we can extract some structure
instead of just storing the code. Basically, a list of primitive and one of
pointer members for each class could simplify things.
https://reviews.llvm.org/D37383
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37383: [AST] Add TableGen for StmtDataCollectors
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: > https://github.com/llvm-mirror/clang/blob/master/CMakeLists.txt#L266 Not sure > if it actually fires for our case, but it looks like it does. I see, thanks. > Also it would be nicer to maintain once we have a second attribute for an > optional single-TU code as tablegen could just fall back from `SingleTUCode` > to the normal `Code` attribute. And we would have an easy solution for the > CrossTU/SingleTU inc files as tablegen could just create both without us > having to manually copy code around or do preprocessor hacks. If you think there's a need for TableGen in the future then I see no problem with this approach. I was just reluctant to accept it only for code. > I guess this review will make more sense once we have both CrossTU and > SingleTU code ready. https://reviews.llvm.org/D37383 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37383: [AST] Add TableGen for StmtDataCollectors
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 our case, but it looks like it does. Also it would be nicer to maintain once we have a second attribute for an optional single-TU code as tablegen could just fall back from `SingleTUCode` to the normal `Code` attribute. And we would have an easy solution for the CrossTU/SingleTU inc files as tablegen could just create both without us having to manually copy code around or do preprocessor hacks. I guess this review will make more sense once we have both CrossTU and SingleTU code ready. https://reviews.llvm.org/D37383 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37383: [AST] Add TableGen for StmtDataCollectors
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 mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37383: [AST] Add TableGen for StmtDataCollectors
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:
include/clang/AST/CMakeLists.txt
include/clang/AST/StmtDataCollectors.td
lib/AST/StmtDataCollectors.inc
lib/Analysis/CloneDetection.cpp
unittests/AST/DataCollectionTest.cpp
utils/TableGen/CMakeLists.txt
utils/TableGen/ClangDataCollectorsEmitter.cpp
utils/TableGen/TableGen.cpp
utils/TableGen/TableGenBackends.h
Index: utils/TableGen/TableGenBackends.h
===
--- utils/TableGen/TableGenBackends.h
+++ utils/TableGen/TableGenBackends.h
@@ -75,6 +75,8 @@
void EmitClangDiagDocs(RecordKeeper , raw_ostream );
void EmitClangOptDocs(RecordKeeper , raw_ostream );
+void EmitClangDataCollectors(RecordKeeper , raw_ostream );
+
void EmitTestPragmaAttributeSupportedAttributes(RecordKeeper ,
raw_ostream );
Index: utils/TableGen/TableGen.cpp
===
--- utils/TableGen/TableGen.cpp
+++ utils/TableGen/TableGen.cpp
@@ -57,6 +57,7 @@
GenAttrDocs,
GenDiagDocs,
GenOptDocs,
+ GenDataCollectors,
GenTestPragmaAttributeSupportedAttributes
};
@@ -147,6 +148,7 @@
clEnumValN(GenDiagDocs, "gen-diag-docs",
"Generate diagnostic documentation"),
clEnumValN(GenOptDocs, "gen-opt-docs", "Generate option documentation"),
+clEnumValN(GenDataCollectors, "gen-clang-data-collectors", "Generate data collectors for AST nodes"),
clEnumValN(GenTestPragmaAttributeSupportedAttributes,
"gen-clang-test-pragma-attribute-supported-attributes",
"Generate a list of attributes supported by #pragma clang "
@@ -262,6 +264,9 @@
case GenOptDocs:
EmitClangOptDocs(Records, OS);
break;
+ case GenDataCollectors:
+EmitClangDataCollectors(Records, OS);
+break;
case GenTestPragmaAttributeSupportedAttributes:
EmitTestPragmaAttributeSupportedAttributes(Records, OS);
break;
Index: utils/TableGen/ClangDataCollectorsEmitter.cpp
===
--- /dev/null
+++ utils/TableGen/ClangDataCollectorsEmitter.cpp
@@ -0,0 +1,18 @@
+#include "llvm/TableGen/Record.h"
+#include "llvm/TableGen/TableGenBackend.h"
+
+using namespace llvm;
+
+namespace clang {
+void EmitClangDataCollectors(RecordKeeper , raw_ostream ) {
+ const auto = RK.getClasses();
+ for (const auto : Defs) {
+Record = *Entry.second;
+OS << "DEF_ADD_DATA(" << R.getName() << ", {";
+auto Code = R.getValue("Code")->getValue();
+OS << Code->getAsUnquotedString() << "}\n)";
+OS << "\n";
+ }
+ OS << "#undef DEF_ADD_DATA\n";
+}
+} // end namespace clang
Index: utils/TableGen/CMakeLists.txt
===
--- utils/TableGen/CMakeLists.txt
+++ utils/TableGen/CMakeLists.txt
@@ -6,6 +6,7 @@
ClangCommentCommandInfoEmitter.cpp
ClangCommentHTMLNamedCharacterReferenceEmitter.cpp
ClangCommentHTMLTagsEmitter.cpp
+ ClangDataCollectorsEmitter.cpp
ClangDiagnosticsEmitter.cpp
ClangOptionDocEmitter.cpp
ClangSACheckersEmitter.cpp
Index: unittests/AST/DataCollectionTest.cpp
===
--- unittests/AST/DataCollectionTest.cpp
+++ unittests/AST/DataCollectionTest.cpp
@@ -46,7 +46,7 @@
ConstStmtVisitor::Visit##CLASS(S); \
}
-#include "../../lib/AST/StmtDataCollectors.inc"
+#include "clang/AST/StmtDataCollectors.inc"
};
} // end anonymous namespace
Index: lib/Analysis/CloneDetection.cpp
===
--- lib/Analysis/CloneDetection.cpp
+++ lib/Analysis/CloneDetection.cpp
@@ -205,7 +205,7 @@
ConstStmtVisitor::Visit##CLASS(S);\
}
-#include "../AST/StmtDataCollectors.inc"
+#include "clang/AST/StmtDataCollectors.inc"
// Type II clones ignore variable names and literals, so let's skip them.
#define SKIP(CLASS)\
Index: lib/AST/StmtDataCollectors.inc
===
--- lib/AST/StmtDataCollectors.inc
+++ /dev/null
@@ -1,141 +0,0 @@
-// The functions below collect the class specific data of each Stmt subclass.
-
-DEF_ADD_DATA(Stmt, {
- addData(S->getStmtClass());
- // This ensures that non-macro-generated code isn't identical to
- // macro-generated code.
- addData(data_collection::getMacroStack(S->getLocStart(), Context));
- addData(data_collection::getMacroStack(S->getLocEnd(), Context));
-})
-DEF_ADD_DATA(Expr, { addData(S->getType()); })
-
-//--- Builtin functionality
