[Clang] [NFC] Refactor AST visitors in Sema and the static analyser to use DynamicRecursiveASTVisitor (#115144)

This pr refactors all recursive AST visitors in `Sema`, `Analyze`, and
`StaticAnalysis` to inherit from DRAV instead. This is over half of the
visitors that inherit from RAV directly.

See also #115132, #110040, #93462

LLVM Compile-Time Tracker link for this branch:
https://llvm-compile-time-tracker.com/compare.php?from=5adb5c05a2e9f31385fbba8b0436cbc07d91a44d&to=b58e589a86c06ba28d4d90613864d10be29aa5ba&stat=instructions%3Au

Author: Sirraide

clang/include/clang/Analysis/CallGraph.h

@@ -18,7 +18,8 @@
 #define LLVM_CLANG_ANALYSIS_CALLGRAPH_H
 
 #include "clang/AST/Decl.h"
-#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/DeclObjC.h"
+#include "clang/AST/DynamicRecursiveASTVisitor.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/GraphTraits.h"
 #include "llvm/ADT/STLExtras.h"
@@ -40,7 +41,7 @@ class Stmt;
 /// The call graph extends itself with the given declarations by implementing
 /// the recursive AST visitor, which constructs the graph by visiting the given
 /// declarations.
-class CallGraph : public RecursiveASTVisitor<CallGraph> {
+class CallGraph : public DynamicRecursiveASTVisitor {
   friend class CallGraphNode;
 
   using FunctionMapTy =
@@ -116,7 +117,7 @@ class CallGraph : public RecursiveASTVisitor<CallGraph> {
 
   /// Part of recursive declaration visitation. We recursively visit all the
   /// declarations to collect the root functions.
-  bool VisitFunctionDecl(FunctionDecl *FD) {
+  bool VisitFunctionDecl(FunctionDecl *FD) override {
     // We skip function template definitions, as their semantics is
     // only determined when they are instantiated.
     if (includeInGraph(FD) && FD->isThisDeclarationADefinition()) {
@@ -131,7 +132,7 @@ class CallGraph : public RecursiveASTVisitor<CallGraph> {
   }
 
   /// Part of recursive declaration visitation.
-  bool VisitObjCMethodDecl(ObjCMethodDecl *MD) {
+  bool VisitObjCMethodDecl(ObjCMethodDecl *MD) override {
     if (includeInGraph(MD)) {
       addNodesForBlocks(MD);
       addNodeForDecl(MD, true);
@@ -140,11 +141,8 @@ class CallGraph : public RecursiveASTVisitor<CallGraph> {
   }
 
   // We are only collecting the declarations, so do not step into the bodies.
-  bool TraverseStmt(Stmt *S) { return true; }
+  bool TraverseStmt(Stmt *S) override { return true; }
 
-  bool shouldWalkTypesOfTypeLocs() const { return false; }
-  bool shouldVisitTemplateInstantiations() const { return true; }
-  bool shouldVisitImplicitCode() const { return true; }
   bool shouldSkipConstantExpressions() const { return ShouldSkipConstexpr; }
   void setSkipConstantExpressions(ASTContext &Context) {
     Ctx = &Context;

clang/include/clang/Analysis/FlowSensitive/ASTOps.h

@@ -14,8 +14,9 @@
 #define LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_ASTOPS_H
 
 #include "clang/AST/Decl.h"
+#include "clang/AST/DynamicRecursiveASTVisitor.h"
 #include "clang/AST/Expr.h"
-#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/ExprCXX.h"
 #include "clang/AST/Type.h"
 #include "clang/Analysis/FlowSensitive/StorageLocation.h"
 #include "llvm/ADT/DenseSet.h"
@@ -88,14 +89,14 @@ class RecordInitListHelper {
 /// the function to analyze. Don't call `TraverseDecl()` on the function itself;
 /// this won't work as `TraverseDecl()` contains code to avoid traversing nested
 /// functions.
-template <class Derived>
-class AnalysisASTVisitor : public RecursiveASTVisitor<Derived> {
+class AnalysisASTVisitor : public DynamicRecursiveASTVisitor {
 public:
-  bool shouldVisitImplicitCode() { return true; }
-
-  bool shouldVisitLambdaBody() const { return false; }
+  AnalysisASTVisitor() {
+    ShouldVisitImplicitCode = true;
+    ShouldVisitLambdaBody = false;
+  }
 
-  bool TraverseDecl(Decl *D) {
+  bool TraverseDecl(Decl *D) override {
     // Don't traverse nested record or function declarations.
     // - We won't be analyzing code contained in these anyway
     // - We don't model fields that are used only in these nested declaration,
@@ -104,30 +105,30 @@ class AnalysisASTVisitor : public RecursiveASTVisitor<Derived> {
     if (isa_and_nonnull<RecordDecl>(D) || isa_and_nonnull<FunctionDecl>(D))
       return true;
 
-    return RecursiveASTVisitor<Derived>::TraverseDecl(D);
+    return DynamicRecursiveASTVisitor::TraverseDecl(D);
   }
 
   // Don't traverse expressions in unevaluated contexts, as we don't model
   // fields that are only used in these.
   // Note: The operand of the `noexcept` operator is an unevaluated operand, but
   // nevertheless it appears in the Clang CFG, so we don't exclude it here.
-  bool TraverseDecltypeTypeLoc(DecltypeTypeLoc) { return true; }
-  bool TraverseTypeOfExprTypeLoc(TypeOfExprTypeLoc) { return true; }
-  bool TraverseCXXTypeidExpr(CXXTypeidExpr *TIE) {
+  bool TraverseDecltypeTypeLoc(DecltypeTypeLoc) override { return true; }
+  bool TraverseTypeOfExprTypeLoc(TypeOfExprTypeLoc) override { return true; }
+  bool TraverseCXXTypeidExpr(CXXTypeidExpr *TIE) override {
     if (TIE->isPotentiallyEvaluated())
-      return RecursiveASTVisitor<Derived>::TraverseCXXTypeidExpr(TIE);
+      return DynamicRecursiveASTVisitor::TraverseCXXTypeidExpr(TIE);
     return true;
   }
-  bool TraverseUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr *) {
+  bool TraverseUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr *) override {
     return true;
   }
 
-  bool TraverseBindingDecl(BindingDecl *BD) {
+  bool TraverseBindingDecl(BindingDecl *BD) override {
     // `RecursiveASTVisitor` doesn't traverse holding variables for
     // `BindingDecl`s by itself, so we need to tell it to.
     if (VarDecl *HoldingVar = BD->getHoldingVar())
       TraverseDecl(HoldingVar);
-    return RecursiveASTVisitor<Derived>::TraverseBindingDecl(BD);
+    return DynamicRecursiveASTVisitor::TraverseBindingDecl(BD);
   }
 };
 

clang/lib/Analysis/CallGraph.cpp

@@ -190,6 +190,9 @@ void CallGraph::addNodesForBlocks(DeclContext *D) {
 }
 
 CallGraph::CallGraph() {
+  ShouldWalkTypesOfTypeLocs = false;
+  ShouldVisitTemplateInstantiations = true;
+  ShouldVisitImplicitCode = true;
   Root = getOrInsertNode(nullptr);
 }
 

clang/lib/Analysis/CalledOnceCheck.cpp

@@ -11,11 +11,11 @@
 #include "clang/AST/Attr.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
+#include "clang/AST/DynamicRecursiveASTVisitor.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/OperationKinds.h"
 #include "clang/AST/ParentMap.h"
-#include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/AST/StmtVisitor.h"
@@ -426,7 +426,7 @@ const Expr *getCondition(const Stmt *S) {
 /// of the AST will end up in the results.
 /// Results might have duplicate names, if this is a problem, convert to
 /// string sets afterwards.
-class NamesCollector : public RecursiveASTVisitor<NamesCollector> {
+class NamesCollector : public DynamicRecursiveASTVisitor {
 public:
   static constexpr unsigned EXPECTED_NUMBER_OF_NAMES = 5;
   using NameCollection =
@@ -438,12 +438,12 @@ class NamesCollector : public RecursiveASTVisitor<NamesCollector> {
     return Impl.Result;
   }
 
-  bool VisitDeclRefExpr(const DeclRefExpr *E) {
+  bool VisitDeclRefExpr(DeclRefExpr *E) override {
     Result.push_back(E->getDecl()->getName());
     return true;
   }
 
-  bool VisitObjCPropertyRefExpr(const ObjCPropertyRefExpr *E) {
+  bool VisitObjCPropertyRefExpr(ObjCPropertyRefExpr *E) override {
     llvm::StringRef Name;
 
     if (E->isImplicitProperty()) {

clang/lib/Analysis/FlowSensitive/ASTOps.cpp

@@ -198,13 +198,12 @@ static MemberExpr *getMemberForAccessor(const CXXMemberCallExpr &C) {
   return nullptr;
 }
 
-class ReferencedDeclsVisitor
-    : public AnalysisASTVisitor<ReferencedDeclsVisitor> {
+class ReferencedDeclsVisitor : public AnalysisASTVisitor {
 public:
   ReferencedDeclsVisitor(ReferencedDecls &Referenced)
       : Referenced(Referenced) {}
 
-  void TraverseConstructorInits(const CXXConstructorDecl *Ctor) {
+  void traverseConstructorInits(const CXXConstructorDecl *Ctor) {
     for (const CXXCtorInitializer *Init : Ctor->inits()) {
       if (Init->isMemberInitializer()) {
         Referenced.Fields.insert(Init->getMember());
@@ -225,21 +224,21 @@ class ReferencedDeclsVisitor
     }
   }
 
-  bool VisitDecl(Decl *D) {
+  bool VisitDecl(Decl *D) override {
     insertIfGlobal(*D, Referenced.Globals);
     insertIfLocal(*D, Referenced.Locals);
     insertIfFunction(*D, Referenced.Functions);
     return true;
   }
 
-  bool VisitDeclRefExpr(DeclRefExpr *E) {
+  bool VisitDeclRefExpr(DeclRefExpr *E) override {
     insertIfGlobal(*E->getDecl(), Referenced.Globals);
     insertIfLocal(*E->getDecl(), Referenced.Locals);
     insertIfFunction(*E->getDecl(), Referenced.Functions);
     return true;
   }
 
-  bool VisitCXXMemberCallExpr(CXXMemberCallExpr *C) {
+  bool VisitCXXMemberCallExpr(CXXMemberCallExpr *C) override {
     // If this is a method that returns a member variable but does nothing else,
     // model the field of the return value.
     if (MemberExpr *E = getMemberForAccessor(*C))
@@ -248,7 +247,7 @@ class ReferencedDeclsVisitor
     return true;
   }
 
-  bool VisitMemberExpr(MemberExpr *E) {
+  bool VisitMemberExpr(MemberExpr *E) override {
     // FIXME: should we be using `E->getFoundDecl()`?
     const ValueDecl *VD = E->getMemberDecl();
     insertIfGlobal(*VD, Referenced.Globals);
@@ -258,14 +257,14 @@ class ReferencedDeclsVisitor
     return true;
   }
 
-  bool VisitInitListExpr(InitListExpr *InitList) {
+  bool VisitInitListExpr(InitListExpr *InitList) override {
     if (InitList->getType()->isRecordType())
       for (const auto *FD : getFieldsForInitListExpr(InitList))
         Referenced.Fields.insert(FD);
     return true;
   }
 
-  bool VisitCXXParenListInitExpr(CXXParenListInitExpr *ParenInitList) {
+  bool VisitCXXParenListInitExpr(CXXParenListInitExpr *ParenInitList) override {
     if (ParenInitList->getType()->isRecordType())
       for (const auto *FD : getFieldsForInitListExpr(ParenInitList))
         Referenced.Fields.insert(FD);
@@ -281,7 +280,7 @@ ReferencedDecls getReferencedDecls(const FunctionDecl &FD) {
   ReferencedDeclsVisitor Visitor(Result);
   Visitor.TraverseStmt(FD.getBody());
   if (const auto *CtorDecl = dyn_cast<CXXConstructorDecl>(&FD))
-    Visitor.TraverseConstructorInits(CtorDecl);
+    Visitor.traverseConstructorInits(CtorDecl);
 
   return Result;
 }

clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

@@ -298,7 +298,7 @@ namespace {
 // Visitor that builds a map from record prvalues to result objects.
 // For each result object that it encounters, it propagates the storage location
 // of the result object to all record prvalues that can initialize it.
-class ResultObjectVisitor : public AnalysisASTVisitor<ResultObjectVisitor> {
+class ResultObjectVisitor : public AnalysisASTVisitor {
 public:
   // `ResultObjectMap` will be filled with a map from record prvalues to result
   // object. If this visitor will traverse a function that returns a record by
@@ -315,7 +315,7 @@ class ResultObjectVisitor : public AnalysisASTVisitor<ResultObjectVisitor> {
   // called by `RecursiveASTVisitor`; it should be called manually if we are
   // analyzing a constructor. `ThisPointeeLoc` is the storage location that
   // `this` points to.
-  void TraverseConstructorInits(const CXXConstructorDecl *Ctor,
+  void traverseConstructorInits(const CXXConstructorDecl *Ctor,
                                 RecordStorageLocation *ThisPointeeLoc) {
     assert(ThisPointeeLoc != nullptr);
     for (const CXXCtorInitializer *Init : Ctor->inits()) {
@@ -339,31 +339,31 @@ class ResultObjectVisitor : public AnalysisASTVisitor<ResultObjectVisitor> {
     }
   }
 
-  bool VisitVarDecl(VarDecl *VD) {
+  bool VisitVarDecl(VarDecl *VD) override {
     if (VD->getType()->isRecordType() && VD->hasInit())
       PropagateResultObject(
           VD->getInit(),
           &cast<RecordStorageLocation>(DACtx.getStableStorageLocation(*VD)));
     return true;
   }
 
-  bool VisitMaterializeTemporaryExpr(MaterializeTemporaryExpr *MTE) {
+  bool VisitMaterializeTemporaryExpr(MaterializeTemporaryExpr *MTE) override {
     if (MTE->getType()->isRecordType())
       PropagateResultObject(
           MTE->getSubExpr(),
           &cast<RecordStorageLocation>(DACtx.getStableStorageLocation(*MTE)));
     return true;
   }
 
-  bool VisitReturnStmt(ReturnStmt *Return) {
+  bool VisitReturnStmt(ReturnStmt *Return) override {
     Expr *RetValue = Return->getRetValue();
     if (RetValue != nullptr && RetValue->getType()->isRecordType() &&
         RetValue->isPRValue())
       PropagateResultObject(RetValue, LocForRecordReturnVal);
     return true;
   }
 
-  bool VisitExpr(Expr *E) {
+  bool VisitExpr(Expr *E) override {
     // Clang's AST can have record-type prvalues without a result object -- for
     // example as full-expressions contained in a compound statement or as
     // arguments of call expressions. We notice this if we get here and a
@@ -1211,7 +1211,7 @@ Environment::PrValueToResultObject Environment::buildResultObjectMap(
 
   ResultObjectVisitor Visitor(Map, LocForRecordReturnVal, *DACtx);
   if (const auto *Ctor = dyn_cast<CXXConstructorDecl>(FuncDecl))
-    Visitor.TraverseConstructorInits(Ctor, ThisPointeeLoc);
+    Visitor.traverseConstructorInits(Ctor, ThisPointeeLoc);
 
   return Map;
 }

clang/lib/Analysis/ReachableCode.cpp

@@ -13,11 +13,11 @@
 
 #include "clang/Analysis/Analyses/ReachableCode.h"
 #include "clang/AST/Attr.h"
+#include "clang/AST/DynamicRecursiveASTVisitor.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/ParentMap.h"
-#include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/StmtCXX.h"
 #include "clang/Analysis/AnalysisDeclContext.h"
 #include "clang/Analysis/CFG.h"
@@ -476,17 +476,19 @@ static bool isInCoroutineStmt(const Stmt *DeadStmt, const CFGBlock *Block) {
     }
   if (!CoroStmt)
     return false;
-  struct Checker : RecursiveASTVisitor<Checker> {
+  struct Checker : DynamicRecursiveASTVisitor {
     const Stmt *DeadStmt;
     bool CoroutineSubStmt = false;
-    Checker(const Stmt *S) : DeadStmt(S) {}
-    bool VisitStmt(const Stmt *S) {
+    Checker(const Stmt *S) : DeadStmt(S) {
+      // Statements captured in the CFG can be implicit.
+      ShouldVisitImplicitCode = true;
+    }
+
+    bool VisitStmt(Stmt *S) override {
       if (S == DeadStmt)
         CoroutineSubStmt = true;
       return true;
     }
-    // Statements captured in the CFG can be implicit.
-    bool shouldVisitImplicitCode() const { return true; }
   };
   Checker checker(DeadStmt);
   checker.TraverseStmt(const_cast<Stmt *>(CoroStmt));