[clang][dataflow] Use NoopLattice in optional model

Followup to D128352. This patch pulls the `NoopLattice` class out from the `NoopAnalysis.h` test file into its own `NoopLattice.h` source file, and uses it to replace usage of `SourceLocationsLattice` in `UncheckedOptionalAccessModel`.

Reviewed By: ymandel, sgatev, gribozavr2, xazax.hun

Differential Revision: https://reviews.llvm.org/D128356

diff --git a/clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp
index 9766450..8a945a5 100644 (file)
--- a/clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp
@@ -18,7 +18,6 @@
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/Analysis/FlowSensitive/DataflowLattice.h"
 #include "clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h"
-#include "clang/Analysis/FlowSensitive/SourceLocationsLattice.h"
 #include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h"
 #include "clang/Basic/SourceLocation.h"
 #include "llvm/ADT/Any.h"
@@ -32,7 +31,6 @@ namespace clang {
 namespace tidy {
 namespace bugprone {
 using ast_matchers::MatchFinder;
-using dataflow::SourceLocationsLattice;
 using dataflow::UncheckedOptionalAccessDiagnoser;
 using dataflow::UncheckedOptionalAccessModel;
 using llvm::Optional;
@@ -56,13 +54,14 @@ analyzeFunction(const FunctionDecl &FuncDecl, ASTContext &ASTCtx) {
   UncheckedOptionalAccessModel Analysis(ASTCtx);
   UncheckedOptionalAccessDiagnoser Diagnoser;
   std::vector<SourceLocation> Diagnostics;
-  Expected<std::vector<Optional<DataflowAnalysisState<SourceLocationsLattice>>>>
+  Expected<std::vector<
+      Optional<DataflowAnalysisState<UncheckedOptionalAccessModel::Lattice>>>>
       BlockToOutputState = dataflow::runDataflowAnalysis(
           *Context, Analysis, Env,
-          [&ASTCtx, &Diagnoser,
-           &Diagnostics](const Stmt *Stmt,
-                         const DataflowAnalysisState<SourceLocationsLattice>
-                             &State) mutable {
+          [&ASTCtx, &Diagnoser, &Diagnostics](
+              const Stmt *Stmt,
+              const DataflowAnalysisState<UncheckedOptionalAccessModel::Lattice>
+                  &State) mutable {
             auto StmtDiagnostics = Diagnoser.diagnose(ASTCtx, Stmt, State.Env);
             llvm::move(StmtDiagnostics, std::back_inserter(Diagnostics));
           });

diff --git a/clang/docs/tools/clang-formatted-files.txt b/clang/docs/tools/clang-formatted-files.txt
index ed8b5e7..40d9541 100644 (file)
--- a/clang/docs/tools/clang-formatted-files.txt
+++ b/clang/docs/tools/clang-formatted-files.txt
@@ -131,6 +131,7 @@ clang/include/clang/Analysis/FlowSensitive/DataflowLattice.h
 clang/include/clang/Analysis/FlowSensitive/DataflowWorklist.h
 clang/include/clang/Analysis/FlowSensitive/MapLattice.h
 clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
+clang/include/clang/Analysis/FlowSensitive/NoopLattice.h
 clang/include/clang/Analysis/FlowSensitive/Solver.h
 clang/include/clang/Analysis/FlowSensitive/SourceLocationsLattice.h
 clang/include/clang/Analysis/FlowSensitive/StorageLocation.h

diff --git a/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h b/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
index 701db64..25054de 100644 (file)
--- a/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
+++ b/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
@@ -19,7 +19,7 @@
 #include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/Analysis/FlowSensitive/MatchSwitch.h"
-#include "clang/Analysis/FlowSensitive/SourceLocationsLattice.h"
+#include "clang/Analysis/FlowSensitive/NoopLattice.h"
 #include "clang/Basic/SourceLocation.h"
 #include <vector>
 
@@ -38,15 +38,11 @@ struct UncheckedOptionalAccessModelOptions {
   bool IgnoreSmartPointerDereference = false;
 };
 
-/// Dataflow analysis that discovers unsafe accesses of optional values and
-/// adds the respective source locations to the lattice.
+/// Dataflow analysis that models whether optionals hold values or not.
 ///
 /// Models the `std::optional`, `absl::optional`, and `base::Optional` types.
-///
-/// FIXME: Consider separating the models from the unchecked access analysis.
 class UncheckedOptionalAccessModel
-    : public DataflowAnalysis<UncheckedOptionalAccessModel,
-                              SourceLocationsLattice> {
+    : public DataflowAnalysis<UncheckedOptionalAccessModel, NoopLattice> {
 public:
   UncheckedOptionalAccessModel(
       ASTContext &AstContext, UncheckedOptionalAccessModelOptions Options = {});
@@ -54,12 +50,9 @@ public:
   /// Returns a matcher for the optional classes covered by this model.
   static ast_matchers::DeclarationMatcher optionalClassDecl();
 
-  static SourceLocationsLattice initialElement() {
-    return SourceLocationsLattice();
-  }
+  static NoopLattice initialElement() { return {}; }
 
-  void transfer(const Stmt *Stmt, SourceLocationsLattice &State,
-                Environment &Env);
+  void transfer(const Stmt *Stmt, NoopLattice &State, Environment &Env);
 
   bool compareEquivalent(QualType Type, const Value &Val1,
                          const Environment &Env1, const Value &Val2,
@@ -70,7 +63,7 @@ public:
              Environment &MergedEnv) override;
 
 private:
-  MatchSwitch<TransferState<SourceLocationsLattice>> TransferMatchSwitch;
+  MatchSwitch<TransferState<NoopLattice>> TransferMatchSwitch;
 };
 
 class UncheckedOptionalAccessDiagnoser {

diff --git a/clang/include/clang/Analysis/FlowSensitive/NoopLattice.h b/clang/include/clang/Analysis/FlowSensitive/NoopLattice.h
new file mode 100644 (file)
index 0000000..0192193
--- /dev/null
+++ b/clang/include/clang/Analysis/FlowSensitive/NoopLattice.h
@@ -0,0 +1,41 @@
+//===-- NoopLattice.h -------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+//  This file defines the lattice with exactly one element.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_NOOP_LATTICE_H
+#define LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_NOOP_LATTICE_H
+
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include <ostream>
+
+namespace clang {
+namespace dataflow {
+
+/// Trivial lattice for dataflow analysis with exactly one element.
+///
+/// Useful for analyses that only need the Environment and nothing more.
+class NoopLattice {
+public:
+  bool operator==(const NoopLattice &Other) const { return true; }
+
+  LatticeJoinEffect join(const NoopLattice &Other) {
+    return LatticeJoinEffect::Unchanged;
+  }
+};
+
+inline std::ostream &operator<<(std::ostream &OS, const NoopLattice &) {
+  return OS << "noop";
+}
+
+} // namespace dataflow
+} // namespace clang
+
+#endif // LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_NOOP_LATTICE_H

diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index 7d87d81..eef3cc8 100644 (file)
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -20,7 +20,7 @@
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/Analysis/FlowSensitive/MatchSwitch.h"
-#include "clang/Analysis/FlowSensitive/SourceLocationsLattice.h"
+#include "clang/Analysis/FlowSensitive/NoopLattice.h"
 #include "clang/Analysis/FlowSensitive/Value.h"
 #include "clang/Basic/SourceLocation.h"
 #include "llvm/ADT/StringRef.h"
@@ -35,7 +35,7 @@ namespace dataflow {
 namespace {
 
 using namespace ::clang::ast_matchers;
-using LatticeTransferState = TransferState<SourceLocationsLattice>;
+using LatticeTransferState = TransferState<NoopLattice>;
 
 DeclarationMatcher optionalClass() {
   return classTemplateSpecializationDecl(
@@ -312,18 +312,7 @@ void transferUnwrapCall(const Expr *UnwrapExpr, const Expr *ObjectExpr,
       if (auto *Loc = maybeInitializeOptionalValueMember(
               UnwrapExpr->getType(), *OptionalVal, State.Env))
         State.Env.setStorageLocation(*UnwrapExpr, *Loc);
-
-    auto *Prop = OptionalVal->getProperty("has_value");
-    if (auto *HasValueVal = cast_or_null<BoolValue>(Prop)) {
-      if (State.Env.flowConditionImplies(*HasValueVal))
-        return;
-    }
   }
-
-  // Record that this unwrap is *not* provably safe.
-  // FIXME: include either the name of the optional (if applicable) or a source
-  // range of the access for easier interpretation of the result.
-  State.Lattice.getSourceLocations().insert(ObjectExpr->getBeginLoc());
 }
 
 void transferMakeOptionalCall(const CallExpr *E,
@@ -716,12 +705,10 @@ UncheckedOptionalAccessModel::optionalClassDecl() {
 
 UncheckedOptionalAccessModel::UncheckedOptionalAccessModel(
     ASTContext &Ctx, UncheckedOptionalAccessModelOptions Options)
-    : DataflowAnalysis<UncheckedOptionalAccessModel, SourceLocationsLattice>(
-          Ctx),
+    : DataflowAnalysis<UncheckedOptionalAccessModel, NoopLattice>(Ctx),
       TransferMatchSwitch(buildTransferMatchSwitch(Options)) {}
 
-void UncheckedOptionalAccessModel::transfer(const Stmt *S,
-                                            SourceLocationsLattice &L,
+void UncheckedOptionalAccessModel::transfer(const Stmt *S, NoopLattice &L,
                                             Environment &Env) {
   LatticeTransferState State(L, Env);
   TransferMatchSwitch(*S, getASTContext(), State);

diff --git a/clang/unittests/Analysis/FlowSensitive/NoopAnalysis.h b/clang/unittests/Analysis/FlowSensitive/NoopAnalysis.h
index eab5782..45ed414 100644 (file)
--- a/clang/unittests/Analysis/FlowSensitive/NoopAnalysis.h
+++ b/clang/unittests/Analysis/FlowSensitive/NoopAnalysis.h
@@ -18,25 +18,11 @@
 #include "clang/AST/Stmt.h"
 #include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
-#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
-#include <ostream>
+#include "clang/Analysis/FlowSensitive/NoopLattice.h"
 
 namespace clang {
 namespace dataflow {
 
-class NoopLattice {
-public:
-  bool operator==(const NoopLattice &) const { return true; }
-
-  LatticeJoinEffect join(const NoopLattice &) {
-    return LatticeJoinEffect::Unchanged;
-  }
-};
-
-inline std::ostream &operator<<(std::ostream &OS, const NoopLattice &) {
-  return OS << "noop";
-}
-
 class NoopAnalysis : public DataflowAnalysis<NoopAnalysis, NoopLattice> {
 public:
   /// `ApplyBuiltinTransfer` controls whether to run the built-in transfer