Use new caller interface for runDataflowAnalysis

clang-tools-extra/clang-tidy/bugprone/NullCheckAfterDereferenceCheck.cpp

@@ -31,16 +31,16 @@ namespace clang::tidy::bugprone {
 using ast_matchers::MatchFinder;
 using dataflow::NullCheckAfterDereferenceDiagnoser;
 using dataflow::NullPointerAnalysisModel;
+using Diagnoser = NullCheckAfterDereferenceDiagnoser;
 
 static constexpr llvm::StringLiteral FuncID("fun");
 
 struct ExpandedResult {
-  SourceLocation WarningLoc;
+  Diagnoser::DiagnosticEntry Entry;
   std::optional<SourceLocation> DerefLoc;
 };
 
-using ExpandedResultType =
-    std::pair<std::vector<ExpandedResult>, std::vector<ExpandedResult>>;
+using ExpandedResultType = llvm::SmallVector<ExpandedResult>;
 
 static std::optional<ExpandedResultType>
 analyzeFunction(const FunctionDecl &FuncDecl) {
@@ -63,51 +63,30 @@ analyzeFunction(const FunctionDecl &FuncDecl) {
       std::make_unique<dataflow::WatchedLiteralsSolver>());
   dataflow::Environment Env(AnalysisContext, FuncDecl);
   NullPointerAnalysisModel Analysis(ASTCtx);
-  NullCheckAfterDereferenceDiagnoser Diagnoser;
-  NullCheckAfterDereferenceDiagnoser::ResultType Diagnostics;
+  Diagnoser Diagnoser;
 
-  using State = DataflowAnalysisState<NullPointerAnalysisModel::Lattice>;
-  using DetailMaybeStates = std::vector<std::optional<State>>;
+  Expected<Diagnoser::ResultType> Diagnostics =
+      dataflow::diagnoseFunction<NullPointerAnalysisModel, Diagnoser::DiagnosticEntry>(
+      FuncDecl, ASTCtx, Diagnoser);
 
-  auto DiagnoserImpl = [&ASTCtx, &Diagnoser,
-                        &Diagnostics](const CFGElement &Elt,
-                                      const State &S) mutable -> void {
-    auto EltDiagnostics = Diagnoser.diagnose(ASTCtx, &Elt, S.Env);
-    llvm::move(EltDiagnostics.first, std::back_inserter(Diagnostics.first));
-    llvm::move(EltDiagnostics.second, std::back_inserter(Diagnostics.second)); 
-  };
-
-  Expected<DetailMaybeStates> BlockToOutputState =
-      dataflow::runDataflowAnalysis(*Context, Analysis, Env, DiagnoserImpl);
-
-  if (llvm::Error E = BlockToOutputState.takeError()) {
+
+  if (llvm::Error E = Diagnostics.takeError()) {
     llvm::dbgs() << "Dataflow analysis failed: " << llvm::toString(std::move(E))
                  << ".\n";
     return std::nullopt;
   }
 
   ExpandedResultType ExpandedDiagnostics;
 
-  llvm::transform(Diagnostics.first,
-                  std::back_inserter(ExpandedDiagnostics.first),
-                  [&](SourceLocation WarningLoc) -> ExpandedResult {
-                    if (auto Val = Diagnoser.WarningLocToVal[WarningLoc];
-                        auto DerefExpr = Diagnoser.ValToDerefLoc[Val]) {
-                      return {WarningLoc, DerefExpr->getBeginLoc()};
-                    }
-
-                    return {WarningLoc, std::nullopt};
-                  });
-
-  llvm::transform(Diagnostics.second,
-                  std::back_inserter(ExpandedDiagnostics.second),
-                  [&](SourceLocation WarningLoc) -> ExpandedResult {
-                    if (auto Val = Diagnoser.WarningLocToVal[WarningLoc];
+  llvm::transform(*Diagnostics,
+                  std::back_inserter(ExpandedDiagnostics),
+                  [&](Diagnoser::DiagnosticEntry Entry) -> ExpandedResult {
+                    if (auto Val = Diagnoser.WarningLocToVal[Entry.Location];
                         auto DerefExpr = Diagnoser.ValToDerefLoc[Val]) {
-                      return {WarningLoc, DerefExpr->getBeginLoc()};
+                      return {Entry, DerefExpr->getBeginLoc()};
                     }
 
-                    return {WarningLoc, std::nullopt};
+                    return {Entry, std::nullopt};
                   });
 
   return ExpandedDiagnostics;
@@ -143,28 +122,30 @@ void NullCheckAfterDereferenceCheck::check(
     return;
 
   if (const auto Diagnostics = analyzeFunction(*FuncDecl)) {
-    const auto &[CheckWhenNullLocations, CheckWhenNonnullLocations] =
-        *Diagnostics;
-
-    for (const auto [WarningLoc, DerefLoc] : CheckWhenNonnullLocations) {
-      diag(WarningLoc, "pointer value is checked even though "
-                       "it cannot be null at this point");
-
-      if (DerefLoc) {
-        diag(*DerefLoc,
-             "one of the locations where the pointer's value cannot be null",
-             DiagnosticIDs::Note);
-      }
-    }
-
-    for (const auto [WarningLoc, DerefLoc] : CheckWhenNullLocations) {
-      diag(WarningLoc,
-           "pointer value is checked but it can only be null at this point");
-
-      if (DerefLoc) {
-        diag(*DerefLoc,
-             "one of the locations where the pointer's value can only be null",
-             DiagnosticIDs::Note);
+    for (const auto [Entry, DerefLoc] : *Diagnostics) {
+      const auto [WarningLoc, Type] = Entry;
+
+      switch (Type) {
+      case Diagnoser::DiagnosticType::CheckAfterDeref:
+        diag(WarningLoc, "pointer value is checked even though "
+                         "it cannot be null at this point");
+
+        if (DerefLoc) {
+          diag(*DerefLoc,
+               "one of the locations where the pointer's value cannot be null",
+               DiagnosticIDs::Note);
+        }
+        break;
+      case Diagnoser::DiagnosticType::CheckWhenNull:
+        diag(WarningLoc,
+             "pointer value is checked but it can only be null at this point");
+
+        if (DerefLoc) {
+          diag(*DerefLoc,
+               "one of the locations where the pointer's value can only be null",
+               DiagnosticIDs::Note);
+        }
+        break;
       }
     }
   }

clang/include/clang/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.h

@@ -81,8 +81,13 @@ class NullCheckAfterDereferenceDiagnoser {
 
   /// Returns source locations for pointers that were checked when known to be
   // null, and checked after already dereferenced, respectively.
-  using ResultType =
-      std::pair<std::vector<SourceLocation>, std::vector<SourceLocation>>;
+  enum class DiagnosticType { CheckWhenNull, CheckAfterDeref };
+
+  struct DiagnosticEntry {
+    SourceLocation Location;
+    DiagnosticType Type;
+  };
+  using ResultType = llvm::SmallVector<DiagnosticEntry>;
 
   // Maps a pointer's Value to a dereference, null-assignment, etc.
   // This is used later to construct the Note tag.
@@ -96,8 +101,8 @@ class NullCheckAfterDereferenceDiagnoser {
 public:
   NullCheckAfterDereferenceDiagnoser();
 
-  ResultType diagnose(ASTContext &Ctx, const CFGElement *Elt,
-                      const Environment &Env);
+  ResultType operator()(const CFGElement &Elt, ASTContext &Ctx,
+                        const TransferStateForDiagnostics<NoopLattice> &State);
 };
 
 } // namespace clang::dataflow

clang/lib/Analysis/FlowSensitive/Models/NullPointerAnalysisModel.cpp

@@ -36,6 +36,7 @@ namespace clang::dataflow {
 
 namespace {
 using namespace ast_matchers;
+using Diagnoser = NullCheckAfterDereferenceDiagnoser;
 
 constexpr char kCond[] = "condition";
 constexpr char kVar[] = "var";
@@ -423,9 +424,9 @@ void matchAnyPointerExpr(const Expr *fncall,
   setUnknownValue(*Var, *RootValue, Env);
 }
 
-NullCheckAfterDereferenceDiagnoser::ResultType
+Diagnoser::ResultType
 diagnoseDerefLocation(const Expr *Deref, const MatchFinder::MatchResult &Result,
-                      NullCheckAfterDereferenceDiagnoser::DiagnoseArgs &Data) {
+                      Diagnoser::DiagnoseArgs &Data) {
   auto [ValToDerefLoc, WarningLocToVal, Env] = Data;
 
   const auto *Var = Result.Nodes.getNodeAs<Expr>(kVar);
@@ -444,10 +445,9 @@ diagnoseDerefLocation(const Expr *Deref, const MatchFinder::MatchResult &Result,
   return {};
 }
 
-NullCheckAfterDereferenceDiagnoser::ResultType
-diagnoseAssignLocation(const Expr *Assign,
-                       const MatchFinder::MatchResult &Result,
-                       NullCheckAfterDereferenceDiagnoser::DiagnoseArgs &Data) {
+Diagnoser::ResultType diagnoseAssignLocation(const Expr *Assign,
+                                             const MatchFinder::MatchResult &Result,
+                                             Diagnoser::DiagnoseArgs &Data) {
   auto [ValToDerefLoc, WarningLocToVal, Env] = Data;
 
   const auto *RHSVar = Result.Nodes.getNodeAs<Expr>(kValue);
@@ -468,7 +468,7 @@ diagnoseAssignLocation(const Expr *Assign,
 NullCheckAfterDereferenceDiagnoser::ResultType
 diagnoseNullCheckExpr(const Expr *NullCheck,
       const MatchFinder::MatchResult &Result,
-      const NullCheckAfterDereferenceDiagnoser::DiagnoseArgs &Data) {
+      const Diagnoser::DiagnoseArgs &Data) {
   auto [ValToDerefLoc, WarningLocToVal, Env] = Data;
 
   const auto *Var = Result.Nodes.getNodeAs<Expr>(kVar);
@@ -488,7 +488,7 @@ diagnoseNullCheckExpr(const Expr *NullCheck,
         assert(Inserted && "multiple warnings at the same source location");
         (void)Inserted;
 
-        return {{}, {Var->getBeginLoc()}};
+        return {{Var->getBeginLoc(), Diagnoser::DiagnosticType::CheckAfterDeref}};
       }
 
       if (IsNull && !IsNonnull) {
@@ -497,7 +497,7 @@ diagnoseNullCheckExpr(const Expr *NullCheck,
         assert(Inserted && "multiple warnings at the same source location");
         (void)Inserted;
 
-        return {{Var->getBeginLoc()}, {}};
+        return {{Var->getBeginLoc(), Diagnoser::DiagnosticType::CheckWhenNull}};
       }
     }
 
@@ -513,7 +513,7 @@ diagnoseNullCheckExpr(const Expr *NullCheck,
 
 NullCheckAfterDereferenceDiagnoser::ResultType
 diagnoseEqualExpr(const Expr *PtrCheck, const MatchFinder::MatchResult &Result,
-                  NullCheckAfterDereferenceDiagnoser::DiagnoseArgs &Data) {
+                  Diagnoser::DiagnoseArgs &Data) {
   auto [ValToDerefLoc, WarningLocToVal, Env] = Data;
 
   const auto *LHSVar = Result.Nodes.getNodeAs<Expr>(kVar);
@@ -522,23 +522,23 @@ diagnoseEqualExpr(const Expr *PtrCheck, const MatchFinder::MatchResult &Result,
   assert(RHSVar != nullptr);
 
   Arena &A = Env.arena();
-  std::vector<SourceLocation> NullVarLocations;
+  llvm::SmallVector<Diagnoser::DiagnosticEntry> NullVarLocations;
 
   if (Value *LHSValue = Env.getValue(*LHSVar);
       LHSValue->getProperty(kIsNonnull) && 
       Env.proves(A.makeNot(getVal(kIsNonnull, *LHSValue).formula()))) {
     WarningLocToVal.try_emplace(LHSVar->getBeginLoc(), LHSValue);
-    NullVarLocations.push_back(LHSVar->getBeginLoc());
+    NullVarLocations.push_back({LHSVar->getBeginLoc(), Diagnoser::DiagnosticType::CheckWhenNull});
   }
 
   if (Value *RHSValue = Env.getValue(*RHSVar);
       RHSValue->getProperty(kIsNonnull) && 
       Env.proves(A.makeNot(getVal(kIsNonnull, *RHSValue).formula()))) {
     WarningLocToVal.try_emplace(RHSVar->getBeginLoc(), RHSValue);
-    NullVarLocations.push_back(RHSVar->getBeginLoc());
+    NullVarLocations.push_back({RHSVar->getBeginLoc(), Diagnoser::DiagnosticType::CheckWhenNull});
   }
 
-  return {NullVarLocations, {}};
+  return NullVarLocations;
 }
 
 auto buildTransferMatchSwitch() {
@@ -556,8 +556,7 @@ auto buildTransferMatchSwitch() {
 }
 
 auto buildDiagnoseMatchSwitch() {
-  return CFGMatchSwitchBuilder<NullCheckAfterDereferenceDiagnoser::DiagnoseArgs,
-                               NullCheckAfterDereferenceDiagnoser::ResultType>()
+  return CFGMatchSwitchBuilder<Diagnoser::DiagnoseArgs, Diagnoser::ResultType>()
       .CaseOfCFGStmt<Expr>(derefMatcher(), diagnoseDerefLocation)
       .CaseOfCFGStmt<Expr>(arrowMatcher(), diagnoseDerefLocation)
       .CaseOfCFGStmt<Expr>(assignMatcher(), diagnoseAssignLocation)
@@ -761,11 +760,11 @@ NullCheckAfterDereferenceDiagnoser::NullCheckAfterDereferenceDiagnoser()
     : DiagnoseMatchSwitch(buildDiagnoseMatchSwitch()) {}
 
 NullCheckAfterDereferenceDiagnoser::ResultType
-NullCheckAfterDereferenceDiagnoser::diagnose(ASTContext &Ctx,
-                                             const CFGElement *Elt,
-                                             const Environment &Env) {
-  DiagnoseArgs Args = {ValToDerefLoc, WarningLocToVal, Env};
-  return DiagnoseMatchSwitch(*Elt, Ctx, Args);
+NullCheckAfterDereferenceDiagnoser::operator()(
+    const CFGElement &Elt, ASTContext &Ctx, 
+    const TransferStateForDiagnostics<NoopLattice> &State) {
+  DiagnoseArgs Args = {ValToDerefLoc, WarningLocToVal, State.Env};
+  return DiagnoseMatchSwitch(Elt, Ctx, Args);
 }
 
 } // namespace clang::dataflow