[AST] Add TableGen for StmtDataCollectors

[johannes]

Summary

This adds an option "-gen-clang-data-collectors" to the Clang TableGen
that is used to generate StmtDataCollectors.inc.

[arphaman]

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?

[teemperor]

@arphaman I suggested tablegen in 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.

[arphaman]

In D37383#858905, @teemperor wrote:

@arphaman I suggested tablegen in 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.

[johannes]

In D37383#858905, @teemperor wrote:

@arphaman I suggested tablegen in 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 <class = void> 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.

[arphaman]

defs 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.

[arphaman]

LGTM with one inline comment below:

utils/TableGen/TableGen.cpp

151 ↗

(On Diff #113547)

Looks like an 80 column violation, please reformat this line.

[johannes]

@teemperor ok for you? did phabricator make you a blocking reviewer because of the affected code, or did I do that somehow?

[teemperor]

@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!

cfe/trunk/include/clang/AST/CMakeLists.txt

 clang_tablegen(CommentCommandInfo.inc -gen-clang-comment-command-info
   SOURCE CommentCommands.td
   TARGET ClangCommentCommandInfo)
 
 clang_tablegen(CommentCommandList.inc -gen-clang-comment-command-list
   SOURCE CommentCommands.td
   TARGET ClangCommentCommandList)
 
+clang_tablegen(StmtDataCollectors.inc -gen-clang-data-collectors
+  SOURCE StmtDataCollectors.td
+  TARGET StmtDataCollectors)

cfe/trunk/include/clang/AST/StmtDataCollectors.td

+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<X, XX>()' != 'foo<XX, X>()'.
+          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 = [{
+    addData(S->getLabel()->getName());
+  }];
+}
+class IndirectGotoStmt {
+  code Code = [{
+    if (S->getConstantTarget())
+      addData(S->getConstantTarget()->getName());
+  }];
+}
+class LabelStmt {
+  code Code = [{
+    addData(S->getDecl()->getName());
+  }];
+}
+class MSDependentExistsStmt {
+  code Code = [{
+    addData(S->isIfExists());
+  }];
+}
+class AddrLabelExpr {
+  code Code = [{
+    addData(S->getLabel()->getName());
+  }];
+}
+
+//--- Objective-C --------------------------------------------------------//
+class ObjCIndirectCopyRestoreExpr {
+  code Code = [{
+    addData(S->shouldCopy());
+  }];
+}
+class ObjCPropertyRefExpr {
+  code Code = [{
+    addData(S->isSuperReceiver()); addData(S->isImplicitProperty());
+  }];
+}
+class ObjCAtCatchStmt {
+  code Code = [{
+    addData(S->hasEllipsis());
+  }];
+}
+
+//--- Miscellaneous Stmts ------------------------------------------------//
+class CXXFoldExpr {
+  code Code = [{
+    addData(S->isRightFold()); addData(S->getOperator());
+  }];
+}
+class GenericSelectionExpr {
+  code Code = [{
+    for (unsigned i = 0; i < S->getNumAssocs(); ++i) {
+      addData(S->getAssocType(i));
+    }
+  }];
+}
+class LambdaExpr {
+  code Code = [{
+    for (const LambdaCapture &C : S->captures()) {
+      addData(C.isPackExpansion());
+      addData(C.getCaptureKind());
+      if (C.capturesVariable())
+        addData(C.getCapturedVar()->getType());
+    }
+    addData(S->isGenericLambda());
+    addData(S->isMutable());
+  }];
+}
+class DeclStmt {
+  code Code = [{
+    auto numDecls = std::distance(S->decl_begin(), S->decl_end());
+    addData(static_cast<unsigned>(numDecls));
+    for (const Decl *D : S->decls()) {
+      if (const VarDecl *VD = dyn_cast<VarDecl>(D)) {
+        addData(VD->getType());
+      }
+    }
+  }];
+}
+class AsmStmt {
+  code Code = [{
+    addData(S->isSimple());
+    addData(S->isVolatile());
+    addData(S->generateAsmString(Context));
+    for (unsigned i = 0; i < S->getNumInputs(); ++i) {
+      addData(S->getInputConstraint(i));
+    }
+    for (unsigned i = 0; i < S->getNumOutputs(); ++i) {
+      addData(S->getOutputConstraint(i));
+    }
+    for (unsigned i = 0; i < S->getNumClobbers(); ++i) {
+      addData(S->getClobber(i));
+    }
+  }];
+}
+class AttributedStmt {
+  code Code = [{
+    for (const Attr *A : S->getAttrs()) {
+      addData(std::string(A->getSpelling()));
+    }
+  }];
+}

cfe/trunk/lib/AST/StmtDataCollectors.inc

-// 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 ----------------------------------------------//
-DEF_ADD_DATA(ArrayTypeTraitExpr, { addData(S->getTrait()); })
-DEF_ADD_DATA(ExpressionTraitExpr, { addData(S->getTrait()); })
-DEF_ADD_DATA(PredefinedExpr, { addData(S->getIdentType()); })
-DEF_ADD_DATA(TypeTraitExpr, {
-  addData(S->getTrait());
-  for (unsigned i = 0; i < S->getNumArgs(); ++i)
-    addData(S->getArg(i)->getType());
-})
-
-//--- Calls --------------------------------------------------------------//
-DEF_ADD_DATA(CallExpr, {
-  // 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<X, XX>()' != 'foo<XX, X>()'.
-        OS << '\n';
-      }
-      OS.flush();
-
-      addData(ArgString);
-    }
-    addData(D->getQualifiedNameAsString());
-  }
-})
-
-//--- Value references ---------------------------------------------------//
-DEF_ADD_DATA(DeclRefExpr,
-             { addData(S->getDecl()->getQualifiedNameAsString()); })
-DEF_ADD_DATA(MemberExpr,
-             { addData(S->getMemberDecl()->getName()); })
-
-//--- Literals -----------------------------------------------------------//
-DEF_ADD_DATA(IntegerLiteral, { addData(llvm::hash_value(S->getValue())); })
-DEF_ADD_DATA(FloatingLiteral, { addData(llvm::hash_value(S->getValue())); })
-DEF_ADD_DATA(StringLiteral, { addData(S->getString()); })
-DEF_ADD_DATA(CXXBoolLiteralExpr, { addData(S->getValue()); })
-DEF_ADD_DATA(CharacterLiteral, { addData(S->getValue()); })
-
-//--- Exceptions ---------------------------------------------------------//
-DEF_ADD_DATA(CXXCatchStmt, { addData(S->getCaughtType()); })
-
-//--- C++ OOP Stmts ------------------------------------------------------//
-DEF_ADD_DATA(CXXDeleteExpr, {
-  addData(S->isArrayFormAsWritten());
-  addData(S->isGlobalDelete());
-})
-
-//--- Casts --------------------------------------------------------------//
-DEF_ADD_DATA(ObjCBridgedCastExpr, { addData(S->getBridgeKind()); })
-
-//--- Miscellaneous Exprs ------------------------------------------------//
-DEF_ADD_DATA(BinaryOperator, { addData(S->getOpcode()); })
-DEF_ADD_DATA(UnaryOperator, { addData(S->getOpcode()); })
-
-//--- Control flow -------------------------------------------------------//
-DEF_ADD_DATA(GotoStmt, { addData(S->getLabel()->getName()); })
-DEF_ADD_DATA(IndirectGotoStmt, {
-  if (S->getConstantTarget())
-    addData(S->getConstantTarget()->getName());
-})
-DEF_ADD_DATA(LabelStmt, { addData(S->getDecl()->getName()); })
-DEF_ADD_DATA(MSDependentExistsStmt, { addData(S->isIfExists()); })
-DEF_ADD_DATA(AddrLabelExpr, { addData(S->getLabel()->getName()); })
-
-//--- Objective-C --------------------------------------------------------//
-DEF_ADD_DATA(ObjCIndirectCopyRestoreExpr, { addData(S->shouldCopy()); })
-DEF_ADD_DATA(ObjCPropertyRefExpr, {
-  addData(S->isSuperReceiver());
-  addData(S->isImplicitProperty());
-})
-DEF_ADD_DATA(ObjCAtCatchStmt, { addData(S->hasEllipsis()); })
-
-//--- Miscellaneous Stmts ------------------------------------------------//
-DEF_ADD_DATA(CXXFoldExpr, {
-  addData(S->isRightFold());
-  addData(S->getOperator());
-})
-DEF_ADD_DATA(GenericSelectionExpr, {
-  for (unsigned i = 0; i < S->getNumAssocs(); ++i) {
-    addData(S->getAssocType(i));
-  }
-})
-DEF_ADD_DATA(LambdaExpr, {
-  for (const LambdaCapture &C : S->captures()) {
-    addData(C.isPackExpansion());
-    addData(C.getCaptureKind());
-    if (C.capturesVariable())
-      addData(C.getCapturedVar()->getType());
-  }
-  addData(S->isGenericLambda());
-  addData(S->isMutable());
-})
-DEF_ADD_DATA(DeclStmt, {
-  auto numDecls = std::distance(S->decl_begin(), S->decl_end());
-  addData(static_cast<unsigned>(numDecls));
-  for (const Decl *D : S->decls()) {
-    if (const VarDecl *VD = dyn_cast<VarDecl>(D)) {
-      addData(VD->getType());
-    }
-  }
-})
-DEF_ADD_DATA(AsmStmt, {
-  addData(S->isSimple());
-  addData(S->isVolatile());
-  addData(S->generateAsmString(Context));
-  for (unsigned i = 0; i < S->getNumInputs(); ++i) {
-    addData(S->getInputConstraint(i));
-  }
-  for (unsigned i = 0; i < S->getNumOutputs(); ++i) {
-    addData(S->getOutputConstraint(i));
-  }
-  for (unsigned i = 0; i < S->getNumClobbers(); ++i) {
-    addData(S->getClobber(i));
-  }
-})
-DEF_ADD_DATA(AttributedStmt, {
-  for (const Attr *A : S->getAttrs()) {
-    addData(std::string(A->getSpelling()));
-  }
-})
-#undef DEF_ADD_DATA

cfe/trunk/lib/Analysis/CloneDetection.cpp

 // all parent classes. This uses a template so that custom visit methods by us
 // take precedence.
 #define DEF_ADD_DATA(CLASS, CODE)                                              \
   template <class = void> void Visit##CLASS(const CLASS *S) {                  \
     CODE;                                                                      \
     ConstStmtVisitor<CloneTypeIIStmtDataCollector<T>>::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)                                                            \
   void Visit##CLASS(const CLASS *S) {                                          \
     ConstStmtVisitor<CloneTypeIIStmtDataCollector<T>>::Visit##CLASS(S);        \
   }
   SKIP(DeclRefExpr)
   SKIP(MemberExpr)

cfe/trunk/unittests/AST/DataCollectionTest.cpp

public:
   }
 
 #define DEF_ADD_DATA(CLASS, CODE)                                              \
   template <class Dummy = void> Dummy Visit##CLASS(const CLASS *S) {           \
     CODE;                                                                      \
     ConstStmtVisitor<StmtDataCollector>::Visit##CLASS(S);                      \
   }
 
-#include "../../lib/AST/StmtDataCollectors.inc"
+#include "clang/AST/StmtDataCollectors.inc"
 };
 } // end anonymous namespace
 
 namespace {
 struct StmtHashMatch : public MatchFinder::MatchCallback {
   unsigned NumFound;
   llvm::MD5::MD5Result &Hash;
   StmtHashMatch(llvm::MD5::MD5Result &Hash) : NumFound(0), Hash(Hash) {}

cfe/trunk/utils/TableGen/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS Support)
 
 add_tablegen(clang-tblgen CLANG
   ClangASTNodesEmitter.cpp
   ClangAttrEmitter.cpp
   ClangCommentCommandInfoEmitter.cpp
   ClangCommentHTMLNamedCharacterReferenceEmitter.cpp
   ClangCommentHTMLTagsEmitter.cpp
+  ClangDataCollectorsEmitter.cpp
   ClangDiagnosticsEmitter.cpp
   ClangOptionDocEmitter.cpp
   ClangSACheckersEmitter.cpp
   NeonEmitter.cpp
   TableGen.cpp
   )

cfe/trunk/utils/TableGen/ClangDataCollectorsEmitter.cpp

+#include "llvm/TableGen/Record.h"
+#include "llvm/TableGen/TableGenBackend.h"
+
+using namespace llvm;
+
+namespace clang {
+void EmitClangDataCollectors(RecordKeeper &RK, raw_ostream &OS) {
+  const auto &Defs = RK.getClasses();
+  for (const auto &Entry : Defs) {
+    Record &R = *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

cfe/trunk/utils/TableGen/TableGen.cpp

enum ActionType {
   GenClangCommentCommandInfo,
   GenClangCommentCommandList,
   GenArmNeon,
   GenArmNeonSema,
   GenArmNeonTest,
   GenAttrDocs,
   GenDiagDocs,
   GenOptDocs,
+  GenDataCollectors,
   GenTestPragmaAttributeSupportedAttributes
 };
 
 namespace {
 cl::opt<ActionType> Action(
     cl::desc("Action to perform:"),
     cl::values(
         clEnumValN(GenClangAttrClasses, "gen-clang-attr-classes",
cl::values(
                    "Generate ARM NEON sema support for clang"),
         clEnumValN(GenArmNeonTest, "gen-arm-neon-test",
                    "Generate ARM NEON tests for clang"),
         clEnumValN(GenAttrDocs, "gen-attr-docs",
                    "Generate attribute documentation"),
         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 "
                    "attribute for testing purposes")));
 
 cl::opt<std::string>
 ClangComponent("clang-component",
                cl::desc("Only use warnings from specified component"),
case GenAttrDocs:
     EmitClangAttrDocs(Records, OS);
     break;
   case GenDiagDocs:
     EmitClangDiagDocs(Records, OS);
     break;
   case GenOptDocs:
     EmitClangOptDocs(Records, OS);
     break;
+  case GenDataCollectors:
+    EmitClangDataCollectors(Records, OS);
+    break;
   case GenTestPragmaAttributeSupportedAttributes:
     EmitTestPragmaAttributeSupportedAttributes(Records, OS);
     break;
   }
 
   return false;
 }
 }

cfe/trunk/utils/TableGen/TableGenBackends.h

 void EmitNeon2(RecordKeeper &Records, raw_ostream &OS);
 void EmitNeonSema2(RecordKeeper &Records, raw_ostream &OS);
 void EmitNeonTest2(RecordKeeper &Records, raw_ostream &OS);
 
 void EmitClangAttrDocs(RecordKeeper &Records, raw_ostream &OS);
 void EmitClangDiagDocs(RecordKeeper &Records, raw_ostream &OS);
 void EmitClangOptDocs(RecordKeeper &Records, raw_ostream &OS);
 
+void EmitClangDataCollectors(RecordKeeper &Records, raw_ostream &OS);
+
 void EmitTestPragmaAttributeSupportedAttributes(RecordKeeper &Records,
                                                 raw_ostream &OS);
 
 } // end namespace clang
 
 #endif