Porting shouldVisitImplicitCode to DataRecursiveASTVisitor.

[bcraig]

Summary

RecursiveASTVisitor allows visitors to specify whether they wish to visit
implicitly created code or not. I have ported that code over to
DataRecursiveASTVisitor.

DataRecursiveASTVisitor didn't have any tests before this change, so I forked
RecursiveASTVisitorTestExprVisitor. This test exercised some
defaultArgumentWasInherited() cases, so I got those working as well.

I plan on using this new feature in a checker that searches for excess padding.

[zaks.anna]

Removed myself as a reviewer and added Argyrios instead.

[bcraig]

Ping. Note that the test is basically a copy / paste job, and the new code in DataRecursiveASTVisitor.h is a very direct translation from the 'regular' RecursiveASTVisitor.h.

[akyrtzi]

LGTM.

[bcraig]

Largely supplanted by r253958

include/clang/AST/DataRecursiveASTVisitor.h

   /// TypeLocs.
   bool shouldWalkTypesOfTypeLocs() const { return true; }
 
+  /// \brief Return whether this visitor should recurse into implicit
+  /// code, e.g., implicit constructors and destructors.
+  bool shouldVisitImplicitCode() const { return false; }
+
   /// \brief Recursively visit a statement or expression, by
   /// dispatching to Traverse*() based on the argument's dynamic type.
   ///

   if (!D)
     return true;
 
-  // As a syntax visitor, we want to ignore declarations for
-  // implicitly-defined declarations (ones not typed explicitly by the
-  // user).
-  if (D->isImplicit())
+  // As a syntax visitor, by default we want to ignore declarations for
+  // implicit declarations (ones not typed explicitly by the user).
+  if (!getDerived().shouldVisitImplicitCode() && D->isImplicit())
     return true;
 
   switch (D->getKind()) {

   if (TypeSourceInfo *TInfo = Init->getTypeSourceInfo())
     TRY_TO(TraverseTypeLoc(TInfo->getTypeLoc()));
 
-  if (Init->isWritten())
+  if (Init->isWritten() || getDerived().shouldVisitImplicitCode())
     TRY_TO(TraverseStmt(Init->getInit()));
   return true;
 }

   // D is the "T" in something like
   //   template <template <typename> class T> class container { };
   TRY_TO(TraverseDecl(D->getTemplatedDecl()));
-  if (D->hasDefaultArgument()) {
+  if (D->hasDefaultArgument() && !D->defaultArgumentWasInherited()) {
     TRY_TO(TraverseTemplateArgumentLoc(D->getDefaultArgument()));
   }
   TRY_TO(TraverseTemplateParameterListHelper(D->getTemplateParameters()));

   // D is the "T" in something like "template<typename T> class vector;"
   if (D->getTypeForDecl())
     TRY_TO(TraverseType(QualType(D->getTypeForDecl(), 0)));
-  if (D->hasDefaultArgument())
+  if (D->hasDefaultArgument() && !D->defaultArgumentWasInherited())
     TRY_TO(TraverseTypeLoc(D->getDefaultArgumentInfo()->getTypeLoc()));
 })
 

   // FunctionNoProtoType or FunctionProtoType, or a typedef.  This
   // also covers the return type and the function parameters,
   // including exception specifications.
-  TRY_TO(TraverseTypeLoc(D->getTypeSourceInfo()->getTypeLoc()));
+  if (TypeSourceInfo *TSI = D->getTypeSourceInfo()) {
+    TRY_TO(TraverseTypeLoc(TSI->getTypeLoc()));
+  } else if (getDerived().shouldVisitImplicitCode()) {
+    // Visit parameter variable declarations of the implicit function
+    // if the traverser is visiting implicit code. Parameter variable
+    // declarations do not have valid TypeSourceInfo, so to visit them
+    // we need to traverse the declarations explicitly.
+    for (FunctionDecl::param_const_iterator I = D->param_begin(),
+                                            E = D->param_end();
+         I != E; ++I)
+      TRY_TO(TraverseDecl(*I));
+  }
 
   if (CXXConstructorDecl *Ctor = dyn_cast<CXXConstructorDecl>(D)) {
     // Constructor initializers.

 bool RecursiveASTVisitor<Derived>::TraverseVarHelper(VarDecl *D) {
   TRY_TO(TraverseDeclaratorHelper(D));
   // Default params are taken care of when we traverse the ParmVarDecl.
-  if (!isa<ParmVarDecl>(D))
+  if (!isa<ParmVarDecl>(D) &&
+      (!D->isCXXForRangeDecl() || getDerived().shouldVisitImplicitCode()))
     TRY_TO(TraverseStmt(D->getInit()));
   return true;
 }

 DEF_TRAVERSE_DECL(NonTypeTemplateParmDecl, {
   // A non-type template parameter, e.g. "S" in template<int S> class Foo ...
   TRY_TO(TraverseDeclaratorHelper(D));
-  TRY_TO(TraverseStmt(D->getDefaultArgument()));
+  if (D->hasDefaultArgument() && !D->defaultArgumentWasInherited())
+    TRY_TO(TraverseStmt(D->getDefaultArgument()));
 })
 
 DEF_TRAVERSE_DECL(ParmVarDecl, {

 DEF_TRAVERSE_STMT(ObjCAtTryStmt, {})
 DEF_TRAVERSE_STMT(ObjCForCollectionStmt, {})
 DEF_TRAVERSE_STMT(ObjCAutoreleasePoolStmt, {})
-DEF_TRAVERSE_STMT(CXXForRangeStmt, {})
+DEF_TRAVERSE_STMT(CXXForRangeStmt, {
+  if (!getDerived().shouldVisitImplicitCode()) {
+    TRY_TO(TraverseStmt(S->getLoopVarStmt()));
+    TRY_TO(TraverseStmt(S->getRangeInit()));
+    TRY_TO(TraverseStmt(S->getBody()));
+    // Visit everything else only if shouldVisitImplicitCode().
+    return true;
+  }
+})
 DEF_TRAVERSE_STMT(MSDependentExistsStmt, {
   TRY_TO(TraverseNestedNameSpecifierLoc(S->getQualifierLoc()));
   TRY_TO(TraverseDeclarationNameInfo(S->getNameInfo()));

unittests/Tooling/CMakeLists.txt

 add_clang_unittest(ToolingTests
   CommentHandlerTest.cpp
   CompilationDatabaseTest.cpp
+  DataRecursiveASTVisitorTestExprVisitor.cpp
   LookupTest.cpp
   ToolingTest.cpp
   RecursiveASTVisitorTest.cpp

unittests/Tooling/DataRecursiveASTVisitorTestExprVisitor.cpp

+//===- unittest/Tooling/DataRecursiveASTVisitorTestExprVisitor.cpp ------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "TestVisitor.h"
+#include <stack>
+
+using namespace clang;
+
+namespace {
+
+class ParenExprVisitor : public ExpectedLocationDataVisitor<ParenExprVisitor> {
+public:
+  bool VisitParenExpr(ParenExpr *Parens) {
+    Match("", Parens->getExprLoc());
+    return true;
+  }
+};
+
+TEST(DataRecursiveASTVisitor, VisitsParensDuringDataRecursion) {
+  ParenExprVisitor Visitor;
+  Visitor.ExpectMatch("", 1, 9);
+  EXPECT_TRUE(Visitor.runOver("int k = (4) + 9;\n"));
+}
+
+class TemplateArgumentLocTraverser
+  : public ExpectedLocationDataVisitor<TemplateArgumentLocTraverser> {
+public:
+  bool TraverseTemplateArgumentLoc(const TemplateArgumentLoc &ArgLoc) {
+    std::string ArgStr;
+    llvm::raw_string_ostream Stream(ArgStr);
+    const TemplateArgument &Arg = ArgLoc.getArgument();
+
+    Arg.print(Context->getPrintingPolicy(), Stream);
+    Match(Stream.str(), ArgLoc.getLocation());
+    return ExpectedLocationDataVisitor<TemplateArgumentLocTraverser>::
+      TraverseTemplateArgumentLoc(ArgLoc);
+  }
+};
+
+TEST(DataRecursiveASTVisitor, VisitsClassTemplateTemplateParmDefaultArgument) {
+  TemplateArgumentLocTraverser Visitor;
+  Visitor.ExpectMatch("X", 2, 40);
+  EXPECT_TRUE(Visitor.runOver(
+    "template<typename T> class X;\n"
+    "template<template <typename> class T = X> class Y;\n"
+    "template<template <typename> class T> class Y {};\n"));
+}
+
+class CXXBoolLiteralExprVisitor
+  : public ExpectedLocationDataVisitor<CXXBoolLiteralExprVisitor> {
+public:
+  bool VisitCXXBoolLiteralExpr(CXXBoolLiteralExpr *BE) {
+    if (BE->getValue())
+      Match("true", BE->getLocation());
+    else
+      Match("false", BE->getLocation());
+    return true;
+  }
+};
+
+TEST(DataRecursiveASTVisitor, VisitsClassTemplateNonTypeParmDefaultArgument) {
+  CXXBoolLiteralExprVisitor Visitor;
+  Visitor.ExpectMatch("true", 2, 19);
+  EXPECT_TRUE(Visitor.runOver(
+    "template<bool B> class X;\n"
+    "template<bool B = true> class Y;\n"
+    "template<bool B> class Y {};\n"));
+}
+
+// A visitor that visits implicit declarations and matches constructors.
+class ImplicitCtorVisitor
+    : public ExpectedLocationDataVisitor<ImplicitCtorVisitor> {
+public:
+  bool shouldVisitImplicitCode() const { return true; }
+
+  bool VisitCXXConstructorDecl(CXXConstructorDecl* Ctor) {
+    if (Ctor->isImplicit()) {  // Was not written in source code
+      if (const CXXRecordDecl* Class = Ctor->getParent()) {
+        Match(Class->getName(), Ctor->getLocation());
+      }
+    }
+    return true;
+  }
+};
+
+TEST(DataRecursiveASTVisitor, VisitsImplicitCopyConstructors) {
+  ImplicitCtorVisitor Visitor;
+  Visitor.ExpectMatch("Simple", 2, 8);
+  // Note: Clang lazily instantiates implicit declarations, so we need
+  // to use them in order to force them to appear in the AST.
+  EXPECT_TRUE(Visitor.runOver(
+      "struct WithCtor { WithCtor(); }; \n"
+      "struct Simple { Simple(); WithCtor w; }; \n"
+      "int main() { Simple s; Simple t(s); }\n"));
+}
+
+/// \brief A visitor that optionally includes implicit code and matches
+/// CXXConstructExpr.
+///
+/// The name recorded for the match is the name of the class whose constructor
+/// is invoked by the CXXConstructExpr, not the name of the class whose
+/// constructor the CXXConstructExpr is contained in.
+class ConstructExprVisitor
+    : public ExpectedLocationDataVisitor<ConstructExprVisitor> {
+public:
+  ConstructExprVisitor() : ShouldVisitImplicitCode(false) {}
+
+  bool shouldVisitImplicitCode() const { return ShouldVisitImplicitCode; }
+
+  void setShouldVisitImplicitCode(bool NewValue) {
+    ShouldVisitImplicitCode = NewValue;
+  }
+
+  bool VisitCXXConstructExpr(CXXConstructExpr* Expr) {
+    if (const CXXConstructorDecl* Ctor = Expr->getConstructor()) {
+      if (const CXXRecordDecl* Class = Ctor->getParent()) {
+        Match(Class->getName(), Expr->getLocation());
+      }
+    }
+    return true;
+  }
+
+ private:
+  bool ShouldVisitImplicitCode;
+};
+
+TEST(DataRecursiveASTVisitor, CanVisitImplicitMemberInitializations) {
+  ConstructExprVisitor Visitor;
+  Visitor.setShouldVisitImplicitCode(true);
+  Visitor.ExpectMatch("WithCtor", 2, 8);
+  // Simple has a constructor that implicitly initializes 'w'.  Test
+  // that a visitor that visits implicit code visits that initialization.
+  // Note: Clang lazily instantiates implicit declarations, so we need
+  // to use them in order to force them to appear in the AST.
+  EXPECT_TRUE(Visitor.runOver(
+      "struct WithCtor { WithCtor(); }; \n"
+      "struct Simple { WithCtor w; }; \n"
+      "int main() { Simple s; }\n"));
+}
+
+// The same as CanVisitImplicitMemberInitializations, but checking that the
+// visits are omitted when the visitor does not include implicit code.
+TEST(DataRecursiveASTVisitor, CanSkipImplicitMemberInitializations) {
+  ConstructExprVisitor Visitor;
+  Visitor.setShouldVisitImplicitCode(false);
+  Visitor.DisallowMatch("WithCtor", 2, 8);
+  // Simple has a constructor that implicitly initializes 'w'.  Test
+  // that a visitor that skips implicit code skips that initialization.
+  // Note: Clang lazily instantiates implicit declarations, so we need
+  // to use them in order to force them to appear in the AST.
+  EXPECT_TRUE(Visitor.runOver(
+      "struct WithCtor { WithCtor(); }; \n"
+      "struct Simple { WithCtor w; }; \n"
+      "int main() { Simple s; }\n"));
+}
+
+class DeclRefExprVisitor : public ExpectedLocationDataVisitor<DeclRefExprVisitor> {
+public:
+  bool VisitDeclRefExpr(DeclRefExpr *Reference) {
+    Match(Reference->getNameInfo().getAsString(), Reference->getLocation());
+    return true;
+  }
+};
+
+TEST(DataRecursiveASTVisitor, VisitsBaseClassTemplateArguments) {
+  DeclRefExprVisitor Visitor;
+  Visitor.ExpectMatch("x", 2, 3);
+  EXPECT_TRUE(Visitor.runOver(
+    "void x(); template <void (*T)()> class X {};\nX<x> y;"));
+}
+
+TEST(DataRecursiveASTVisitor, VisitsCXXForRangeStmtRange) {
+  DeclRefExprVisitor Visitor;
+  Visitor.ExpectMatch("x", 2, 25);
+  Visitor.ExpectMatch("x", 2, 30);
+  EXPECT_TRUE(Visitor.runOver(
+    "int x[5];\n"
+    "void f() { for (int i : x) { x[0] = 1; } }",
+    DeclRefExprVisitor::Lang_CXX11));
+}
+
+TEST(DataRecursiveASTVisitor, VisitsCallExpr) {
+  DeclRefExprVisitor Visitor;
+  Visitor.ExpectMatch("x", 1, 22);
+  EXPECT_TRUE(Visitor.runOver(
+    "void x(); void y() { x(); }"));
+}
+
+/* FIXME: According to Richard Smith this is a bug in the AST.
+TEST(DataRecursiveASTVisitor, VisitsBaseClassTemplateArgumentsInInstantiation) {
+  DeclRefExprVisitor Visitor;
+  Visitor.ExpectMatch("x", 3, 43);
+  EXPECT_TRUE(Visitor.runOver(
+    "template <typename T> void x();\n"
+    "template <void (*T)()> class X {};\n"
+    "template <typename T> class Y : public X< x<T> > {};\n"
+    "Y<int> y;"));
+}
+*/
+
+TEST(DataRecursiveASTVisitor, VisitsExtension) {
+  DeclRefExprVisitor Visitor;
+  Visitor.ExpectMatch("s", 1, 24);
+  EXPECT_TRUE(Visitor.runOver(
+    "int s = __extension__ (s);\n"));
+}
+
+TEST(DataRecursiveASTVisitor, VisitsCopyExprOfBlockDeclCapture) {
+  DeclRefExprVisitor Visitor;
+  Visitor.ExpectMatch("x", 3, 24);
+  EXPECT_TRUE(Visitor.runOver("void f(int(^)(int)); \n"
+                              "void g() { \n"
+                              "  f([&](int x){ return x; }); \n"
+                              "}",
+                              DeclRefExprVisitor::Lang_OBJCXX11));
+}
+
+} // end anonymous namespace

unittests/Tooling/LookupTest.cpp

 
   bool TraverseDecl(Decl *D) {
     DeclStack.push_back(D);
-    bool Ret = TestVisitor::TraverseDecl(D);
+    bool Ret = TestVisitor<GetDeclsVisitor>::TraverseDecl(D);
     DeclStack.pop_back();
     return Ret;
   }

unittests/Tooling/TestVisitor.h

 
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/DataRecursiveASTVisitor.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"

 ///
 /// Visits template instantiations and implicit code by default.
 template <typename T>
-class TestVisitor : public RecursiveASTVisitor<T> {
+class TestVisitorImpl : public T {
 public:
-  TestVisitor() { }
+  TestVisitorImpl() { }
 
-  virtual ~TestVisitor() { }
+  virtual ~TestVisitorImpl() { }
 
   enum Language {
     Lang_C,

 
   class FindConsumer : public ASTConsumer {
   public:
-    FindConsumer(TestVisitor *Visitor) : Visitor(Visitor) {}
+    FindConsumer(TestVisitorImpl *Visitor) : Visitor(Visitor) {}
 
     void HandleTranslationUnit(clang::ASTContext &Context) override {
       Visitor->Context = &Context;

     }
 
   private:
-    TestVisitor *Visitor;
+    TestVisitorImpl *Visitor;
   };
 
   class TestAction : public ASTFrontendAction {
   public:
-    TestAction(TestVisitor *Visitor) : Visitor(Visitor) {}
+    TestAction(TestVisitorImpl *Visitor) : Visitor(Visitor) {}
 
     std::unique_ptr<clang::ASTConsumer>
     CreateASTConsumer(CompilerInstance &, llvm::StringRef dummy) override {

     }
 
   protected:
-    TestVisitor *Visitor;
+    TestVisitorImpl *Visitor;
   };
 
   ASTContext *Context;
 };
 
+template <typename T>
+using TestVisitor = TestVisitorImpl<RecursiveASTVisitor<T>>;
+template <typename T>
+using TestDataVisitor = TestVisitorImpl<DataRecursiveASTVisitor<T>>;
+
 /// \brief A RecursiveASTVisitor to check that certain matches are (or are
 /// not) observed during visitation.
 ///

   std::vector<MatchCandidate> DisallowedMatches;
   std::vector<ExpectedMatch> ExpectedMatches;
 };
+
+template <typename T>
+using ExpectedLocationDataVisitor = ExpectedLocationVisitor<T, TestDataVisitor>;
+
 }
 
 #endif