[clang] a4f8e3d - Revert "[clang][dataflow] Replace `transfer(const Stmt *, ...)` with `transfer(const CFGElement *, ...)` in `Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel`."

Wei Yi Tee via cfe-commits cfe-commits at lists.llvm.org
Fri Sep 16 11:09:14 PDT 2022


Author: Wei Yi Tee
Date: 2022-09-16T18:07:35Z
New Revision: a4f8e3d24087356ba75eb6f3bd4e1af436c83270

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

LOG: Revert "[clang][dataflow] Replace `transfer(const Stmt *, ...)` with `transfer(const CFGElement *, ...)` in `Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel`."

This reverts commit 41f235d26887946f472d71a8417507c35d5f9074.

Details at https://lab.llvm.org/buildbot#builders/139/builds/28171.
Breakage due to API change.

Added: 
    

Modified: 
    clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
    clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
    clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h b/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
index 66aabb531a213..25054deaf8afc 100644
--- a/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
+++ b/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
@@ -15,10 +15,10 @@
 #define CLANG_ANALYSIS_FLOWSENSITIVE_MODELS_UNCHECKEDOPTIONALACCESSMODEL_H
 
 #include "clang/AST/ASTContext.h"
-#include "clang/Analysis/CFG.h"
-#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
+#include "clang/AST/Stmt.h"
 #include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/MatchSwitch.h"
 #include "clang/Analysis/FlowSensitive/NoopLattice.h"
 #include "clang/Basic/SourceLocation.h"
 #include <vector>
@@ -45,14 +45,14 @@ class UncheckedOptionalAccessModel
     : public DataflowAnalysis<UncheckedOptionalAccessModel, NoopLattice> {
 public:
   UncheckedOptionalAccessModel(
-      ASTContext &Ctx, UncheckedOptionalAccessModelOptions Options = {});
+      ASTContext &AstContext, UncheckedOptionalAccessModelOptions Options = {});
 
   /// Returns a matcher for the optional classes covered by this model.
   static ast_matchers::DeclarationMatcher optionalClassDecl();
 
   static NoopLattice initialElement() { return {}; }
 
-  void transfer(const CFGElement *Elt, NoopLattice &L, Environment &Env);
+  void transfer(const Stmt *Stmt, NoopLattice &State, Environment &Env);
 
   bool compareEquivalent(QualType Type, const Value &Val1,
                          const Environment &Env1, const Value &Val2,
@@ -63,7 +63,7 @@ class UncheckedOptionalAccessModel
              Environment &MergedEnv) override;
 
 private:
-  CFGMatchSwitch<TransferState<NoopLattice>> TransferMatchSwitch;
+  MatchSwitch<TransferState<NoopLattice>> TransferMatchSwitch;
 };
 
 class UncheckedOptionalAccessDiagnoser {
@@ -71,11 +71,11 @@ class UncheckedOptionalAccessDiagnoser {
   UncheckedOptionalAccessDiagnoser(
       UncheckedOptionalAccessModelOptions Options = {});
 
-  std::vector<SourceLocation> diagnose(ASTContext &Ctx, const CFGElement *Elt,
+  std::vector<SourceLocation> diagnose(ASTContext &Context, const Stmt *Stmt,
                                        const Environment &Env);
 
 private:
-  CFGMatchSwitch<const Environment, std::vector<SourceLocation>>
+  MatchSwitch<const Environment, std::vector<SourceLocation>>
       DiagnoseMatchSwitch;
 };
 

diff  --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index 1ffd88697f3a7..eef3cc813a4ac 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -18,9 +18,8 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/Stmt.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
-#include "clang/Analysis/CFG.h"
-#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/MatchSwitch.h"
 #include "clang/Analysis/FlowSensitive/NoopLattice.h"
 #include "clang/Analysis/FlowSensitive/Value.h"
 #include "clang/Basic/SourceLocation.h"
@@ -560,42 +559,41 @@ auto buildTransferMatchSwitch(
   // lot of duplicated work (e.g. string comparisons), consider providing APIs
   // that avoid it through memoization.
   auto IgnorableOptional = ignorableOptional(Options);
-  return CFGMatchSwitchBuilder<LatticeTransferState>()
+  return MatchSwitchBuilder<LatticeTransferState>()
       // Attach a symbolic "has_value" state to optional values that we see for
       // the first time.
-      .CaseOfCFGStmt<Expr>(
+      .CaseOf<Expr>(
           expr(anyOf(declRefExpr(), memberExpr()), hasOptionalType()),
           initializeOptionalReference)
 
       // make_optional
-      .CaseOfCFGStmt<CallExpr>(isMakeOptionalCall(), transferMakeOptionalCall)
+      .CaseOf<CallExpr>(isMakeOptionalCall(), transferMakeOptionalCall)
 
       // optional::optional
-      .CaseOfCFGStmt<CXXConstructExpr>(
+      .CaseOf<CXXConstructExpr>(
           isOptionalInPlaceConstructor(),
           [](const CXXConstructExpr *E, const MatchFinder::MatchResult &,
              LatticeTransferState &State) {
             assignOptionalValue(*E, State, State.Env.getBoolLiteralValue(true));
           })
-      .CaseOfCFGStmt<CXXConstructExpr>(
+      .CaseOf<CXXConstructExpr>(
           isOptionalNulloptConstructor(),
           [](const CXXConstructExpr *E, const MatchFinder::MatchResult &,
              LatticeTransferState &State) {
             assignOptionalValue(*E, State,
                                 State.Env.getBoolLiteralValue(false));
           })
-      .CaseOfCFGStmt<CXXConstructExpr>(isOptionalValueOrConversionConstructor(),
-                                       transferValueOrConversionConstructor)
+      .CaseOf<CXXConstructExpr>(isOptionalValueOrConversionConstructor(),
+                                transferValueOrConversionConstructor)
 
       // optional::operator=
-      .CaseOfCFGStmt<CXXOperatorCallExpr>(
-          isOptionalValueOrConversionAssignment(),
-          transferValueOrConversionAssignment)
-      .CaseOfCFGStmt<CXXOperatorCallExpr>(isOptionalNulloptAssignment(),
-                                          transferNulloptAssignment)
+      .CaseOf<CXXOperatorCallExpr>(isOptionalValueOrConversionAssignment(),
+                                   transferValueOrConversionAssignment)
+      .CaseOf<CXXOperatorCallExpr>(isOptionalNulloptAssignment(),
+                                   transferNulloptAssignment)
 
       // optional::value
-      .CaseOfCFGStmt<CXXMemberCallExpr>(
+      .CaseOf<CXXMemberCallExpr>(
           valueCall(IgnorableOptional),
           [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &,
              LatticeTransferState &State) {
@@ -603,25 +601,22 @@ auto buildTransferMatchSwitch(
           })
 
       // optional::operator*, optional::operator->
-      .CaseOfCFGStmt<CallExpr>(valueOperatorCall(IgnorableOptional),
-                               [](const CallExpr *E,
-                                  const MatchFinder::MatchResult &,
-                                  LatticeTransferState &State) {
-                                 transferUnwrapCall(E, E->getArg(0), State);
-                               })
+      .CaseOf<CallExpr>(valueOperatorCall(IgnorableOptional),
+                        [](const CallExpr *E, const MatchFinder::MatchResult &,
+                           LatticeTransferState &State) {
+                          transferUnwrapCall(E, E->getArg(0), State);
+                        })
 
       // optional::has_value
-      .CaseOfCFGStmt<CXXMemberCallExpr>(
-          isOptionalMemberCallWithName("has_value"),
-          transferOptionalHasValueCall)
+      .CaseOf<CXXMemberCallExpr>(isOptionalMemberCallWithName("has_value"),
+                                 transferOptionalHasValueCall)
 
       // optional::operator bool
-      .CaseOfCFGStmt<CXXMemberCallExpr>(
-          isOptionalMemberCallWithName("operator bool"),
-          transferOptionalHasValueCall)
+      .CaseOf<CXXMemberCallExpr>(isOptionalMemberCallWithName("operator bool"),
+                                 transferOptionalHasValueCall)
 
       // optional::emplace
-      .CaseOfCFGStmt<CXXMemberCallExpr>(
+      .CaseOf<CXXMemberCallExpr>(
           isOptionalMemberCallWithName("emplace"),
           [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &,
              LatticeTransferState &State) {
@@ -630,7 +625,7 @@ auto buildTransferMatchSwitch(
           })
 
       // optional::reset
-      .CaseOfCFGStmt<CXXMemberCallExpr>(
+      .CaseOf<CXXMemberCallExpr>(
           isOptionalMemberCallWithName("reset"),
           [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &,
              LatticeTransferState &State) {
@@ -639,22 +634,21 @@ auto buildTransferMatchSwitch(
           })
 
       // optional::swap
-      .CaseOfCFGStmt<CXXMemberCallExpr>(isOptionalMemberCallWithName("swap"),
-                                        transferSwapCall)
+      .CaseOf<CXXMemberCallExpr>(isOptionalMemberCallWithName("swap"),
+                                 transferSwapCall)
 
       // std::swap
-      .CaseOfCFGStmt<CallExpr>(isStdSwapCall(), transferStdSwapCall)
+      .CaseOf<CallExpr>(isStdSwapCall(), transferStdSwapCall)
 
       // opt.value_or("").empty()
-      .CaseOfCFGStmt<Expr>(isValueOrStringEmptyCall(),
-                           transferValueOrStringEmptyCall)
+      .CaseOf<Expr>(isValueOrStringEmptyCall(), transferValueOrStringEmptyCall)
 
       // opt.value_or(X) != X
-      .CaseOfCFGStmt<Expr>(isValueOrNotEqX(), transferValueOrNotEqX)
+      .CaseOf<Expr>(isValueOrNotEqX(), transferValueOrNotEqX)
 
       // returns optional
-      .CaseOfCFGStmt<CallExpr>(isCallReturningOptional(),
-                               transferCallReturningOptional)
+      .CaseOf<CallExpr>(isCallReturningOptional(),
+                        transferCallReturningOptional)
 
       .Build();
 }
@@ -683,9 +677,9 @@ auto buildDiagnoseMatchSwitch(
   // lot of duplicated work (e.g. string comparisons), consider providing APIs
   // that avoid it through memoization.
   auto IgnorableOptional = ignorableOptional(Options);
-  return CFGMatchSwitchBuilder<const Environment, std::vector<SourceLocation>>()
+  return MatchSwitchBuilder<const Environment, std::vector<SourceLocation>>()
       // optional::value
-      .CaseOfCFGStmt<CXXMemberCallExpr>(
+      .CaseOf<CXXMemberCallExpr>(
           valueCall(IgnorableOptional),
           [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &,
              const Environment &Env) {
@@ -693,7 +687,7 @@ auto buildDiagnoseMatchSwitch(
           })
 
       // optional::operator*, optional::operator->
-      .CaseOfCFGStmt<CallExpr>(
+      .CaseOf<CallExpr>(
           valueOperatorCall(IgnorableOptional),
           [](const CallExpr *E, const MatchFinder::MatchResult &,
              const Environment &Env) {
@@ -714,10 +708,10 @@ UncheckedOptionalAccessModel::UncheckedOptionalAccessModel(
     : DataflowAnalysis<UncheckedOptionalAccessModel, NoopLattice>(Ctx),
       TransferMatchSwitch(buildTransferMatchSwitch(Options)) {}
 
-void UncheckedOptionalAccessModel::transfer(const CFGElement *Elt,
-                                            NoopLattice &L, Environment &Env) {
+void UncheckedOptionalAccessModel::transfer(const Stmt *S, NoopLattice &L,
+                                            Environment &Env) {
   LatticeTransferState State(L, Env);
-  TransferMatchSwitch(*Elt, getASTContext(), State);
+  TransferMatchSwitch(*S, getASTContext(), State);
 }
 
 bool UncheckedOptionalAccessModel::compareEquivalent(QualType Type,
@@ -751,8 +745,8 @@ UncheckedOptionalAccessDiagnoser::UncheckedOptionalAccessDiagnoser(
     : DiagnoseMatchSwitch(buildDiagnoseMatchSwitch(Options)) {}
 
 std::vector<SourceLocation> UncheckedOptionalAccessDiagnoser::diagnose(
-    ASTContext &Ctx, const CFGElement *Elt, const Environment &Env) {
-  return DiagnoseMatchSwitch(*Elt, Ctx, Env);
+    ASTContext &Context, const Stmt *Stmt, const Environment &Env) {
+  return DiagnoseMatchSwitch(*Stmt, Context, Env);
 }
 
 } // namespace dataflow

diff  --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
index 845209b90004d..c06cd3a173fed 100644
--- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -14,8 +14,10 @@
 #include "clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -1246,9 +1248,13 @@ class UncheckedOptionalAccessTest
                  Diagnoser = UncheckedOptionalAccessDiagnoser(Options)](
                     ASTContext &Ctx, const CFGElement &Elt,
                     const TypeErasedDataflowAnalysisState &State) mutable {
-                  auto EltDiagnostics =
-                      Diagnoser.diagnose(Ctx, &Elt, State.Env);
-                  llvm::move(EltDiagnostics, std::back_inserter(Diagnostics));
+                  auto Stmt = Elt.getAs<CFGStmt>();
+                  if (!Stmt) {
+                    return;
+                  }
+                  auto StmtDiagnostics =
+                      Diagnoser.diagnose(Ctx, Stmt->getStmt(), State.Env);
+                  llvm::move(StmtDiagnostics, std::back_inserter(Diagnostics));
                 })
             .withASTBuildArgs(
                 {"-fsyntax-only", "-std=c++17", "-Wno-undefined-inline"})


        


More information about the cfe-commits mailing list