[PATCH] D156255: [clang-tidy] Update unchecked-optiona-access-check to use convenience function for diagnosing `FunctionDecl`s.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel created this revision.
ymandel added reviewers: xazax.hun, mboehme.
Herald added subscribers: PiotrZSL, ChuanqiXu, carlosgalvezp.
Herald added a reviewer: NoQ.
Herald added a reviewer: njames93.
Herald added a project: All.
ymandel requested review of this revision.
Herald added projects: clang, clang-tools-extra.

Also changes code in the underlying model to fit the type expected by the 
convenience function.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156255

Files:
  clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp
  
clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
  clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
  clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp


Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -1323,7 +1323,7 @@
                     const TransferStateForDiagnostics<NoopLattice>
                         &State) mutable {
                   auto EltDiagnostics =
-                      Diagnoser.diagnose(Ctx, &Elt, State.Env);
+                      Diagnoser(Elt, Ctx, State);
                   llvm::move(EltDiagnostics, std::back_inserter(Diagnostics));
                 })
             .withASTBuildArgs(
Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -1042,10 +1042,5 @@
     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
Index: clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
+++ clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
@@ -74,8 +74,11 @@
   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>>
Index: clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp
+++ 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 @@
 
 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 @@
   if (FuncDecl->isTemplated())
     return;
 
-  if (std::optional<std::vector<SourceLocation>> Errors =
-          analyzeFunction(*FuncDecl, *Result.Context, ModelOptions))
+  UncheckedOptionalAccessDiagnoser Diagnoser(ModelOptions);
+  // FIXME: Allow user to set the (defaulted) SAT iterations max for
+  // `diagnoseFunction` with config options.
+  if (llvm::Expected<std::vector<SourceLocation>> Errors =
+          dataflow::diagnoseFunction<UncheckedOptionalAccessModel,
+                                     SourceLocation>(*FuncDecl, *Result.Context,
+                                                     Diagnoser))
     for (const SourceLocation &Loc : *Errors)
       diag(Loc, "unchecked access to optional value");
+  else
+    llvm::consumeError(Errors.takeError());
 }
 
 } // namespace clang::tidy::bugprone

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits