[clang-tools-extra] 2a33d12 - Revert "[clang][dataflow] Use diagnosis API in optional checker"

[Sam Estep via cfe-commits]

Author: Sam Estep
Date: 2022-06-29T19:34:44Z
New Revision: 2a33d12642d862a8aa307e4a8b8a94d2d0c5ad1d

URL: https://github.com/llvm/llvm-project/commit/2a33d12642d862a8aa307e4a8b8a94d2d0c5ad1d
DIFF: https://github.com/llvm/llvm-project/commit/2a33d12642d862a8aa307e4a8b8a94d2d0c5ad1d.diff

LOG: Revert "[clang][dataflow] Use diagnosis API in optional checker"

This reverts commit cafb8b4ff2c38f81e65f97193eb1d8d16c581522.

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp
    clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp
index 9766450e81654..07fb19ff81ac1 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp
@@ -23,7 +23,6 @@
 #include "clang/Basic/SourceLocation.h"
 #include "llvm/ADT/Any.h"
 #include "llvm/ADT/Optional.h"
-#include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Error.h"
 #include <memory>
 #include <vector>
@@ -33,13 +32,12 @@ namespace tidy {
 namespace bugprone {
 using ast_matchers::MatchFinder;
 using dataflow::SourceLocationsLattice;
-using dataflow::UncheckedOptionalAccessDiagnoser;
 using dataflow::UncheckedOptionalAccessModel;
 using llvm::Optional;
 
 static constexpr llvm::StringLiteral FuncID("fun");
 
-static Optional<std::vector<SourceLocation>>
+static Optional<SourceLocationsLattice>
 analyzeFunction(const FunctionDecl &FuncDecl, ASTContext &ASTCtx) {
   using dataflow::ControlFlowContext;
   using dataflow::DataflowAnalysisState;
@@ -54,22 +52,23 @@ analyzeFunction(const FunctionDecl &FuncDecl, ASTContext &ASTCtx) {
       std::make_unique<dataflow::WatchedLiteralsSolver>());
   dataflow::Environment Env(AnalysisContext, FuncDecl);
   UncheckedOptionalAccessModel Analysis(ASTCtx);
-  UncheckedOptionalAccessDiagnoser Diagnoser;
-  std::vector<SourceLocation> Diagnostics;
   Expected<std::vector<Optional<DataflowAnalysisState<SourceLocationsLattice>>>>
-      BlockToOutputState = dataflow::runDataflowAnalysis(
-          *Context, Analysis, Env,
-          [&ASTCtx, &Diagnoser,
-           &Diagnostics](const Stmt *Stmt,
-                         const DataflowAnalysisState<SourceLocationsLattice>
-                             &State) mutable {
-            auto StmtDiagnostics = Diagnoser.diagnose(ASTCtx, Stmt, State.Env);
-            llvm::move(StmtDiagnostics, std::back_inserter(Diagnostics));
-          });
+      BlockToOutputState =
+          dataflow::runDataflowAnalysis(*Context, Analysis, Env);
   if (!BlockToOutputState)
     return llvm::None;
+  assert(Context->getCFG().getExit().getBlockID() < BlockToOutputState->size());
 
-  return Diagnostics;
+  const Optional<DataflowAnalysisState<SourceLocationsLattice>>
+      &ExitBlockState =
+          (*BlockToOutputState)[Context->getCFG().getExit().getBlockID()];
+  // `runDataflowAnalysis` doesn't guarantee that the exit block is visited;
+  // for example, when it is unreachable.
+  // FIXME: Diagnose violations even when the exit block is unreachable.
+  if (!ExitBlockState)
+    return llvm::None;
+
+  return std::move(ExitBlockState->Lattice);
 }
 
 void UncheckedOptionalAccessCheck::registerMatchers(MatchFinder *Finder) {
@@ -98,9 +97,9 @@ void UncheckedOptionalAccessCheck::check(
   if (FuncDecl->isTemplated())
     return;
 
-  if (Optional<std::vector<SourceLocation>> Errors =
+  if (Optional<SourceLocationsLattice> Errors =
           analyzeFunction(*FuncDecl, *Result.Context))
-    for (const SourceLocation &Loc : *Errors)
+    for (const SourceLocation &Loc : Errors->getSourceLocations())
       diag(Loc, "unchecked access to optional value");
 }
 

diff  --git a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
index 40ac95b3abddb..f4d6a221abecc 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
@@ -109,33 +109,14 @@ template <typename LatticeT> struct DataflowAnalysisState {
 /// dataflow analysis states that model the respective basic blocks. The
 /// returned vector, if any, will have the same size as the number of CFG
 /// blocks, with indices corresponding to basic block IDs. Returns an error if
-/// the dataflow analysis cannot be performed successfully. Otherwise, calls
-/// `PostVisitStmt` on each statement with the final analysis results at that
-/// program point.
+/// the dataflow analysis cannot be performed successfully.
 template <typename AnalysisT>
 llvm::Expected<std::vector<
     llvm::Optional<DataflowAnalysisState<typename AnalysisT::Lattice>>>>
-runDataflowAnalysis(
-    const ControlFlowContext &CFCtx, AnalysisT &Analysis,
-    const Environment &InitEnv,
-    std::function<void(const Stmt *, const DataflowAnalysisState<
-                                         typename AnalysisT::Lattice> &)>
-        PostVisitStmt = nullptr) {
-  std::function<void(const Stmt *, const TypeErasedDataflowAnalysisState &)>
-      PostVisitStmtClosure = nullptr;
-  if (PostVisitStmt != nullptr) {
-    PostVisitStmtClosure = [&PostVisitStmt](
-                               const Stmt *Stmt,
-                               const TypeErasedDataflowAnalysisState &State) {
-      auto *Lattice =
-          llvm::any_cast<typename AnalysisT::Lattice>(&State.Lattice.Value);
-      PostVisitStmt(Stmt, DataflowAnalysisState<typename AnalysisT::Lattice>{
-                              *Lattice, State.Env});
-    };
-  }
-
-  auto TypeErasedBlockStates = runTypeErasedDataflowAnalysis(
-      CFCtx, Analysis, InitEnv, PostVisitStmtClosure);
+runDataflowAnalysis(const ControlFlowContext &CFCtx, AnalysisT &Analysis,
+                    const Environment &InitEnv) {
+  auto TypeErasedBlockStates =
+      runTypeErasedDataflowAnalysis(CFCtx, Analysis, InitEnv);
   if (!TypeErasedBlockStates)
     return TypeErasedBlockStates.takeError();