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"}) _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
