[clang] [Clang] [Tests] Refactor most unit tests to use DRAV instead (PR #115132)

via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 6 00:13:26 PST 2024


https://github.com/Sirraide created https://github.com/llvm/llvm-project/pull/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.

>From 0dd745286a124f7142d827810be5185abdc196cf Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Mon, 19 Aug 2024 17:46:53 +0200
Subject: [PATCH] [Tests] Refactor most tests to use the dynamic visitor

---
 clang/unittests/AST/EvaluateAsRValueTest.cpp  |  12 +-
 .../unittests/Analysis/CloneDetectionTest.cpp |   7 +-
 .../unittests/Frontend/FrontendActionTest.cpp |   6 +-
 clang/unittests/Tooling/ASTSelectionTest.cpp  |   4 +-
 clang/unittests/Tooling/CRTPTestVisitor.h     |  42 +++++
 clang/unittests/Tooling/CastExprTest.cpp      |   6 +-
 .../unittests/Tooling/CommentHandlerTest.cpp  |   9 +-
 clang/unittests/Tooling/ExecutionTest.cpp     |  13 +-
 ...exicallyOrderedRecursiveASTVisitorTest.cpp |   5 +-
 clang/unittests/Tooling/LookupTest.cpp        |  10 +-
 clang/unittests/Tooling/QualTypeNamesTest.cpp |   4 +-
 .../RecursiveASTVisitorTestDeclVisitor.cpp    |  22 ++-
 ...ecursiveASTVisitorTestPostOrderVisitor.cpp |   6 +-
 .../RecursiveASTVisitorTestTypeLocVisitor.cpp |   4 +-
 .../Tooling/RecursiveASTVisitorTests/Attr.cpp |   5 +-
 .../BitfieldInitializer.cpp                   |   5 +-
 .../CXXBoolLiteralExpr.cpp                    |   5 +-
 .../CXXMemberCall.cpp                         |   5 +-
 .../CXXMethodDecl.cpp                         |  18 +-
 .../CXXOperatorCallExprTraverser.cpp          |   8 +-
 .../CallbacksCommon.h                         |   4 +-
 .../RecursiveASTVisitorTests/Class.cpp        |   5 +-
 .../RecursiveASTVisitorTests/Concept.cpp      | 115 +++++++------
 .../ConstructExpr.cpp                         |  20 +--
 .../RecursiveASTVisitorTests/DeclRefExpr.cpp  |  17 +-
 .../DeductionGuide.cpp                        |  17 +-
 .../RecursiveASTVisitorTests/ImplicitCtor.cpp |   7 +-
 .../ImplicitCtorInitializer.cpp               |  18 +-
 .../InitListExprPostOrder.cpp                 |   4 +-
 .../InitListExprPostOrderNoQueue.cpp          |   6 +-
 .../InitListExprPreOrder.cpp                  |  15 +-
 .../InitListExprPreOrderNoQueue.cpp           |   7 +-
 .../IntegerLiteral.cpp                        |   5 +-
 .../LambdaDefaultCapture.cpp                  |   5 +-
 .../RecursiveASTVisitorTests/LambdaExpr.cpp   |  17 +-
 .../LambdaTemplateParams.cpp                  |  11 +-
 .../MemberPointerTypeLoc.cpp                  |   7 +-
 .../NestedNameSpecifiers.cpp                  |   7 +-
 .../RecursiveASTVisitorTests/ParenExpr.cpp    |   4 +-
 .../TemplateArgumentLocTraverser.cpp          |   8 +-
 .../TraversalScope.cpp                        |   6 +-
 clang/unittests/Tooling/RefactoringTest.cpp   |  21 +--
 clang/unittests/Tooling/SourceCodeTest.cpp    |  55 +++---
 clang/unittests/Tooling/TestVisitor.h         | 160 +++++++++++-------
 44 files changed, 378 insertions(+), 359 deletions(-)
 create mode 100644 clang/unittests/Tooling/CRTPTestVisitor.h

diff --git a/clang/unittests/AST/EvaluateAsRValueTest.cpp b/clang/unittests/AST/EvaluateAsRValueTest.cpp
index f6261b827671bc..1e17330863f264 100644
--- a/clang/unittests/AST/EvaluateAsRValueTest.cpp
+++ b/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);
 
diff --git a/clang/unittests/Analysis/CloneDetectionTest.cpp b/clang/unittests/Analysis/CloneDetectionTest.cpp
index 738f6efd2018d7..d0148a8c28c54e 100644
--- a/clang/unittests/Analysis/CloneDetectionTest.cpp
+++ b/clang/unittests/Analysis/CloneDetectionTest.cpp
@@ -6,8 +6,8 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Analysis/CloneDetection.h"
+#include "clang/AST/DynamicRecursiveASTVisitor.h"
 #include "clang/Tooling/Tooling.h"
 #include "gtest/gtest.h"
 
@@ -15,15 +15,14 @@ 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;
   }
diff --git a/clang/unittests/Frontend/FrontendActionTest.cpp b/clang/unittests/Frontend/FrontendActionTest.cpp
index 818e8cef27e51b..6ce9ba6f6a0888 100644
--- a/clang/unittests/Frontend/FrontendActionTest.cpp
+++ b/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;
     }
diff --git a/clang/unittests/Tooling/ASTSelectionTest.cpp b/clang/unittests/Tooling/ASTSelectionTest.cpp
index 113165f68449ca..1897bc15196ec2 100644
--- a/clang/unittests/Tooling/ASTSelectionTest.cpp
+++ b/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();
 
diff --git a/clang/unittests/Tooling/CRTPTestVisitor.h b/clang/unittests/Tooling/CRTPTestVisitor.h
new file mode 100644
index 00000000000000..67ae36b2e3ddd9
--- /dev/null
+++ b/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
diff --git a/clang/unittests/Tooling/CastExprTest.cpp b/clang/unittests/Tooling/CastExprTest.cpp
index eab23a5a98e5d5..e5a8d994bf011b 100644
--- a/clang/unittests/Tooling/CastExprTest.cpp
+++ b/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;
diff --git a/clang/unittests/Tooling/CommentHandlerTest.cpp b/clang/unittests/Tooling/CommentHandlerTest.cpp
index 7eb11ccd6ee2d1..edfb72e2ec599b 100644
--- a/clang/unittests/Tooling/CommentHandlerTest.cpp
+++ b/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) { }
diff --git a/clang/unittests/Tooling/ExecutionTest.cpp b/clang/unittests/Tooling/ExecutionTest.cpp
index 91ab8594f6823d..b0fd7ccb950ff4 100644
--- a/clang/unittests/Tooling/ExecutionTest.cpp
+++ b/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:
diff --git a/clang/unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp b/clang/unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp
index 5d16595aec8014..b167eb4b811755 100644
--- a/clang/unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp
+++ b/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,
diff --git a/clang/unittests/Tooling/LookupTest.cpp b/clang/unittests/Tooling/LookupTest.cpp
index 2cf5ebb2a4cbd0..acd1714a26e071 100644
--- a/clang/unittests/Tooling/LookupTest.cpp
+++ b/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();
diff --git a/clang/unittests/Tooling/QualTypeNamesTest.cpp b/clang/unittests/Tooling/QualTypeNamesTest.cpp
index 686d189cf69eb2..5ded64d4fcc8c5 100644
--- a/clang/unittests/Tooling/QualTypeNamesTest.cpp
+++ b/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 != "") {
diff --git a/clang/unittests/Tooling/RecursiveASTVisitorTestDeclVisitor.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTestDeclVisitor.cpp
index d72a110d37e0fd..eed016e9ee7c24 100644
--- a/clang/unittests/Tooling/RecursiveASTVisitorTestDeclVisitor.cpp
+++ b/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,
diff --git a/clang/unittests/Tooling/RecursiveASTVisitorTestPostOrderVisitor.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTestPostOrderVisitor.cpp
index 8ac0604c09110a..481559ed08efdf 100644
--- a/clang/unittests/Tooling/RecursiveASTVisitorTestPostOrderVisitor.cpp
+++ b/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:
diff --git a/clang/unittests/Tooling/RecursiveASTVisitorTestTypeLocVisitor.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTestTypeLocVisitor.cpp
index a21186265db6a9..eec628ca396417 100644
--- a/clang/unittests/Tooling/RecursiveASTVisitorTestTypeLocVisitor.cpp
+++ b/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;
   }
diff --git a/clang/unittests/Tooling/RecursiveASTVisitorTests/Attr.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTests/Attr.cpp
index 022ef8b8322868..7693e77236b0c4 100644
--- a/clang/unittests/Tooling/RecursiveASTVisitorTests/Attr.cpp
+++ b/clang/unittests/Tooling/RecursiveASTVisitorTests/Attr.cpp
@@ -6,7 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "TestVisitor.h"
+#include "CRTPTestVisitor.h"
 
 using namespace clang;
 
@@ -14,7 +14,7 @@ namespace {
 
 // Check to ensure that attributes and expressions within them are being
 // visited.
-class AttrVisitor : public ExpectedLocationVisitor<AttrVisitor> {
+class AttrVisitor : public CRTPExpectedLocationVisitor<AttrVisitor> {
 public:
   bool VisitMemberExpr(MemberExpr *ME) {
     Match(ME->getMemberDecl()->getNameAsString(), ME->getBeginLoc());
@@ -30,7 +30,6 @@ class AttrVisitor : public ExpectedLocationVisitor<AttrVisitor> {
   }
 };
 
-
 TEST(RecursiveASTVisitor, AttributesAreVisited) {
   AttrVisitor Visitor;
   Visitor.ExpectMatch("Attr", 4, 24);
diff --git a/clang/unittests/Tooling/RecursiveASTVisitorTests/BitfieldInitializer.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTests/BitfieldInitializer.cpp
index c11e726fe85528..c1217179768ac2 100644
--- a/clang/unittests/Tooling/RecursiveASTVisitorTests/BitfieldInitializer.cpp
+++ b/clang/unittests/Tooling/RecursiveASTVisitorTests/BitfieldInitializer.cpp
@@ -14,10 +14,9 @@ using namespace clang;
 namespace {
 
 // Check to ensure that bitfield initializers are visited.
-class BitfieldInitializerVisitor
-    : public ExpectedLocationVisitor<BitfieldInitializerVisitor> {
+class BitfieldInitializerVisitor : public ExpectedLocationVisitor {
 public:
-  bool VisitIntegerLiteral(IntegerLiteral *IL) {
+  bool VisitIntegerLiteral(IntegerLiteral *IL) override {
     Match(std::to_string(IL->getValue().getSExtValue()), IL->getLocation());
     return true;
   }
diff --git a/clang/unittests/Tooling/RecursiveASTVisitorTests/CXXBoolLiteralExpr.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTests/CXXBoolLiteralExpr.cpp
index 1fb192dcda0863..4b0c4c31f2dd2e 100644
--- a/clang/unittests/Tooling/RecursiveASTVisitorTests/CXXBoolLiteralExpr.cpp
+++ b/clang/unittests/Tooling/RecursiveASTVisitorTests/CXXBoolLiteralExpr.cpp
@@ -12,10 +12,9 @@ using namespace clang;
 
 namespace {
 
-class CXXBoolLiteralExprVisitor 
-  : public ExpectedLocationVisitor<CXXBoolLiteralExprVisitor> {
+class CXXBoolLiteralExprVisitor : public ExpectedLocationVisitor {
 public:
-  bool VisitCXXBoolLiteralExpr(CXXBoolLiteralExpr *BE) {
+  bool VisitCXXBoolLiteralExpr(CXXBoolLiteralExpr *BE) override {
     if (BE->getValue())
       Match("true", BE->getLocation());
     else
diff --git a/clang/unittests/Tooling/RecursiveASTVisitorTests/CXXMemberCall.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTests/CXXMemberCall.cpp
index c7b31e06e0e8e9..fe95e8987a73a1 100644
--- a/clang/unittests/Tooling/RecursiveASTVisitorTests/CXXMemberCall.cpp
+++ b/clang/unittests/Tooling/RecursiveASTVisitorTests/CXXMemberCall.cpp
@@ -12,10 +12,9 @@ using namespace clang;
 
 namespace {
 
-class CXXMemberCallVisitor
-  : public ExpectedLocationVisitor<CXXMemberCallVisitor> {
+class CXXMemberCallVisitor : public ExpectedLocationVisitor {
 public:
-  bool VisitCXXMemberCallExpr(CXXMemberCallExpr *Call) {
+  bool VisitCXXMemberCallExpr(CXXMemberCallExpr *Call) override {
     Match(Call->getMethodDecl()->getQualifiedNameAsString(),
           Call->getBeginLoc());
     return true;
diff --git a/clang/unittests/Tooling/RecursiveASTVisitorTests/CXXMethodDecl.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTests/CXXMethodDecl.cpp
index 90fa84bd448124..1eeb3df81a3168 100644
--- a/clang/unittests/Tooling/RecursiveASTVisitorTests/CXXMethodDecl.cpp
+++ b/clang/unittests/Tooling/RecursiveASTVisitorTests/CXXMethodDecl.cpp
@@ -13,25 +13,21 @@ using namespace clang;
 
 namespace {
 
-class CXXMethodDeclVisitor
-    : public ExpectedLocationVisitor<CXXMethodDeclVisitor> {
+class CXXMethodDeclVisitor : public ExpectedLocationVisitor {
 public:
-  CXXMethodDeclVisitor(bool VisitImplicitCode)
-      : VisitImplicitCode(VisitImplicitCode) {}
-
-  bool shouldVisitImplicitCode() const { return VisitImplicitCode; }
+  CXXMethodDeclVisitor(bool VisitImplicitCode) {
+    ShouldVisitImplicitCode = VisitImplicitCode;
+  }
 
-  bool VisitDeclRefExpr(DeclRefExpr *D) {
+  bool VisitDeclRefExpr(DeclRefExpr *D) override {
     Match("declref", D->getLocation());
     return true;
   }
-  bool VisitParmVarDecl(ParmVarDecl *P) {
+
+  bool VisitParmVarDecl(ParmVarDecl *P) override {
     Match("parm", P->getLocation());
     return true;
   }
-
-private:
-  bool VisitImplicitCode;
 };
 
 TEST(RecursiveASTVisitor, CXXMethodDeclNoDefaultBodyVisited) {
diff --git a/clang/unittests/Tooling/RecursiveASTVisitorTests/CXXOperatorCallExprTraverser.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTests/CXXOperatorCallExprTraverser.cpp
index 376874eb351de1..46686199c05d4e 100644
--- a/clang/unittests/Tooling/RecursiveASTVisitorTests/CXXOperatorCallExprTraverser.cpp
+++ b/clang/unittests/Tooling/RecursiveASTVisitorTests/CXXOperatorCallExprTraverser.cpp
@@ -12,15 +12,13 @@ using namespace clang;
 
 namespace {
 
-class CXXOperatorCallExprTraverser
-  : public ExpectedLocationVisitor<CXXOperatorCallExprTraverser> {
+class CXXOperatorCallExprTraverser : public ExpectedLocationVisitor {
 public:
   // Use Traverse, not Visit, to check that data recursion optimization isn't
   // bypassing the call of this function.
-  bool TraverseCXXOperatorCallExpr(CXXOperatorCallExpr *CE) {
+  bool TraverseCXXOperatorCallExpr(CXXOperatorCallExpr *CE) override {
     Match(getOperatorSpelling(CE->getOperator()), CE->getExprLoc());
-    return ExpectedLocationVisitor<CXXOperatorCallExprTraverser>::
-        TraverseCXXOperatorCallExpr(CE);
+    return ExpectedLocationVisitor::TraverseCXXOperatorCallExpr(CE);
   }
 };
 
diff --git a/clang/unittests/Tooling/RecursiveASTVisitorTests/CallbacksCommon.h b/clang/unittests/Tooling/RecursiveASTVisitorTests/CallbacksCommon.h
index 92e30c2d46e5fa..355ecfb452e7e8 100644
--- a/clang/unittests/Tooling/RecursiveASTVisitorTests/CallbacksCommon.h
+++ b/clang/unittests/Tooling/RecursiveASTVisitorTests/CallbacksCommon.h
@@ -6,7 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "TestVisitor.h"
+#include "CRTPTestVisitor.h"
 
 using namespace clang;
 
@@ -21,7 +21,7 @@ enum class ShouldTraversePostOrder : bool {
 /// sequence of calls to user-defined callbacks like Traverse*(), WalkUp*(),
 /// Visit*().
 template <typename Derived>
-class RecordingVisitorBase : public TestVisitor<Derived> {
+class RecordingVisitorBase : public CRTPTestVisitor<Derived> {
   ShouldTraversePostOrder ShouldTraversePostOrderValue;
 
 public:
diff --git a/clang/unittests/Tooling/RecursiveASTVisitorTests/Class.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTests/Class.cpp
index 3ea5abd46a1eca..79dc84b2fdb7b9 100644
--- a/clang/unittests/Tooling/RecursiveASTVisitorTests/Class.cpp
+++ b/clang/unittests/Tooling/RecursiveASTVisitorTests/Class.cpp
@@ -14,10 +14,11 @@ namespace {
 
 // Checks for lambda classes that are not marked as implicitly-generated.
 // (There should be none.)
-class ClassVisitor : public ExpectedLocationVisitor<ClassVisitor> {
+class ClassVisitor : public ExpectedLocationVisitor {
 public:
   ClassVisitor() : SawNonImplicitLambdaClass(false) {}
-  bool VisitCXXRecordDecl(CXXRecordDecl* record) {
+
+  bool VisitCXXRecordDecl(CXXRecordDecl *record) override {
     if (record->isLambda() && !record->isImplicit())
       SawNonImplicitLambdaClass = true;
     return true;
diff --git a/clang/unittests/Tooling/RecursiveASTVisitorTests/Concept.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTests/Concept.cpp
index 6a8d91672f1d93..6dd28e27d7ec2c 100644
--- a/clang/unittests/Tooling/RecursiveASTVisitorTests/Concept.cpp
+++ b/clang/unittests/Tooling/RecursiveASTVisitorTests/Concept.cpp
@@ -16,78 +16,87 @@ using namespace clang;
 
 namespace {
 
-struct ConceptVisitor : ExpectedLocationVisitor<ConceptVisitor> {
-  bool VisitConceptSpecializationExpr(ConceptSpecializationExpr *E) {
+struct ConceptVisitor : ExpectedLocationVisitor {
+  ConceptVisitor(bool VisitImplicitCode = false) {
+    ShouldVisitImplicitCode = VisitImplicitCode;
+  }
+
+  bool VisitConceptSpecializationExpr(ConceptSpecializationExpr *E) override {
     ++ConceptSpecializationExprsVisited;
     return true;
   }
-  bool TraverseTypeConstraint(const TypeConstraint *C) {
+  bool TraverseTypeConstraint(const TypeConstraint *C) override {
     ++TypeConstraintsTraversed;
     return ExpectedLocationVisitor::TraverseTypeConstraint(C);
   }
-  bool TraverseConceptRequirement(concepts::Requirement *R) {
+  bool TraverseConceptRequirement(concepts::Requirement *R) override {
     ++ConceptRequirementsTraversed;
     return ExpectedLocationVisitor::TraverseConceptRequirement(R);
   }
-  bool TraverseConceptReference(ConceptReference *CR) {
+  bool TraverseConceptReference(ConceptReference *CR) override {
     ++ConceptReferencesTraversed;
     return ExpectedLocationVisitor::TraverseConceptReference(CR);
   }
-  bool VisitConceptReference(ConceptReference *CR) {
+  bool VisitConceptReference(ConceptReference *CR) override {
     ++ConceptReferencesVisited;
     return true;
   }
 
-  bool shouldVisitImplicitCode() { return ShouldVisitImplicitCode; }
-
   int ConceptSpecializationExprsVisited = 0;
   int TypeConstraintsTraversed = 0;
   int ConceptRequirementsTraversed = 0;
   int ConceptReferencesTraversed = 0;
   int ConceptReferencesVisited = 0;
-  bool ShouldVisitImplicitCode = false;
 };
 
 TEST(RecursiveASTVisitor, Concepts) {
-  ConceptVisitor Visitor;
-  Visitor.ShouldVisitImplicitCode = true;
-  EXPECT_TRUE(Visitor.runOver("template <typename T> concept Fooable = true;\n"
-                              "template <Fooable T> void bar(T);",
-                              ConceptVisitor::Lang_CXX2a));
-  // Check that we traverse the "Fooable T" template parameter's
-  // TypeConstraint's ImmediatelyDeclaredConstraint, which is a
-  // ConceptSpecializationExpr.
-  EXPECT_EQ(1, Visitor.ConceptSpecializationExprsVisited);
-  // Also check we traversed the TypeConstraint that produced the expr.
-  EXPECT_EQ(1, Visitor.TypeConstraintsTraversed);
-  EXPECT_EQ(1, Visitor.ConceptReferencesTraversed);
-  EXPECT_EQ(1, Visitor.ConceptReferencesVisited);
+  {
+    ConceptVisitor Visitor{true};
+    EXPECT_TRUE(
+        Visitor.runOver("template <typename T> concept Fooable = true;\n"
+                        "template <Fooable T> void bar(T);",
+                        ConceptVisitor::Lang_CXX2a));
+    // Check that we traverse the "Fooable T" template parameter's
+    // TypeConstraint's ImmediatelyDeclaredConstraint, which is a
+    // ConceptSpecializationExpr.
+    EXPECT_EQ(1, Visitor.ConceptSpecializationExprsVisited);
+    // Also check we traversed the TypeConstraint that produced the expr.
+    EXPECT_EQ(1, Visitor.TypeConstraintsTraversed);
+    EXPECT_EQ(1, Visitor.ConceptReferencesTraversed);
+    EXPECT_EQ(1, Visitor.ConceptReferencesVisited);
+  }
 
-  Visitor = {}; // Don't visit implicit code now.
-  EXPECT_TRUE(Visitor.runOver("template <typename T> concept Fooable = true;\n"
-                              "template <Fooable T> void bar(T);",
-                              ConceptVisitor::Lang_CXX2a));
-  // Check that we only visit the TypeConstraint, but not the implicitly
-  // generated immediately declared expression.
-  EXPECT_EQ(0, Visitor.ConceptSpecializationExprsVisited);
-  EXPECT_EQ(1, Visitor.TypeConstraintsTraversed);
-  EXPECT_EQ(1, Visitor.ConceptReferencesTraversed);
-  EXPECT_EQ(1, Visitor.ConceptReferencesVisited);
+  {
+    ConceptVisitor Visitor; // Don't visit implicit code now.
+    EXPECT_TRUE(
+        Visitor.runOver("template <typename T> concept Fooable = true;\n"
+                        "template <Fooable T> void bar(T);",
+                        ConceptVisitor::Lang_CXX2a));
+    // Check that we only visit the TypeConstraint, but not the implicitly
+    // generated immediately declared expression.
+    EXPECT_EQ(0, Visitor.ConceptSpecializationExprsVisited);
+    EXPECT_EQ(1, Visitor.TypeConstraintsTraversed);
+    EXPECT_EQ(1, Visitor.ConceptReferencesTraversed);
+    EXPECT_EQ(1, Visitor.ConceptReferencesVisited);
+  }
 
-  Visitor = {};
-  EXPECT_TRUE(Visitor.runOver("template <class T> concept A = true;\n"
-                              "template <class T> struct vector {};\n"
-                              "template <class T> concept B = requires(T x) {\n"
-                              "  typename vector<T*>;\n"
-                              "  {x} -> A;\n"
-                              "  requires true;\n"
-                              "};",
-                              ConceptVisitor::Lang_CXX2a));
-  EXPECT_EQ(3, Visitor.ConceptRequirementsTraversed);
-  EXPECT_EQ(1, Visitor.ConceptReferencesTraversed);
-  EXPECT_EQ(1, Visitor.ConceptReferencesVisited);
+  {
+    ConceptVisitor Visitor;
+    EXPECT_TRUE(
+        Visitor.runOver("template <class T> concept A = true;\n"
+                        "template <class T> struct vector {};\n"
+                        "template <class T> concept B = requires(T x) {\n"
+                        "  typename vector<T*>;\n"
+                        "  {x} -> A;\n"
+                        "  requires true;\n"
+                        "};",
+                        ConceptVisitor::Lang_CXX2a));
+    EXPECT_EQ(3, Visitor.ConceptRequirementsTraversed);
+    EXPECT_EQ(1, Visitor.ConceptReferencesTraversed);
+    EXPECT_EQ(1, Visitor.ConceptReferencesVisited);
+  }
 
-  Visitor = {};
+  ConceptVisitor Visitor;
   llvm::StringRef Code =
       R"cpp(
 template<typename T> concept True = false;
@@ -107,34 +116,34 @@ struct Foo<F>  {};
   EXPECT_EQ(2, Visitor.ConceptReferencesVisited);
 }
 
-struct VisitDeclOnlyOnce : ExpectedLocationVisitor<VisitDeclOnlyOnce> {
-  bool VisitConceptDecl(ConceptDecl *D) {
+struct VisitDeclOnlyOnce : ExpectedLocationVisitor {
+  VisitDeclOnlyOnce() { ShouldWalkTypesOfTypeLocs = false; }
+
+  bool VisitConceptDecl(ConceptDecl *D) override {
     ++ConceptDeclsVisited;
     return true;
   }
 
-  bool VisitAutoType(AutoType *) {
+  bool VisitAutoType(AutoType *) override {
     ++AutoTypeVisited;
     return true;
   }
-  bool VisitAutoTypeLoc(AutoTypeLoc) {
+  bool VisitAutoTypeLoc(AutoTypeLoc) override {
     ++AutoTypeLocVisited;
     return true;
   }
-  bool VisitConceptReference(ConceptReference *) {
+  bool VisitConceptReference(ConceptReference *) override {
     ++ConceptReferencesVisited;
     return true;
   }
 
-  bool TraverseVarDecl(VarDecl *V) {
+  bool TraverseVarDecl(VarDecl *V) override {
     // The base traversal visits only the `TypeLoc`.
     // However, in the test we also validate the underlying `QualType`.
     TraverseType(V->getType());
     return ExpectedLocationVisitor::TraverseVarDecl(V);
   }
 
-  bool shouldWalkTypesOfTypeLocs() { return false; }
-
   int ConceptDeclsVisited = 0;
   int AutoTypeVisited = 0;
   int AutoTypeLocVisited = 0;
diff --git a/clang/unittests/Tooling/RecursiveASTVisitorTests/ConstructExpr.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTests/ConstructExpr.cpp
index b4f4f54dc7e2f9..7b2ed9715aa80d 100644
--- a/clang/unittests/Tooling/RecursiveASTVisitorTests/ConstructExpr.cpp
+++ b/clang/unittests/Tooling/RecursiveASTVisitorTests/ConstructExpr.cpp
@@ -18,18 +18,11 @@ namespace {
 /// 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 ExpectedLocationVisitor<ConstructExprVisitor> {
+class ConstructExprVisitor : public ExpectedLocationVisitor {
 public:
-  ConstructExprVisitor() : ShouldVisitImplicitCode(false) {}
+  ConstructExprVisitor() { ShouldVisitImplicitCode = false; }
 
-  bool shouldVisitImplicitCode() const { return ShouldVisitImplicitCode; }
-
-  void setShouldVisitImplicitCode(bool NewValue) {
-    ShouldVisitImplicitCode = NewValue;
-  }
-
-  bool VisitCXXConstructExpr(CXXConstructExpr* Expr) {
+  bool VisitCXXConstructExpr(CXXConstructExpr *Expr) override {
     if (const CXXConstructorDecl* Ctor = Expr->getConstructor()) {
       if (const CXXRecordDecl* Class = Ctor->getParent()) {
         Match(Class->getName(), Expr->getLocation());
@@ -37,14 +30,11 @@ class ConstructExprVisitor
     }
     return true;
   }
-
- private:
-  bool ShouldVisitImplicitCode;
 };
 
 TEST(RecursiveASTVisitor, CanVisitImplicitMemberInitializations) {
   ConstructExprVisitor Visitor;
-  Visitor.setShouldVisitImplicitCode(true);
+  Visitor.ShouldVisitImplicitCode = 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.
@@ -60,7 +50,7 @@ TEST(RecursiveASTVisitor, CanVisitImplicitMemberInitializations) {
 // visits are omitted when the visitor does not include implicit code.
 TEST(RecursiveASTVisitor, CanSkipImplicitMemberInitializations) {
   ConstructExprVisitor Visitor;
-  Visitor.setShouldVisitImplicitCode(false);
+  Visitor.ShouldVisitImplicitCode = 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.
diff --git a/clang/unittests/Tooling/RecursiveASTVisitorTests/DeclRefExpr.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTests/DeclRefExpr.cpp
index adc972e1c3d913..6ed986c187eb52 100644
--- a/clang/unittests/Tooling/RecursiveASTVisitorTests/DeclRefExpr.cpp
+++ b/clang/unittests/Tooling/RecursiveASTVisitorTests/DeclRefExpr.cpp
@@ -12,23 +12,14 @@ using namespace clang;
 
 namespace {
 
-class DeclRefExprVisitor : public ExpectedLocationVisitor<DeclRefExprVisitor> {
+class DeclRefExprVisitor : public ExpectedLocationVisitor {
 public:
-  DeclRefExprVisitor() : ShouldVisitImplicitCode(false) {}
+  DeclRefExprVisitor() { ShouldVisitImplicitCode = false; }
 
-  bool shouldVisitImplicitCode() const { return ShouldVisitImplicitCode; }
-
-  void setShouldVisitImplicitCode(bool NewValue) {
-    ShouldVisitImplicitCode = NewValue;
-  }
-
-  bool VisitDeclRefExpr(DeclRefExpr *Reference) {
+  bool VisitDeclRefExpr(DeclRefExpr *Reference) override {
     Match(Reference->getNameInfo().getAsString(), Reference->getLocation());
     return true;
   }
-
-private:
-  bool ShouldVisitImplicitCode;
 };
 
 TEST(RecursiveASTVisitor, VisitsBaseClassTemplateArguments) {
@@ -73,7 +64,7 @@ TEST(RecursiveASTVisitor, VisitsUseOfImplicitLambdaCapture) {
 
 TEST(RecursiveASTVisitor, VisitsImplicitLambdaCaptureInit) {
   DeclRefExprVisitor Visitor;
-  Visitor.setShouldVisitImplicitCode(true);
+  Visitor.ShouldVisitImplicitCode = true;
   // We're expecting "i" to be visited twice: once for the initialization expr
   // for the captured variable "i" outside of the lambda body, and again for
   // the use of "i" inside the lambda.
diff --git a/clang/unittests/Tooling/RecursiveASTVisitorTests/DeductionGuide.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTests/DeductionGuide.cpp
index df878bfc113e57..7d03b283947993 100644
--- a/clang/unittests/Tooling/RecursiveASTVisitorTests/DeductionGuide.cpp
+++ b/clang/unittests/Tooling/RecursiveASTVisitorTests/DeductionGuide.cpp
@@ -13,23 +13,20 @@ using namespace clang;
 
 namespace {
 
-class DeductionGuideVisitor
-    : public ExpectedLocationVisitor<DeductionGuideVisitor> {
+class DeductionGuideVisitor : public ExpectedLocationVisitor {
 public:
-  DeductionGuideVisitor(bool ShouldVisitImplicitCode)
-      : ShouldVisitImplicitCode(ShouldVisitImplicitCode) {}
-  bool VisitCXXDeductionGuideDecl(CXXDeductionGuideDecl *D) {
+  DeductionGuideVisitor(bool VisitImplicitCode) {
+    ShouldVisitImplicitCode = VisitImplicitCode;
+    ShouldVisitTemplateInstantiations = false;
+  }
+
+  bool VisitCXXDeductionGuideDecl(CXXDeductionGuideDecl *D) override {
     std::string Storage;
     llvm::raw_string_ostream Stream(Storage);
     D->print(Stream);
     Match(Storage, D->getLocation());
     return true;
   }
-
-  bool shouldVisitTemplateInstantiations() const { return false; }
-
-  bool shouldVisitImplicitCode() const { return ShouldVisitImplicitCode; }
-  bool ShouldVisitImplicitCode;
 };
 
 TEST(RecursiveASTVisitor, DeductionGuideNonImplicitMode) {
diff --git a/clang/unittests/Tooling/RecursiveASTVisitorTests/ImplicitCtor.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTests/ImplicitCtor.cpp
index 27999e5ef8efcb..dc9455a01e4882 100644
--- a/clang/unittests/Tooling/RecursiveASTVisitorTests/ImplicitCtor.cpp
+++ b/clang/unittests/Tooling/RecursiveASTVisitorTests/ImplicitCtor.cpp
@@ -13,12 +13,9 @@ using namespace clang;
 namespace {
 
 // A visitor that visits implicit declarations and matches constructors.
-class ImplicitCtorVisitor
-    : public ExpectedLocationVisitor<ImplicitCtorVisitor> {
+class ImplicitCtorVisitor : public ExpectedLocationVisitor {
 public:
-  bool shouldVisitImplicitCode() const { return true; }
-
-  bool VisitCXXConstructorDecl(CXXConstructorDecl* Ctor) {
+  bool VisitCXXConstructorDecl(CXXConstructorDecl *Ctor) override {
     if (Ctor->isImplicit()) {  // Was not written in source code
       if (const CXXRecordDecl* Class = Ctor->getParent()) {
         Match(Class->getName(), Ctor->getLocation());
diff --git a/clang/unittests/Tooling/RecursiveASTVisitorTests/ImplicitCtorInitializer.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTests/ImplicitCtorInitializer.cpp
index c762e1cffc3686..ae952c33bfe78b 100644
--- a/clang/unittests/Tooling/RecursiveASTVisitorTests/ImplicitCtorInitializer.cpp
+++ b/clang/unittests/Tooling/RecursiveASTVisitorTests/ImplicitCtorInitializer.cpp
@@ -12,26 +12,20 @@ using namespace clang;
 
 namespace {
 
-class CXXCtorInitializerVisitor
-    : public ExpectedLocationVisitor<CXXCtorInitializerVisitor> {
+class CXXCtorInitializerVisitor : public ExpectedLocationVisitor {
 public:
-  CXXCtorInitializerVisitor(bool VisitImplicitCode)
-      : VisitImplicitCode(VisitImplicitCode) {}
-
-  bool shouldVisitImplicitCode() const { return VisitImplicitCode; }
+  CXXCtorInitializerVisitor(bool VisitImplicitCode) {
+    ShouldVisitImplicitCode = VisitImplicitCode;
+  }
 
-  bool TraverseConstructorInitializer(CXXCtorInitializer *Init) {
+  bool TraverseConstructorInitializer(CXXCtorInitializer *Init) override {
     if (!Init->isWritten())
       VisitedImplicitInitializer = true;
     Match("initializer", Init->getSourceLocation());
-    return ExpectedLocationVisitor<
-        CXXCtorInitializerVisitor>::TraverseConstructorInitializer(Init);
+    return ExpectedLocationVisitor::TraverseConstructorInitializer(Init);
   }
 
   bool VisitedImplicitInitializer = false;
-
-private:
-  bool VisitImplicitCode;
 };
 
 // Check to ensure that CXXCtorInitializer is not visited when implicit code
diff --git a/clang/unittests/Tooling/RecursiveASTVisitorTests/InitListExprPostOrder.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTests/InitListExprPostOrder.cpp
index 80d9c9873505b4..5836fc1b2e8de5 100644
--- a/clang/unittests/Tooling/RecursiveASTVisitorTests/InitListExprPostOrder.cpp
+++ b/clang/unittests/Tooling/RecursiveASTVisitorTests/InitListExprPostOrder.cpp
@@ -6,14 +6,14 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "TestVisitor.h"
+#include "CRTPTestVisitor.h"
 
 using namespace clang;
 
 namespace {
 
 class InitListExprPostOrderVisitor
-    : public ExpectedLocationVisitor<InitListExprPostOrderVisitor> {
+    : public CRTPExpectedLocationVisitor<InitListExprPostOrderVisitor> {
 public:
   bool shouldTraversePostOrder() const { return true; }
 
diff --git a/clang/unittests/Tooling/RecursiveASTVisitorTests/InitListExprPostOrderNoQueue.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTests/InitListExprPostOrderNoQueue.cpp
index 8750f78349443e..20140d2dcbf9e4 100644
--- a/clang/unittests/Tooling/RecursiveASTVisitorTests/InitListExprPostOrderNoQueue.cpp
+++ b/clang/unittests/Tooling/RecursiveASTVisitorTests/InitListExprPostOrderNoQueue.cpp
@@ -6,19 +6,19 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "TestVisitor.h"
+#include "CRTPTestVisitor.h"
 
 using namespace clang;
 
 namespace {
 
 class InitListExprPostOrderNoQueueVisitor
-    : public ExpectedLocationVisitor<InitListExprPostOrderNoQueueVisitor> {
+    : public CRTPExpectedLocationVisitor<InitListExprPostOrderNoQueueVisitor> {
 public:
   bool shouldTraversePostOrder() const { return true; }
 
   bool TraverseInitListExpr(InitListExpr *ILE) {
-    return ExpectedLocationVisitor::TraverseInitListExpr(ILE);
+    return CRTPExpectedLocationVisitor::TraverseInitListExpr(ILE);
   }
 
   bool VisitInitListExpr(InitListExpr *ILE) {
diff --git a/clang/unittests/Tooling/RecursiveASTVisitorTests/InitListExprPreOrder.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTests/InitListExprPreOrder.cpp
index 3fa1529ea0eefe..933d25898390dd 100644
--- a/clang/unittests/Tooling/RecursiveASTVisitorTests/InitListExprPreOrder.cpp
+++ b/clang/unittests/Tooling/RecursiveASTVisitorTests/InitListExprPreOrder.cpp
@@ -14,21 +14,16 @@ namespace {
 
 // Check to ensure that InitListExpr is visited twice, once each for the
 // syntactic and semantic form.
-class InitListExprPreOrderVisitor
-    : public ExpectedLocationVisitor<InitListExprPreOrderVisitor> {
+class InitListExprPreOrderVisitor : public ExpectedLocationVisitor {
 public:
-  InitListExprPreOrderVisitor(bool VisitImplicitCode)
-      : VisitImplicitCode(VisitImplicitCode) {}
-
-  bool shouldVisitImplicitCode() const { return VisitImplicitCode; }
+  InitListExprPreOrderVisitor(bool VisitImplicitCode) {
+    ShouldVisitImplicitCode = VisitImplicitCode;
+  }
 
-  bool VisitInitListExpr(InitListExpr *ILE) {
+  bool VisitInitListExpr(InitListExpr *ILE) override {
     Match(ILE->isSemanticForm() ? "semantic" : "syntactic", ILE->getBeginLoc());
     return true;
   }
-
-private:
-  bool VisitImplicitCode;
 };
 
 TEST(RecursiveASTVisitor, InitListExprIsPreOrderVisitedTwice) {
diff --git a/clang/unittests/Tooling/RecursiveASTVisitorTests/InitListExprPreOrderNoQueue.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTests/InitListExprPreOrderNoQueue.cpp
index 8db88e1e063975..0dcd11b8027f04 100644
--- a/clang/unittests/Tooling/RecursiveASTVisitorTests/InitListExprPreOrderNoQueue.cpp
+++ b/clang/unittests/Tooling/RecursiveASTVisitorTests/InitListExprPreOrderNoQueue.cpp
@@ -12,14 +12,13 @@ using namespace clang;
 
 namespace {
 
-class InitListExprPreOrderNoQueueVisitor
-    : public ExpectedLocationVisitor<InitListExprPreOrderNoQueueVisitor> {
+class InitListExprPreOrderNoQueueVisitor : public ExpectedLocationVisitor {
 public:
-  bool TraverseInitListExpr(InitListExpr *ILE) {
+  bool TraverseInitListExpr(InitListExpr *ILE) override {
     return ExpectedLocationVisitor::TraverseInitListExpr(ILE);
   }
 
-  bool VisitInitListExpr(InitListExpr *ILE) {
+  bool VisitInitListExpr(InitListExpr *ILE) override {
     Match(ILE->isSemanticForm() ? "semantic" : "syntactic", ILE->getBeginLoc());
     return true;
   }
diff --git a/clang/unittests/Tooling/RecursiveASTVisitorTests/IntegerLiteral.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTests/IntegerLiteral.cpp
index 3fc3cb1a99a744..83136fc11edb2c 100644
--- a/clang/unittests/Tooling/RecursiveASTVisitorTests/IntegerLiteral.cpp
+++ b/clang/unittests/Tooling/RecursiveASTVisitorTests/IntegerLiteral.cpp
@@ -13,10 +13,9 @@ using namespace clang;
 namespace {
 
 // Check to ensure that implicit default argument expressions are visited.
-class IntegerLiteralVisitor
-    : public ExpectedLocationVisitor<IntegerLiteralVisitor> {
+class IntegerLiteralVisitor : public ExpectedLocationVisitor {
 public:
-  bool VisitIntegerLiteral(const IntegerLiteral *IL) {
+  bool VisitIntegerLiteral(IntegerLiteral *IL) override {
     Match("literal", IL->getLocation());
     return true;
   }
diff --git a/clang/unittests/Tooling/RecursiveASTVisitorTests/LambdaDefaultCapture.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTests/LambdaDefaultCapture.cpp
index b1d6d593e733a9..4a9175ed2dda24 100644
--- a/clang/unittests/Tooling/RecursiveASTVisitorTests/LambdaDefaultCapture.cpp
+++ b/clang/unittests/Tooling/RecursiveASTVisitorTests/LambdaDefaultCapture.cpp
@@ -13,10 +13,9 @@ using namespace clang;
 namespace {
 
 // Matches the (optional) capture-default of a lambda-introducer.
-class LambdaDefaultCaptureVisitor
-  : public ExpectedLocationVisitor<LambdaDefaultCaptureVisitor> {
+class LambdaDefaultCaptureVisitor : public ExpectedLocationVisitor {
 public:
-  bool VisitLambdaExpr(LambdaExpr *Lambda) {
+  bool VisitLambdaExpr(LambdaExpr *Lambda) override {
     if (Lambda->getCaptureDefault() != LCD_None) {
       Match("", Lambda->getCaptureDefaultLoc());
     }
diff --git a/clang/unittests/Tooling/RecursiveASTVisitorTests/LambdaExpr.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTests/LambdaExpr.cpp
index 337dace5fd2274..0ce7f4b18762f2 100644
--- a/clang/unittests/Tooling/RecursiveASTVisitorTests/LambdaExpr.cpp
+++ b/clang/unittests/Tooling/RecursiveASTVisitorTests/LambdaExpr.cpp
@@ -14,9 +14,11 @@ using namespace clang;
 
 namespace {
 
-class LambdaExprVisitor : public ExpectedLocationVisitor<LambdaExprVisitor> {
+class LambdaExprVisitor : public ExpectedLocationVisitor {
 public:
-  bool VisitLambdaExpr(LambdaExpr *Lambda) {
+  LambdaExprVisitor() { ShouldVisitImplicitCode = false; }
+
+  bool VisitLambdaExpr(LambdaExpr *Lambda) override {
     PendingBodies.push(Lambda->getBody());
     PendingClasses.push(Lambda->getLambdaClass());
     Match("", Lambda->getIntroducerRange().getBegin());
@@ -24,12 +26,12 @@ class LambdaExprVisitor : public ExpectedLocationVisitor<LambdaExprVisitor> {
   }
   /// For each call to VisitLambdaExpr, we expect a subsequent call to visit
   /// the body (and maybe the lambda class, which is implicit).
-  bool VisitStmt(Stmt *S) {
+  bool VisitStmt(Stmt *S) override {
     if (!PendingBodies.empty() && S == PendingBodies.top())
       PendingBodies.pop();
     return true;
   }
-  bool VisitDecl(Decl *D) {
+  bool VisitDecl(Decl *D) override {
     if (!PendingClasses.empty() && D == PendingClasses.top())
       PendingClasses.pop();
     return true;
@@ -38,9 +40,6 @@ class LambdaExprVisitor : public ExpectedLocationVisitor<LambdaExprVisitor> {
   bool allBodiesHaveBeenTraversed() const { return PendingBodies.empty(); }
   bool allClassesHaveBeenTraversed() const { return PendingClasses.empty(); }
 
-  bool VisitImplicitCode = false;
-  bool shouldVisitImplicitCode() const { return VisitImplicitCode; }
-
 private:
   std::stack<Stmt *> PendingBodies;
   std::stack<Decl *> PendingClasses;
@@ -67,7 +66,7 @@ TEST(RecursiveASTVisitor, LambdaInLambda) {
 
 TEST(RecursiveASTVisitor, TopLevelLambda) {
   LambdaExprVisitor Visitor;
-  Visitor.VisitImplicitCode = true;
+  Visitor.ShouldVisitImplicitCode = true;
   Visitor.ExpectMatch("", 1, 10);
   Visitor.ExpectMatch("", 1, 14);
   EXPECT_TRUE(Visitor.runOver("auto x = []{ [] {}; };",
@@ -78,7 +77,7 @@ TEST(RecursiveASTVisitor, TopLevelLambda) {
 
 TEST(RecursiveASTVisitor, VisitsLambdaExprAndImplicitClass) {
   LambdaExprVisitor Visitor;
-  Visitor.VisitImplicitCode = true;
+  Visitor.ShouldVisitImplicitCode = true;
   Visitor.ExpectMatch("", 1, 12);
   EXPECT_TRUE(Visitor.runOver("void f() { []{ return; }(); }",
                               LambdaExprVisitor::Lang_CXX11));
diff --git a/clang/unittests/Tooling/RecursiveASTVisitorTests/LambdaTemplateParams.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTests/LambdaTemplateParams.cpp
index c355e3f1083f96..cfac3a3c5ad988 100644
--- a/clang/unittests/Tooling/RecursiveASTVisitorTests/LambdaTemplateParams.cpp
+++ b/clang/unittests/Tooling/RecursiveASTVisitorTests/LambdaTemplateParams.cpp
@@ -13,24 +13,23 @@ using namespace clang;
 namespace {
 
 // Matches (optional) explicit template parameters.
-class LambdaTemplateParametersVisitor
-  : public ExpectedLocationVisitor<LambdaTemplateParametersVisitor> {
+class LambdaTemplateParametersVisitor : public ExpectedLocationVisitor {
 public:
-  bool shouldVisitImplicitCode() const { return false; }
+  LambdaTemplateParametersVisitor() { ShouldVisitImplicitCode = false; }
 
-  bool VisitTemplateTypeParmDecl(TemplateTypeParmDecl *D) {
+  bool VisitTemplateTypeParmDecl(TemplateTypeParmDecl *D) override {
     EXPECT_FALSE(D->isImplicit());
     Match(D->getName(), D->getBeginLoc());
     return true;
   }
 
-  bool VisitNonTypeTemplateParmDecl(NonTypeTemplateParmDecl *D) {
+  bool VisitNonTypeTemplateParmDecl(NonTypeTemplateParmDecl *D) override {
     EXPECT_FALSE(D->isImplicit());
     Match(D->getName(), D->getBeginLoc());
     return true;
   }
 
-  bool VisitTemplateTemplateParmDecl(TemplateTemplateParmDecl *D) {
+  bool VisitTemplateTemplateParmDecl(TemplateTemplateParmDecl *D) override {
     EXPECT_FALSE(D->isImplicit());
     Match(D->getName(), D->getBeginLoc());
     return true;
diff --git a/clang/unittests/Tooling/RecursiveASTVisitorTests/MemberPointerTypeLoc.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTests/MemberPointerTypeLoc.cpp
index d67bd0395a670e..587a00dd270511 100644
--- a/clang/unittests/Tooling/RecursiveASTVisitorTests/MemberPointerTypeLoc.cpp
+++ b/clang/unittests/Tooling/RecursiveASTVisitorTests/MemberPointerTypeLoc.cpp
@@ -13,16 +13,15 @@ using namespace clang;
 
 namespace {
 
-class MemberPointerTypeLocVisitor
-    : public ExpectedLocationVisitor<MemberPointerTypeLocVisitor> {
+class MemberPointerTypeLocVisitor : public ExpectedLocationVisitor {
 public:
-  bool VisitTemplateTypeParmTypeLoc(TemplateTypeParmTypeLoc TL) {
+  bool VisitTemplateTypeParmTypeLoc(TemplateTypeParmTypeLoc TL) override {
     if (!TL)
       return true;
     Match(TL.getDecl()->getName(), TL.getNameLoc());
     return true;
   }
-  bool VisitRecordTypeLoc(RecordTypeLoc RTL) {
+  bool VisitRecordTypeLoc(RecordTypeLoc RTL) override {
     if (!RTL)
       return true;
     Match(RTL.getDecl()->getName(), RTL.getNameLoc());
diff --git a/clang/unittests/Tooling/RecursiveASTVisitorTests/NestedNameSpecifiers.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTests/NestedNameSpecifiers.cpp
index 868a3988c756d2..ddc663e2b6fd3f 100644
--- a/clang/unittests/Tooling/RecursiveASTVisitorTests/NestedNameSpecifiers.cpp
+++ b/clang/unittests/Tooling/RecursiveASTVisitorTests/NestedNameSpecifiers.cpp
@@ -13,17 +13,16 @@ using namespace clang;
 namespace {
 
 // Check to ensure that nested name specifiers are visited.
-class NestedNameSpecifiersVisitor
-    : public ExpectedLocationVisitor<NestedNameSpecifiersVisitor> {
+class NestedNameSpecifiersVisitor : public ExpectedLocationVisitor {
 public:
-  bool VisitRecordTypeLoc(RecordTypeLoc RTL) {
+  bool VisitRecordTypeLoc(RecordTypeLoc RTL) override {
     if (!RTL)
       return true;
     Match(RTL.getDecl()->getName(), RTL.getNameLoc());
     return true;
   }
 
-  bool TraverseNestedNameSpecifierLoc(NestedNameSpecifierLoc NNS) {
+  bool TraverseNestedNameSpecifierLoc(NestedNameSpecifierLoc NNS) override {
     if (!NNS)
       return true;
     if (const NamespaceDecl *ND =
diff --git a/clang/unittests/Tooling/RecursiveASTVisitorTests/ParenExpr.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTests/ParenExpr.cpp
index c316f98f40ce0f..89ccf20587ad06 100644
--- a/clang/unittests/Tooling/RecursiveASTVisitorTests/ParenExpr.cpp
+++ b/clang/unittests/Tooling/RecursiveASTVisitorTests/ParenExpr.cpp
@@ -12,9 +12,9 @@ using namespace clang;
 
 namespace {
 
-class ParenExprVisitor : public ExpectedLocationVisitor<ParenExprVisitor> {
+class ParenExprVisitor : public ExpectedLocationVisitor {
 public:
-  bool VisitParenExpr(ParenExpr *Parens) {
+  bool VisitParenExpr(ParenExpr *Parens) override {
     Match("", Parens->getExprLoc());
     return true;
   }
diff --git a/clang/unittests/Tooling/RecursiveASTVisitorTests/TemplateArgumentLocTraverser.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTests/TemplateArgumentLocTraverser.cpp
index b87e89f3fd56ff..e5e743ab8d76da 100644
--- a/clang/unittests/Tooling/RecursiveASTVisitorTests/TemplateArgumentLocTraverser.cpp
+++ b/clang/unittests/Tooling/RecursiveASTVisitorTests/TemplateArgumentLocTraverser.cpp
@@ -12,18 +12,16 @@ using namespace clang;
 
 namespace {
 
-class TemplateArgumentLocTraverser
-  : public ExpectedLocationVisitor<TemplateArgumentLocTraverser> {
+class TemplateArgumentLocTraverser : public ExpectedLocationVisitor {
 public:
-  bool TraverseTemplateArgumentLoc(const TemplateArgumentLoc &ArgLoc) {
+  bool TraverseTemplateArgumentLoc(const TemplateArgumentLoc &ArgLoc) override {
     std::string ArgStr;
     llvm::raw_string_ostream Stream(ArgStr);
     const TemplateArgument &Arg = ArgLoc.getArgument();
 
     Arg.print(Context->getPrintingPolicy(), Stream, /*IncludeType*/ true);
     Match(ArgStr, ArgLoc.getLocation());
-    return ExpectedLocationVisitor<TemplateArgumentLocTraverser>::
-      TraverseTemplateArgumentLoc(ArgLoc);
+    return ExpectedLocationVisitor::TraverseTemplateArgumentLoc(ArgLoc);
   }
 };
 
diff --git a/clang/unittests/Tooling/RecursiveASTVisitorTests/TraversalScope.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTests/TraversalScope.cpp
index 9e71f9554e5951..2feddf58cac7b2 100644
--- a/clang/unittests/Tooling/RecursiveASTVisitorTests/TraversalScope.cpp
+++ b/clang/unittests/Tooling/RecursiveASTVisitorTests/TraversalScope.cpp
@@ -12,17 +12,17 @@ using namespace clang;
 
 namespace {
 
-class Visitor : public ExpectedLocationVisitor<Visitor, clang::TestVisitor> {
+class Visitor : public ExpectedLocationVisitor {
 public:
   Visitor(ASTContext *Context) { this->Context = Context; }
 
-  bool VisitTranslationUnitDecl(TranslationUnitDecl *D) {
+  bool VisitTranslationUnitDecl(TranslationUnitDecl *D) override {
     auto &SM = D->getParentASTContext().getSourceManager();
     Match("TU", SM.getLocForStartOfFile(SM.getMainFileID()));
     return true;
   }
 
-  bool VisitNamedDecl(NamedDecl *D) {
+  bool VisitNamedDecl(NamedDecl *D) override {
     if (!D->isImplicit())
       Match(D->getName(), D->getLocation());
     return true;
diff --git a/clang/unittests/Tooling/RefactoringTest.cpp b/clang/unittests/Tooling/RefactoringTest.cpp
index 4f0cccdc274963..254d95bc20cb01 100644
--- a/clang/unittests/Tooling/RefactoringTest.cpp
+++ b/clang/unittests/Tooling/RefactoringTest.cpp
@@ -13,7 +13,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclGroup.h"
-#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/DynamicRecursiveASTVisitor.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/FileManager.h"
@@ -647,8 +647,7 @@ TEST_F(FlushRewrittenFilesTest, StoresChangesOnDisk) {
 }
 
 namespace {
-template <typename T>
-class TestVisitor : public clang::RecursiveASTVisitor<T> {
+class TestVisitor : public DynamicRecursiveASTVisitor {
 public:
   bool runOver(StringRef Code) {
     return runToolOnCode(std::make_unique<TestAction>(this), Code);
@@ -698,9 +697,9 @@ void expectReplacementAt(const Replacement &Replace,
   EXPECT_EQ(Length, Replace.getLength());
 }
 
-class ClassDeclXVisitor : public TestVisitor<ClassDeclXVisitor> {
+class ClassDeclXVisitor : public TestVisitor {
 public:
-  bool VisitCXXRecordDecl(CXXRecordDecl *Record) {
+  bool VisitCXXRecordDecl(CXXRecordDecl *Record) override {
     if (Record->getName() == "X") {
       Replace = Replacement(*SM, Record, "");
     }
@@ -721,9 +720,9 @@ TEST(Replacement, ReplacesAtSpellingLocation) {
   expectReplacementAt(ClassDeclX.Replace, "input.cc", 17, 7);
 }
 
-class CallToFVisitor : public TestVisitor<CallToFVisitor> {
+class CallToFVisitor : public TestVisitor {
 public:
-  bool VisitCallExpr(CallExpr *Call) {
+  bool VisitCallExpr(CallExpr *Call) override {
     if (Call->getDirectCallee()->getName() == "F") {
       Replace = Replacement(*SM, Call, "");
     }
@@ -745,10 +744,9 @@ TEST(Replacement, TemplatedFunctionCall) {
   expectReplacementAt(CallToF.Replace, "input.cc", 43, 8);
 }
 
-class NestedNameSpecifierAVisitor
-    : public TestVisitor<NestedNameSpecifierAVisitor> {
+class NestedNameSpecifierAVisitor : public TestVisitor {
 public:
-  bool TraverseNestedNameSpecifierLoc(NestedNameSpecifierLoc NNSLoc) {
+  bool TraverseNestedNameSpecifierLoc(NestedNameSpecifierLoc NNSLoc) override {
     if (NNSLoc.getNestedNameSpecifier()) {
       if (const NamespaceDecl* NS = NNSLoc.getNestedNameSpecifier()->getAsNamespace()) {
         if (NS->getName() == "a") {
@@ -756,8 +754,7 @@ class NestedNameSpecifierAVisitor
         }
       }
     }
-    return TestVisitor<NestedNameSpecifierAVisitor>::TraverseNestedNameSpecifierLoc(
-        NNSLoc);
+    return TestVisitor::TraverseNestedNameSpecifierLoc(NNSLoc);
   }
   Replacement Replace;
 };
diff --git a/clang/unittests/Tooling/SourceCodeTest.cpp b/clang/unittests/Tooling/SourceCodeTest.cpp
index 3c24b6220a224a..549b77752f1c21 100644
--- a/clang/unittests/Tooling/SourceCodeTest.cpp
+++ b/clang/unittests/Tooling/SourceCodeTest.cpp
@@ -35,8 +35,8 @@ using tooling::validateEditRange;
 
 namespace {
 
-struct IntLitVisitor : TestVisitor<IntLitVisitor> {
-  bool VisitIntegerLiteral(IntegerLiteral *Expr) {
+struct IntLitVisitor : TestVisitor {
+  bool VisitIntegerLiteral(IntegerLiteral *Expr) override {
     OnIntLit(Expr, Context);
     return true;
   }
@@ -44,8 +44,8 @@ struct IntLitVisitor : TestVisitor<IntLitVisitor> {
   std::function<void(IntegerLiteral *, ASTContext *Context)> OnIntLit;
 };
 
-struct CallsVisitor : TestVisitor<CallsVisitor> {
-  bool VisitCallExpr(CallExpr *Expr) {
+struct CallsVisitor : TestVisitor {
+  bool VisitCallExpr(CallExpr *Expr) override {
     OnCall(Expr, Context);
     return true;
   }
@@ -53,8 +53,8 @@ struct CallsVisitor : TestVisitor<CallsVisitor> {
   std::function<void(CallExpr *, ASTContext *Context)> OnCall;
 };
 
-struct TypeLocVisitor : TestVisitor<TypeLocVisitor> {
-  bool VisitTypeLoc(TypeLoc TL) {
+struct TypeLocVisitor : TestVisitor {
+  bool VisitTypeLoc(TypeLoc TL) override {
     OnTypeLoc(TL, Context);
     return true;
   }
@@ -97,7 +97,7 @@ static ::testing::Matcher<CharSourceRange> AsRange(const SourceManager &SM,
 
 // Base class for visitors that expect a single match corresponding to a
 // specific annotated range.
-template <typename T> class AnnotatedCodeVisitor : public TestVisitor<T> {
+class AnnotatedCodeVisitor : public TestVisitor {
 protected:
   int MatchCount = 0;
   llvm::Annotations Code;
@@ -199,9 +199,8 @@ TEST(SourceCodeTest, getExtendedText) {
 }
 
 TEST(SourceCodeTest, maybeExtendRange_TokenRange) {
-  struct ExtendTokenRangeVisitor
-      : AnnotatedCodeVisitor<ExtendTokenRangeVisitor> {
-    bool VisitCallExpr(CallExpr *CE) {
+  struct ExtendTokenRangeVisitor : AnnotatedCodeVisitor {
+    bool VisitCallExpr(CallExpr *CE) override {
       ++MatchCount;
       EXPECT_THAT(getExtendedRange(*CE, tok::TokenKind::semi, *Context),
                   EqualsAnnotatedRange(Context, Code.range("r")));
@@ -218,8 +217,8 @@ TEST(SourceCodeTest, maybeExtendRange_TokenRange) {
 }
 
 TEST(SourceCodeTest, maybeExtendRange_CharRange) {
-  struct ExtendCharRangeVisitor : AnnotatedCodeVisitor<ExtendCharRangeVisitor> {
-    bool VisitCallExpr(CallExpr *CE) {
+  struct ExtendCharRangeVisitor : AnnotatedCodeVisitor {
+    bool VisitCallExpr(CallExpr *CE) override {
       ++MatchCount;
       CharSourceRange Call = Lexer::getAsCharRange(CE->getSourceRange(),
                                                    Context->getSourceManager(),
@@ -238,8 +237,8 @@ TEST(SourceCodeTest, maybeExtendRange_CharRange) {
 }
 
 TEST(SourceCodeTest, getAssociatedRange) {
-  struct VarDeclsVisitor : AnnotatedCodeVisitor<VarDeclsVisitor> {
-    bool VisitVarDecl(VarDecl *Decl) { return VisitDeclHelper(Decl); }
+  struct VarDeclsVisitor : AnnotatedCodeVisitor {
+    bool VisitVarDecl(VarDecl *Decl) override { return VisitDeclHelper(Decl); }
   };
   VarDeclsVisitor Visitor;
 
@@ -283,8 +282,10 @@ TEST(SourceCodeTest, getAssociatedRange) {
 }
 
 TEST(SourceCodeTest, getAssociatedRangeClasses) {
-  struct RecordDeclsVisitor : AnnotatedCodeVisitor<RecordDeclsVisitor> {
-    bool VisitRecordDecl(RecordDecl *Decl) { return VisitDeclHelper(Decl); }
+  struct RecordDeclsVisitor : AnnotatedCodeVisitor {
+    bool VisitRecordDecl(RecordDecl *Decl) override {
+      return VisitDeclHelper(Decl);
+    }
   };
   RecordDeclsVisitor Visitor;
 
@@ -297,8 +298,8 @@ TEST(SourceCodeTest, getAssociatedRangeClasses) {
 }
 
 TEST(SourceCodeTest, getAssociatedRangeClassTemplateSpecializations) {
-  struct CXXRecordDeclsVisitor : AnnotatedCodeVisitor<CXXRecordDeclsVisitor> {
-    bool VisitCXXRecordDecl(CXXRecordDecl *Decl) {
+  struct CXXRecordDeclsVisitor : AnnotatedCodeVisitor {
+    bool VisitCXXRecordDecl(CXXRecordDecl *Decl) override {
       return Decl->getTemplateSpecializationKind() !=
                  TSK_ExplicitSpecialization ||
              VisitDeclHelper(Decl);
@@ -315,8 +316,10 @@ TEST(SourceCodeTest, getAssociatedRangeClassTemplateSpecializations) {
 }
 
 TEST(SourceCodeTest, getAssociatedRangeFunctions) {
-  struct FunctionDeclsVisitor : AnnotatedCodeVisitor<FunctionDeclsVisitor> {
-    bool VisitFunctionDecl(FunctionDecl *Decl) { return VisitDeclHelper(Decl); }
+  struct FunctionDeclsVisitor : AnnotatedCodeVisitor {
+    bool VisitFunctionDecl(FunctionDecl *Decl) override {
+      return VisitDeclHelper(Decl);
+    }
   };
   FunctionDeclsVisitor Visitor;
 
@@ -328,8 +331,8 @@ TEST(SourceCodeTest, getAssociatedRangeFunctions) {
 }
 
 TEST(SourceCodeTest, getAssociatedRangeMemberTemplates) {
-  struct CXXMethodDeclsVisitor : AnnotatedCodeVisitor<CXXMethodDeclsVisitor> {
-    bool VisitCXXMethodDecl(CXXMethodDecl *Decl) {
+  struct CXXMethodDeclsVisitor : AnnotatedCodeVisitor {
+    bool VisitCXXMethodDecl(CXXMethodDecl *Decl) override {
       // Only consider the definition of the template.
       return !Decl->doesThisDeclarationHaveABody() || VisitDeclHelper(Decl);
     }
@@ -346,8 +349,8 @@ TEST(SourceCodeTest, getAssociatedRangeMemberTemplates) {
 }
 
 TEST(SourceCodeTest, getAssociatedRangeWithComments) {
-  struct VarDeclsVisitor : AnnotatedCodeVisitor<VarDeclsVisitor> {
-    bool VisitVarDecl(VarDecl *Decl) { return VisitDeclHelper(Decl); }
+  struct VarDeclsVisitor : AnnotatedCodeVisitor {
+    bool VisitVarDecl(VarDecl *Decl) override { return VisitDeclHelper(Decl); }
   };
 
   VarDeclsVisitor Visitor;
@@ -447,9 +450,9 @@ TEST(SourceCodeTest, getAssociatedRangeWithComments) {
 }
 
 TEST(SourceCodeTest, getAssociatedRangeInvalidForPartialExpansions) {
-  struct FailingVarDeclsVisitor : TestVisitor<FailingVarDeclsVisitor> {
+  struct FailingVarDeclsVisitor : TestVisitor {
     FailingVarDeclsVisitor() {}
-    bool VisitVarDecl(VarDecl *Decl) {
+    bool VisitVarDecl(VarDecl *Decl) override {
       EXPECT_TRUE(getAssociatedRange(*Decl, *Context).isInvalid());
       return true;
     }
diff --git a/clang/unittests/Tooling/TestVisitor.h b/clang/unittests/Tooling/TestVisitor.h
index 751ca74d1a881c..fdf57a946a6e2d 100644
--- a/clang/unittests/Tooling/TestVisitor.h
+++ b/clang/unittests/Tooling/TestVisitor.h
@@ -16,7 +16,7 @@
 
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
-#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/DynamicRecursiveASTVisitor.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Tooling/Tooling.h"
@@ -24,20 +24,11 @@
 #include <vector>
 
 namespace clang {
-
-/// \brief Base class for simple RecursiveASTVisitor based tests.
-///
-/// This is a drop-in replacement for RecursiveASTVisitor itself, with the
-/// additional capability of running it over a snippet of code.
-///
-/// Visits template instantiations and implicit code by default.
-template <typename T>
-class TestVisitor : public RecursiveASTVisitor<T> {
+namespace detail {
+// Use 'TestVisitor' or include 'CRTPTestVisitor.h' and use 'CRTPTestVisitor'
+// instead of using this directly.
+class TestVisitorHelper {
 public:
-  TestVisitor() { }
-
-  virtual ~TestVisitor() { }
-
   enum Language {
     Lang_C,
     Lang_CXX98,
@@ -54,57 +45,63 @@ class TestVisitor : public RecursiveASTVisitor<T> {
   bool runOver(StringRef Code, Language L = Lang_CXX) {
     std::vector<std::string> Args;
     switch (L) {
-      case Lang_C:
-        Args.push_back("-x");
-        Args.push_back("c");
-        break;
-      case Lang_CXX98: Args.push_back("-std=c++98"); break;
-      case Lang_CXX11: Args.push_back("-std=c++11"); break;
-      case Lang_CXX14: Args.push_back("-std=c++14"); break;
-      case Lang_CXX17: Args.push_back("-std=c++17"); break;
-      case Lang_CXX2a: Args.push_back("-std=c++2a"); break;
-      case Lang_OBJC:
-        Args.push_back("-ObjC");
-        Args.push_back("-fobjc-runtime=macosx-10.12.0");
-        break;
-      case Lang_OBJCXX11:
-        Args.push_back("-ObjC++");
-        Args.push_back("-std=c++11");
-        Args.push_back("-fblocks");
-        break;
+    case Lang_C:
+      Args.push_back("-x");
+      Args.push_back("c");
+      break;
+    case Lang_CXX98:
+      Args.push_back("-std=c++98");
+      break;
+    case Lang_CXX11:
+      Args.push_back("-std=c++11");
+      break;
+    case Lang_CXX14:
+      Args.push_back("-std=c++14");
+      break;
+    case Lang_CXX17:
+      Args.push_back("-std=c++17");
+      break;
+    case Lang_CXX2a:
+      Args.push_back("-std=c++2a");
+      break;
+    case Lang_OBJC:
+      Args.push_back("-ObjC");
+      Args.push_back("-fobjc-runtime=macosx-10.12.0");
+      break;
+    case Lang_OBJCXX11:
+      Args.push_back("-ObjC++");
+      Args.push_back("-std=c++11");
+      Args.push_back("-fblocks");
+      break;
     }
     return tooling::runToolOnCodeWithArgs(CreateTestAction(), Code, Args);
   }
 
-  bool shouldVisitTemplateInstantiations() const {
-    return true;
-  }
-
-  bool shouldVisitImplicitCode() const {
-    return true;
-  }
-
 protected:
+  TestVisitorHelper() = default;
+  virtual ~TestVisitorHelper() = default;
+  virtual void InvokeTraverseDecl(TranslationUnitDecl *D) = 0;
+
   virtual std::unique_ptr<ASTFrontendAction> CreateTestAction() {
     return std::make_unique<TestAction>(this);
   }
 
   class FindConsumer : public ASTConsumer {
   public:
-    FindConsumer(TestVisitor *Visitor) : Visitor(Visitor) {}
+    FindConsumer(TestVisitorHelper *Visitor) : Visitor(Visitor) {}
 
     void HandleTranslationUnit(clang::ASTContext &Context) override {
       Visitor->Context = &Context;
-      Visitor->TraverseDecl(Context.getTranslationUnitDecl());
+      Visitor->InvokeTraverseDecl(Context.getTranslationUnitDecl());
     }
 
   private:
-    TestVisitor *Visitor;
+    TestVisitorHelper *Visitor;
   };
 
   class TestAction : public ASTFrontendAction {
   public:
-    TestAction(TestVisitor *Visitor) : Visitor(Visitor) {}
+    TestAction(TestVisitorHelper *Visitor) : Visitor(Visitor) {}
 
     std::unique_ptr<clang::ASTConsumer>
     CreateASTConsumer(CompilerInstance &, llvm::StringRef dummy) override {
@@ -113,20 +110,13 @@ class TestVisitor : public RecursiveASTVisitor<T> {
     }
 
   protected:
-    TestVisitor *Visitor;
+    TestVisitorHelper *Visitor;
   };
 
   ASTContext *Context;
 };
 
-/// \brief A RecursiveASTVisitor to check that certain matches are (or are
-/// not) observed during visitation.
-///
-/// This is a RecursiveASTVisitor for testing the RecursiveASTVisitor itself,
-/// and allows simple creation of test visitors running matches on only a small
-/// subset of the Visit* methods.
-template <typename T, template <typename> class Visitor = TestVisitor>
-class ExpectedLocationVisitor : public Visitor<T> {
+class ExpectedLocationVisitorHelper {
 public:
   /// \brief Expect 'Match' *not* to occur at the given 'Line' and 'Column'.
   ///
@@ -147,37 +137,44 @@ class ExpectedLocationVisitor : public Visitor<T> {
   }
 
   /// \brief Checks that all expected matches have been found.
-  ~ExpectedLocationVisitor() override {
-    for (typename std::vector<ExpectedMatch>::const_iterator
-             It = ExpectedMatches.begin(), End = ExpectedMatches.end();
+  virtual ~ExpectedLocationVisitorHelper() {
+    // FIXME: Range-based for loop.
+    for (std::vector<ExpectedMatch>::const_iterator
+             It = ExpectedMatches.begin(),
+             End = ExpectedMatches.end();
          It != End; ++It) {
       It->ExpectFound();
     }
   }
 
 protected:
+  virtual ASTContext *getASTContext() = 0;
+
   /// \brief Checks an actual match against expected and disallowed matches.
   ///
   /// Implementations are required to call this with appropriate values
   /// for 'Name' during visitation.
   void Match(StringRef Name, SourceLocation Location) {
-    const FullSourceLoc FullLocation = this->Context->getFullLoc(Location);
+    const FullSourceLoc FullLocation = getASTContext()->getFullLoc(Location);
 
-    for (typename std::vector<MatchCandidate>::const_iterator
-             It = DisallowedMatches.begin(), End = DisallowedMatches.end();
+    // FIXME: Range-based for loop.
+    for (std::vector<MatchCandidate>::const_iterator
+             It = DisallowedMatches.begin(),
+             End = DisallowedMatches.end();
          It != End; ++It) {
       EXPECT_FALSE(It->Matches(Name, FullLocation))
           << "Matched disallowed " << *It;
     }
 
-    for (typename std::vector<ExpectedMatch>::iterator
-             It = ExpectedMatches.begin(), End = ExpectedMatches.end();
+    // FIXME: Range-based for loop.
+    for (std::vector<ExpectedMatch>::iterator It = ExpectedMatches.begin(),
+                                              End = ExpectedMatches.end();
          It != End; ++It) {
-      It->UpdateFor(Name, FullLocation, this->Context->getSourceManager());
+      It->UpdateFor(Name, FullLocation, getASTContext()->getSourceManager());
     }
   }
 
- private:
+private:
   struct MatchCandidate {
     std::string ExpectedName;
     unsigned LineNumber;
@@ -247,6 +244,41 @@ class ExpectedLocationVisitor : public Visitor<T> {
   std::vector<MatchCandidate> DisallowedMatches;
   std::vector<ExpectedMatch> ExpectedMatches;
 };
-}
+} // namespace detail
+
+/// \brief Base class for simple (Dynamic)RecursiveASTVisitor based tests.
+///
+/// This is a drop-in replacement for DynamicRecursiveASTVisitor itself, with
+/// the additional capability of running it over a snippet of code.
+///
+/// Visits template instantiations and implicit code by default.
+///
+/// For post-order traversal etc. use CTRPTestVisitor from
+/// CTRPTestVisitor.h instead.
+class TestVisitor : public DynamicRecursiveASTVisitor,
+                    public detail::TestVisitorHelper {
+public:
+  TestVisitor() {
+    ShouldVisitTemplateInstantiations = true;
+    ShouldVisitImplicitCode = true;
+  }
+
+  void InvokeTraverseDecl(TranslationUnitDecl *D) override { TraverseDecl(D); }
+};
+
+/// \brief A RecursiveASTVisitor to check that certain matches are (or are
+/// not) observed during visitation.
+///
+/// This is a RecursiveASTVisitor for testing the RecursiveASTVisitor itself,
+/// and allows simple creation of test visitors running matches on only a small
+/// subset of the Visit* methods.
+///
+/// For post-order traversal etc. use CTRPExpectedLocationVisitor from
+/// CTRPTestVisitor.h instead.
+class ExpectedLocationVisitor : public TestVisitor,
+                                public detail::ExpectedLocationVisitorHelper {
+  ASTContext *getASTContext() override { return Context; }
+};
+} // namespace clang
 
 #endif



More information about the cfe-commits mailing list