[Clang] [Tests] Refactor most unit tests to use DynamicRecursiveASTVisitor (#115132)

This pr refactors most tests that use RAV to use DRAV instead; this also
has the nice effect of testing both the RAV and DRAV implementations at
the same time w/o having to duplicate all of our AST visitor tests.

Some tests rely on features that DRAV doesn’t support (mainly post-order
traversal), so those haven’t been migrated. At the same time,
`TestVisitor` is now a DRAV, so I’ve had to introduce a new
`CTRPTestVisitor` for any tests that need to use RAV directly.

Author: Sirraide

clang/unittests/AST/EvaluateAsRValueTest.cpp

@@ -13,7 +13,7 @@
 
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
-#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/DynamicRecursiveASTVisitor.h"
 #include "clang/Tooling/Tooling.h"
 #include "gtest/gtest.h"
 #include <map>
@@ -28,8 +28,8 @@ typedef std::map<std::string, bool> VarInfoMap;
 
 /// \brief Records information on variable initializers to a map.
 class EvaluateConstantInitializersVisitor
-    : public clang::RecursiveASTVisitor<EvaluateConstantInitializersVisitor> {
- public:
+    : public clang::DynamicRecursiveASTVisitor {
+public:
   explicit EvaluateConstantInitializersVisitor(VarInfoMap &VarInfo)
       : VarInfo(VarInfo) {}
 
@@ -38,7 +38,7 @@ class EvaluateConstantInitializersVisitor
   ///
   /// For each VarDecl with an initializer this also records in VarInfo
   /// whether the initializer could be evaluated as a constant.
-  bool VisitVarDecl(const clang::VarDecl *VD) {
+  bool VisitVarDecl(clang::VarDecl *VD) override {
     if (const clang::Expr *Init = VD->getInit()) {
       clang::Expr::EvalResult Result;
       bool WasEvaluated = Init->EvaluateAsRValue(Result, VD->getASTContext());
@@ -109,9 +109,9 @@ TEST(EvaluateAsRValue, FailsGracefullyForUnknownTypes) {
 }
 
 class CheckLValueToRValueConversionVisitor
-    : public clang::RecursiveASTVisitor<CheckLValueToRValueConversionVisitor> {
+    : public clang::DynamicRecursiveASTVisitor {
 public:
-  bool VisitDeclRefExpr(const clang::DeclRefExpr *E) {
+  bool VisitDeclRefExpr(clang::DeclRefExpr *E) override {
     clang::Expr::EvalResult Result;
     E->EvaluateAsRValue(Result, E->getDecl()->getASTContext(), true);
 

clang/unittests/Analysis/CloneDetectionTest.cpp

@@ -6,24 +6,23 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Analysis/CloneDetection.h"
+#include "clang/AST/DynamicRecursiveASTVisitor.h"
 #include "clang/Tooling/Tooling.h"
 #include "gtest/gtest.h"
 
 namespace clang {
 namespace analysis {
 namespace {
 
-class CloneDetectionVisitor
-    : public RecursiveASTVisitor<CloneDetectionVisitor> {
+class CloneDetectionVisitor : public DynamicRecursiveASTVisitor {
 
   CloneDetector &Detector;
 
 public:
   explicit CloneDetectionVisitor(CloneDetector &D) : Detector(D) {}
 
-  bool VisitFunctionDecl(FunctionDecl *D) {
+  bool VisitFunctionDecl(FunctionDecl *D) override {
     Detector.analyzeCodeBody(D);
     return true;
   }

clang/unittests/Frontend/FrontendActionTest.cpp

@@ -9,7 +9,7 @@
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
-#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/DynamicRecursiveASTVisitor.h"
 #include "clang/Basic/LangStandard.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
@@ -53,7 +53,7 @@ class TestASTFrontendAction : public ASTFrontendAction {
   }
 
 private:
-  class Visitor : public ASTConsumer, public RecursiveASTVisitor<Visitor> {
+  class Visitor : public ASTConsumer, public DynamicRecursiveASTVisitor {
   public:
     Visitor(CompilerInstance &CI, bool ActOnEndOfTranslationUnit,
             std::vector<std::string> &decl_names) :
@@ -67,7 +67,7 @@ class TestASTFrontendAction : public ASTFrontendAction {
       TraverseDecl(context.getTranslationUnitDecl());
     }
 
-    virtual bool VisitNamedDecl(NamedDecl *Decl) {
+    bool VisitNamedDecl(NamedDecl *Decl) override {
       decl_names_.push_back(Decl->getQualifiedNameAsString());
       return true;
     }

clang/unittests/Tooling/ASTSelectionTest.cpp

@@ -26,7 +26,7 @@ struct FileLocation {
 
 using FileRange = std::pair<FileLocation, FileLocation>;
 
-class SelectionFinderVisitor : public TestVisitor<SelectionFinderVisitor> {
+class SelectionFinderVisitor : public TestVisitor {
   FileLocation Location;
   std::optional<FileRange> SelectionRange;
   llvm::function_ref<void(SourceRange SelectionRange,
@@ -42,7 +42,7 @@ class SelectionFinderVisitor : public TestVisitor<SelectionFinderVisitor> {
       : Location(Location), SelectionRange(SelectionRange), Consumer(Consumer) {
   }
 
-  bool VisitTranslationUnitDecl(const TranslationUnitDecl *TU) {
+  bool VisitTranslationUnitDecl(TranslationUnitDecl *TU) override {
     const ASTContext &Context = TU->getASTContext();
     const SourceManager &SM = Context.getSourceManager();
 

clang/unittests/Tooling/CRTPTestVisitor.h

@@ -0,0 +1,42 @@
+//===--- TestVisitor.h ------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// \brief Defines a CRTP-based RecursiveASTVisitor helper for tests.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_UNITTESTS_TOOLING_CRTPTESTVISITOR_H
+#define LLVM_CLANG_UNITTESTS_TOOLING_CRTPTESTVISITOR_H
+
+#include "TestVisitor.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+
+// CRTP versions of the visitors in TestVisitor.h.
+namespace clang {
+template <typename T>
+class CRTPTestVisitor : public RecursiveASTVisitor<T>,
+                        public detail::TestVisitorHelper {
+public:
+  bool shouldVisitTemplateInstantiations() const { return true; }
+  bool shouldVisitImplicitCode() const { return true; }
+
+  void InvokeTraverseDecl(TranslationUnitDecl *D) override {
+    RecursiveASTVisitor<T>::TraverseDecl(D);
+  }
+};
+
+template <typename T>
+class CRTPExpectedLocationVisitor
+    : public CRTPTestVisitor<T>,
+      public detail::ExpectedLocationVisitorHelper {
+  ASTContext *getASTContext() override { return this->Context; }
+};
+} // namespace clang
+
+#endif // LLVM_CLANG_UNITTESTS_TOOLING_CRTPTESTVISITOR_H

clang/unittests/Tooling/CastExprTest.cpp

@@ -12,17 +12,17 @@ using namespace clang;
 
 namespace {
 
-struct CastExprVisitor : TestVisitor<CastExprVisitor> {
+struct CastExprVisitor : TestVisitor {
   std::function<void(ExplicitCastExpr *)> OnExplicitCast;
   std::function<void(CastExpr *)> OnCast;
 
-  bool VisitExplicitCastExpr(ExplicitCastExpr *Expr) {
+  bool VisitExplicitCastExpr(ExplicitCastExpr *Expr) override {
     if (OnExplicitCast)
       OnExplicitCast(Expr);
     return true;
   }
 
-  bool VisitCastExpr(CastExpr *Expr) {
+  bool VisitCastExpr(CastExpr *Expr) override {
     if (OnCast)
       OnCast(Expr);
     return true;

clang/unittests/Tooling/CommentHandlerTest.cpp

@@ -22,12 +22,9 @@ struct Comment {
 class CommentVerifier;
 typedef std::vector<Comment> CommentList;
 
-class CommentHandlerVisitor : public TestVisitor<CommentHandlerVisitor>,
-                              public CommentHandler {
-  typedef TestVisitor<CommentHandlerVisitor> base;
-
+class CommentHandlerVisitor : public TestVisitor, public CommentHandler {
 public:
-  CommentHandlerVisitor() : base(), PP(nullptr), Verified(false) {}
+  CommentHandlerVisitor() : PP(nullptr), Verified(false) {}
 
   ~CommentHandlerVisitor() override {
     EXPECT_TRUE(Verified) << "CommentVerifier not accessed";
@@ -64,7 +61,7 @@ class CommentHandlerVisitor : public TestVisitor<CommentHandlerVisitor>,
   CommentList Comments;
   bool Verified;
 
-  class CommentHandlerAction : public base::TestAction {
+  class CommentHandlerAction : public TestAction {
   public:
     CommentHandlerAction(CommentHandlerVisitor *Visitor)
         : TestAction(Visitor) { }

clang/unittests/Tooling/ExecutionTest.cpp

@@ -9,7 +9,7 @@
 #include "clang/Tooling/Execution.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/DeclCXX.h"
-#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/DynamicRecursiveASTVisitor.h"
 #include "clang/Frontend/ASTUnit.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Frontend/FrontendActions.h"
@@ -30,12 +30,9 @@ namespace {
 
 // This traverses the AST and outputs function name as key and "1" as value for
 // each function declaration.
-class ASTConsumerWithResult
-    : public ASTConsumer,
-      public RecursiveASTVisitor<ASTConsumerWithResult> {
+class ASTConsumerWithResult : public ASTConsumer,
+                              public DynamicRecursiveASTVisitor {
 public:
-  using ASTVisitor = RecursiveASTVisitor<ASTConsumerWithResult>;
-
   explicit ASTConsumerWithResult(ExecutionContext *Context) : Context(Context) {
     assert(Context != nullptr);
   }
@@ -44,12 +41,12 @@ class ASTConsumerWithResult
     TraverseDecl(Context.getTranslationUnitDecl());
   }
 
-  bool TraverseFunctionDecl(clang::FunctionDecl *Decl) {
+  bool TraverseFunctionDecl(clang::FunctionDecl *Decl) override {
     Context->reportResult(Decl->getNameAsString(),
                           Context->getRevision() + ":" + Context->getCorpus() +
                               ":" + Context->getCurrentCompilationUnit() +
                               "/1");
-    return ASTVisitor::TraverseFunctionDecl(Decl);
+    return DynamicRecursiveASTVisitor::TraverseFunctionDecl(Decl);
   }
 
 private:

clang/unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp

@@ -44,13 +44,14 @@ class LexicallyOrderedDeclVisitor
   llvm::SmallVector<Decl *, 8> TraversalStack;
 };
 
-class DummyMatchVisitor : public ExpectedLocationVisitor<DummyMatchVisitor> {
+class DummyMatchVisitor : public ExpectedLocationVisitor {
   bool EmitDeclIndices, EmitStmtIndices;
 
 public:
   DummyMatchVisitor(bool EmitDeclIndices = false, bool EmitStmtIndices = false)
       : EmitDeclIndices(EmitDeclIndices), EmitStmtIndices(EmitStmtIndices) {}
-  bool VisitTranslationUnitDecl(TranslationUnitDecl *TU) {
+
+  bool VisitTranslationUnitDecl(TranslationUnitDecl *TU) override {
     const ASTContext &Context = TU->getASTContext();
     const SourceManager &SM = Context.getSourceManager();
     LexicallyOrderedDeclVisitor SubVisitor(*this, SM, EmitDeclIndices,

clang/unittests/Tooling/LookupTest.cpp

@@ -13,31 +13,31 @@
 using namespace clang;
 
 namespace {
-struct GetDeclsVisitor : TestVisitor<GetDeclsVisitor> {
+struct GetDeclsVisitor : TestVisitor {
   std::function<void(CallExpr *)> OnCall;
   std::function<void(RecordTypeLoc)> OnRecordTypeLoc;
   std::function<void(UsingTypeLoc)> OnUsingTypeLoc;
   SmallVector<Decl *, 4> DeclStack;
 
-  bool VisitCallExpr(CallExpr *Expr) {
+  bool VisitCallExpr(CallExpr *Expr) override {
     if (OnCall)
       OnCall(Expr);
     return true;
   }
 
-  bool VisitRecordTypeLoc(RecordTypeLoc Loc) {
+  bool VisitRecordTypeLoc(RecordTypeLoc Loc) override {
     if (OnRecordTypeLoc)
       OnRecordTypeLoc(Loc);
     return true;
   }
 
-  bool VisitUsingTypeLoc(UsingTypeLoc Loc) {
+  bool VisitUsingTypeLoc(UsingTypeLoc Loc) override {
     if (OnUsingTypeLoc)
       OnUsingTypeLoc(Loc);
     return true;
   }
 
-  bool TraverseDecl(Decl *D) {
+  bool TraverseDecl(Decl *D) override {
     DeclStack.push_back(D);
     bool Ret = TestVisitor::TraverseDecl(D);
     DeclStack.pop_back();

clang/unittests/Tooling/QualTypeNamesTest.cpp

@@ -11,12 +11,12 @@
 using namespace clang;
 
 namespace {
-struct TypeNameVisitor : TestVisitor<TypeNameVisitor> {
+struct TypeNameVisitor : TestVisitor {
   llvm::StringMap<std::string> ExpectedQualTypeNames;
   bool WithGlobalNsPrefix = false;
 
   // ValueDecls are the least-derived decl with both a qualtype and a name.
-  bool VisitValueDecl(const ValueDecl *VD) {
+  bool VisitValueDecl(ValueDecl *VD) override {
     std::string ExpectedName =
         ExpectedQualTypeNames.lookup(VD->getNameAsString());
     if (ExpectedName != "") {

clang/unittests/Tooling/RecursiveASTVisitorTestDeclVisitor.cpp

@@ -12,12 +12,12 @@ using namespace clang;
 
 namespace {
 
-class VarDeclVisitor : public ExpectedLocationVisitor<VarDeclVisitor> {
+class VarDeclVisitor : public ExpectedLocationVisitor {
 public:
- bool VisitVarDecl(VarDecl *Variable) {
-   Match(Variable->getNameAsString(), Variable->getBeginLoc());
-   return true;
- }
+  bool VisitVarDecl(VarDecl *Variable) override {
+    Match(Variable->getNameAsString(), Variable->getBeginLoc());
+    return true;
+  }
 };
 
 TEST(RecursiveASTVisitor, VisitsCXXForRangeStmtLoopVariable) {
@@ -29,12 +29,11 @@ TEST(RecursiveASTVisitor, VisitsCXXForRangeStmtLoopVariable) {
     VarDeclVisitor::Lang_CXX11));
 }
 
-class ParmVarDeclVisitorForImplicitCode :
-  public ExpectedLocationVisitor<ParmVarDeclVisitorForImplicitCode> {
+class ParmVarDeclVisitorForImplicitCode : public ExpectedLocationVisitor {
 public:
-  bool shouldVisitImplicitCode() const { return true; }
+  ParmVarDeclVisitorForImplicitCode() { ShouldVisitImplicitCode = true; }
 
-  bool VisitParmVarDecl(ParmVarDecl *ParamVar) {
+  bool VisitParmVarDecl(ParmVarDecl *ParamVar) override {
     Match(ParamVar->getNameAsString(), ParamVar->getBeginLoc());
     return true;
   }
@@ -58,10 +57,9 @@ TEST(RecursiveASTVisitor, VisitsParmVarDeclForImplicitCode) {
     "void bar(Y a) {Y b = a;}"));
 }
 
-class NamedDeclVisitor
-  : public ExpectedLocationVisitor<NamedDeclVisitor> {
+class NamedDeclVisitor : public ExpectedLocationVisitor {
 public:
-  bool VisitNamedDecl(NamedDecl *Decl) {
+  bool VisitNamedDecl(NamedDecl *Decl) override {
     std::string NameWithTemplateArgs;
     llvm::raw_string_ostream OS(NameWithTemplateArgs);
     Decl->getNameForDiagnostic(OS,

clang/unittests/Tooling/RecursiveASTVisitorTestPostOrderVisitor.cpp

@@ -11,14 +11,12 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "TestVisitor.h"
+#include "CRTPTestVisitor.h"
 
 using namespace clang;
 
 namespace {
-
-class RecordingVisitor : public TestVisitor<RecordingVisitor> {
-
+class RecordingVisitor : public CRTPTestVisitor<RecordingVisitor> {
   bool VisitPostOrder;
 
 public:

clang/unittests/Tooling/RecursiveASTVisitorTestTypeLocVisitor.cpp

@@ -12,9 +12,9 @@ using namespace clang;
 
 namespace {
 
-class TypeLocVisitor : public ExpectedLocationVisitor<TypeLocVisitor> {
+class TypeLocVisitor : public ExpectedLocationVisitor {
 public:
-  bool VisitTypeLoc(TypeLoc TypeLocation) {
+  bool VisitTypeLoc(TypeLoc TypeLocation) override {
     Match(TypeLocation.getType().getAsString(), TypeLocation.getBeginLoc());
     return true;
   }