[clang-tidy] Update unchecked-optiona-access-check to use convenience…

… function for diagnosing `FunctionDecl`s. Also changes code in the underlying model to fit the type expected by the convenience function. Differential Revision: https://reviews.llvm.org/D156255
llvm · Jul 26, 2023 · e9570d1 · e9570d1
1 parent 1429240
commit e9570d1
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 54 deletions.
diff --git a/clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp
@@ -8,17 +8,11 @@
 
 #include "UncheckedOptionalAccessCheck.h"
 #include "clang/AST/ASTContext.h"
-#include "clang/AST/DeclCXX.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
-#include "clang/Analysis/CFG.h"
-#include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
-#include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
-#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
 #include "clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h"
-#include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h"
 #include "clang/Basic/SourceLocation.h"
-#include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Error.h"
 #include <memory>
 #include <optional>
@@ -32,41 +26,6 @@ using dataflow::UncheckedOptionalAccessModelOptions;
 
 static constexpr llvm::StringLiteral FuncID("fun");
 
-static std::optional<std::vector<SourceLocation>>
-analyzeFunction(const FunctionDecl &FuncDecl, ASTContext &ASTCtx,
-                UncheckedOptionalAccessModelOptions ModelOptions) {
-  using dataflow::ControlFlowContext;
-  using dataflow::DataflowAnalysisState;
-  using llvm::Expected;
-
-  Expected<ControlFlowContext> Context =
-      ControlFlowContext::build(FuncDecl, *FuncDecl.getBody(), ASTCtx);
-  if (!Context)
-    return std::nullopt;
-
-  dataflow::DataflowAnalysisContext AnalysisContext(
-      std::make_unique<dataflow::WatchedLiteralsSolver>());
-  dataflow::Environment Env(AnalysisContext, FuncDecl);
-  UncheckedOptionalAccessModel Analysis(ASTCtx);
-  UncheckedOptionalAccessDiagnoser Diagnoser(ModelOptions);
-  std::vector<SourceLocation> Diagnostics;
-  Expected<std::vector<std::optional<
-      DataflowAnalysisState<UncheckedOptionalAccessModel::Lattice>>>>
-      BlockToOutputState = dataflow::runDataflowAnalysis(
-          *Context, Analysis, Env,
-          [&ASTCtx, &Diagnoser, &Diagnostics](
-              const CFGElement &Elt,
-              const DataflowAnalysisState<UncheckedOptionalAccessModel::Lattice>
-                  &State) mutable {
-            auto EltDiagnostics = Diagnoser.diagnose(ASTCtx, &Elt, State.Env);
-            llvm::move(EltDiagnostics, std::back_inserter(Diagnostics));
-          });
-  if (!BlockToOutputState)
-    return std::nullopt;
-
-  return Diagnostics;
-}
-
 void UncheckedOptionalAccessCheck::registerMatchers(MatchFinder *Finder) {
   using namespace ast_matchers;
 
@@ -93,10 +52,17 @@ void UncheckedOptionalAccessCheck::check(
   if (FuncDecl->isTemplated())
     return;
 
-  if (std::optional<std::vector<SourceLocation>> Errors =
-          analyzeFunction(*FuncDecl, *Result.Context, ModelOptions))
-    for (const SourceLocation &Loc : *Errors)
+  UncheckedOptionalAccessDiagnoser Diagnoser(ModelOptions);
+  // FIXME: Allow user to set the (defaulted) SAT iterations max for
+  // `diagnoseFunction` with config options.
+  if (llvm::Expected<std::vector<SourceLocation>> Locs =
+          dataflow::diagnoseFunction<UncheckedOptionalAccessModel,
+                                     SourceLocation>(*FuncDecl, *Result.Context,
+                                                     Diagnoser))
+    for (const SourceLocation &Loc : *Locs)
       diag(Loc, "unchecked access to optional value");
+  else
+    llvm::consumeError(Locs.takeError());
 }
 
 } // namespace clang::tidy::bugprone
diff --git a/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h b/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
@@ -74,8 +74,11 @@ class UncheckedOptionalAccessDiagnoser {
   UncheckedOptionalAccessDiagnoser(
       UncheckedOptionalAccessModelOptions Options = {});
 
-  std::vector<SourceLocation> diagnose(ASTContext &Ctx, const CFGElement *Elt,
-                                       const Environment &Env);
+  std::vector<SourceLocation>
+  operator()(const CFGElement &Elt, ASTContext &Ctx,
+             const TransferStateForDiagnostics<NoopLattice> &State) {
+    return DiagnoseMatchSwitch(Elt, Ctx, State.Env);
+  }
 
 private:
   CFGMatchSwitch<const Environment, std::vector<SourceLocation>>

diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -1048,10 +1048,5 @@ UncheckedOptionalAccessDiagnoser::UncheckedOptionalAccessDiagnoser(
     UncheckedOptionalAccessModelOptions Options)
     : DiagnoseMatchSwitch(buildDiagnoseMatchSwitch(Options)) {}
 
-std::vector<SourceLocation> UncheckedOptionalAccessDiagnoser::diagnose(
-    ASTContext &Ctx, const CFGElement *Elt, const Environment &Env) {
-  return DiagnoseMatchSwitch(*Elt, Ctx, Env);
-}
-
 } // namespace dataflow
 } // namespace clang
diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -1322,8 +1322,7 @@ class UncheckedOptionalAccessTest
                     ASTContext &Ctx, const CFGElement &Elt,
                     const TransferStateForDiagnostics<NoopLattice>
                         &State) mutable {
-                  auto EltDiagnostics =
-                      Diagnoser.diagnose(Ctx, &Elt, State.Env);
+                  auto EltDiagnostics = Diagnoser(Elt, Ctx, State);
                   llvm::move(EltDiagnostics, std::back_inserter(Diagnostics));
                 })
             .withASTBuildArgs(