[clang][dataflow] Rewrite `getReferencedDecls()` with a `RecursiveASTVisitor`. (#93461)

We previously had a hand-rolled recursive traversal here that was
exactly what
`RecursiveASTVistor` does anyway. Using the visitor not only eliminates
the
explicit traversal logic but also allows us to introduce a common
visitor base
class for `getReferencedDecls()` and `ResultObjectVisitor`, ensuring
that the
two are consistent in terms of the nodes they visit. Inconsistency
between these
two has caused crashes in the past when `ResultObjectVisitor` tried to
propagate
result object locations to entities that weren't modeled becasue
`getReferencedDecls()` didn't visit them.

1 files changed, 1 insertions, 36 deletions

diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 338a855..0d7967c 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -297,7 +297,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 RecursiveASTVisitor<ResultObjectVisitor> {
+class ResultObjectVisitor : public AnalysisASTVisitor<ResultObjectVisitor> {
 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
@@ -310,10 +310,6 @@ public:
       : ResultObjectMap(ResultObjectMap),
         LocForRecordReturnVal(LocForRecordReturnVal), DACtx(DACtx) {}
 
-  bool shouldVisitImplicitCode() { return true; }
-
-  bool shouldVisitLambdaBody() const { return false; }
-
   // Traverse all member and base initializers of `Ctor`. This function is not
   // called by `RecursiveASTVisitor`; it should be called manually if we are
   // analyzing a constructor. `ThisPointeeLoc` is the storage location that
@@ -342,37 +338,6 @@ public:
     }
   }
 
-  bool TraverseDecl(Decl *D) {
-    // 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,
-    //   so trying to propagate a result object to initializers of such fields
-    //   would cause an error.
-    if (isa_and_nonnull<RecordDecl>(D) || isa_and_nonnull<FunctionDecl>(D))
-      return true;
-
-    return RecursiveASTVisitor<ResultObjectVisitor>::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 *) { return true; }
-  bool TraverseUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr *) {
-    return true;
-  }
-
-  bool TraverseBindingDecl(BindingDecl *BD) {
-    // `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<ResultObjectVisitor>::TraverseBindingDecl(BD);
-  }
-
   bool VisitVarDecl(VarDecl *VD) {
     if (VD->getType()->isRecordType() && VD->hasInit())
       PropagateResultObject(