Author: Wei Yi Tee Date: 2022-09-19T17:33:25Z New Revision: 7538b3604519b03d32221cdcc346cc1c181b50fb
URL: https://github.com/llvm/llvm-project/commit/7538b3604519b03d32221cdcc346cc1c181b50fb DIFF: https://github.com/llvm/llvm-project/commit/7538b3604519b03d32221cdcc346cc1c181b50fb.diff LOG: [clang][dataflow] Replace usage of deprecated functions with the optional check - Update `transfer` and `diagnose` to take `const CFGElement *` as input in `Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel`. - Update `clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp` accordingly. - Rename `runDataflowAnalysisOnCFG` to `runDataflowAnalysis` and remove the deprecated `runDataflowAnalysis` (this was only used by the now updated optional check). Reviewed By: gribozavr2, sgatev Differential Revision: https://reviews.llvm.org/D133930 Added: Modified: clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h 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-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp index 5ee1a0126b232..b2f6b9ed62843 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp @@ -59,12 +59,11 @@ analyzeFunction(const FunctionDecl &FuncDecl, ASTContext &ASTCtx) { BlockToOutputState = dataflow::runDataflowAnalysis( *Context, Analysis, Env, [&ASTCtx, &Diagnoser, &Diagnostics]( - const CFGStmt &Stmt, + const CFGElement &Elt, const DataflowAnalysisState<UncheckedOptionalAccessModel::Lattice> &State) mutable { - auto StmtDiagnostics = - Diagnoser.diagnose(ASTCtx, Stmt.getStmt(), State.Env); - llvm::move(StmtDiagnostics, std::back_inserter(Diagnostics)); + auto EltDiagnostics = Diagnoser.diagnose(ASTCtx, &Elt, State.Env); + llvm::move(EltDiagnostics, std::back_inserter(Diagnostics)); }); if (!BlockToOutputState) return llvm::None; diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h index 4e084d57ba011..098c13cf4e35a 100644 --- a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h +++ b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h @@ -136,9 +136,6 @@ template <typename LatticeT> struct DataflowAnalysisState { Environment Env; }; -// FIXME: Rename to `runDataflowAnalysis` after usages of the overload that -// applies to `CFGStmt` have been replaced. -// /// Performs dataflow analysis and returns a mapping from basic block IDs to /// dataflow analysis states that model the respective basic blocks. The /// returned vector, if any, will have the same size as the number of CFG @@ -149,7 +146,7 @@ template <typename LatticeT> struct DataflowAnalysisState { template <typename AnalysisT> llvm::Expected<std::vector< llvm::Optional<DataflowAnalysisState<typename AnalysisT::Lattice>>>> -runDataflowAnalysisOnCFG( +runDataflowAnalysis( const ControlFlowContext &CFCtx, AnalysisT &Analysis, const Environment &InitEnv, std::function<void(const CFGElement &, const DataflowAnalysisState< @@ -191,41 +188,6 @@ runDataflowAnalysisOnCFG( return BlockStates; } -/// Deprecated. Use `runDataflowAnalysisOnCFG` instead. -/// -/// Performs dataflow analysis and returns a mapping from basic block IDs to -/// 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. -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 CFGStmt &, const DataflowAnalysisState< - typename AnalysisT::Lattice> &)> - PostVisitStmt = nullptr) { - std::function<void( - const CFGElement &, - const DataflowAnalysisState<typename AnalysisT::Lattice> &)> - PostVisitCFG = nullptr; - if (PostVisitStmt) { - PostVisitCFG = - [&PostVisitStmt]( - const CFGElement &Element, - const DataflowAnalysisState<typename AnalysisT::Lattice> &State) { - if (auto Stmt = Element.getAs<CFGStmt>()) { - PostVisitStmt(*Stmt, State); - } - }; - } - return runDataflowAnalysisOnCFG(CFCtx, Analysis, InitEnv, PostVisitCFG); -} - /// Abstract base class for dataflow "models": reusable analysis components that /// model a particular aspect of program semantics in the `Environment`. For /// example, a model may capture a type and its related functions. diff --git a/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h b/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h index 25054deaf8afc..66aabb531a213 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/AST/Stmt.h" +#include "clang/Analysis/CFG.h" +#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.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 &AstContext, UncheckedOptionalAccessModelOptions Options = {}); + ASTContext &Ctx, 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 Stmt *Stmt, NoopLattice &State, Environment &Env); + void transfer(const CFGElement *Elt, NoopLattice &L, 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: - MatchSwitch<TransferState<NoopLattice>> TransferMatchSwitch; + CFGMatchSwitch<TransferState<NoopLattice>> TransferMatchSwitch; }; class UncheckedOptionalAccessDiagnoser { @@ -71,11 +71,11 @@ class UncheckedOptionalAccessDiagnoser { UncheckedOptionalAccessDiagnoser( UncheckedOptionalAccessModelOptions Options = {}); - std::vector<SourceLocation> diagnose(ASTContext &Context, const Stmt *Stmt, + std::vector<SourceLocation> diagnose(ASTContext &Ctx, const CFGElement *Elt, const Environment &Env); private: - MatchSwitch<const Environment, std::vector<SourceLocation>> + CFGMatchSwitch<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 eef3cc813a4ac..1ffd88697f3a7 100644 --- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp +++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp @@ -18,8 +18,9 @@ #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" @@ -559,41 +560,42 @@ auto buildTransferMatchSwitch( // lot of duplicated work (e.g. string comparisons), consider providing APIs // that avoid it through memoization. auto IgnorableOptional = ignorableOptional(Options); - return MatchSwitchBuilder<LatticeTransferState>() + return CFGMatchSwitchBuilder<LatticeTransferState>() // Attach a symbolic "has_value" state to optional values that we see for // the first time. - .CaseOf<Expr>( + .CaseOfCFGStmt<Expr>( expr(anyOf(declRefExpr(), memberExpr()), hasOptionalType()), initializeOptionalReference) // make_optional - .CaseOf<CallExpr>(isMakeOptionalCall(), transferMakeOptionalCall) + .CaseOfCFGStmt<CallExpr>(isMakeOptionalCall(), transferMakeOptionalCall) // optional::optional - .CaseOf<CXXConstructExpr>( + .CaseOfCFGStmt<CXXConstructExpr>( isOptionalInPlaceConstructor(), [](const CXXConstructExpr *E, const MatchFinder::MatchResult &, LatticeTransferState &State) { assignOptionalValue(*E, State, State.Env.getBoolLiteralValue(true)); }) - .CaseOf<CXXConstructExpr>( + .CaseOfCFGStmt<CXXConstructExpr>( isOptionalNulloptConstructor(), [](const CXXConstructExpr *E, const MatchFinder::MatchResult &, LatticeTransferState &State) { assignOptionalValue(*E, State, State.Env.getBoolLiteralValue(false)); }) - .CaseOf<CXXConstructExpr>(isOptionalValueOrConversionConstructor(), - transferValueOrConversionConstructor) + .CaseOfCFGStmt<CXXConstructExpr>(isOptionalValueOrConversionConstructor(), + transferValueOrConversionConstructor) // optional::operator= - .CaseOf<CXXOperatorCallExpr>(isOptionalValueOrConversionAssignment(), - transferValueOrConversionAssignment) - .CaseOf<CXXOperatorCallExpr>(isOptionalNulloptAssignment(), - transferNulloptAssignment) + .CaseOfCFGStmt<CXXOperatorCallExpr>( + isOptionalValueOrConversionAssignment(), + transferValueOrConversionAssignment) + .CaseOfCFGStmt<CXXOperatorCallExpr>(isOptionalNulloptAssignment(), + transferNulloptAssignment) // optional::value - .CaseOf<CXXMemberCallExpr>( + .CaseOfCFGStmt<CXXMemberCallExpr>( valueCall(IgnorableOptional), [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &, LatticeTransferState &State) { @@ -601,22 +603,25 @@ auto buildTransferMatchSwitch( }) // optional::operator*, optional::operator-> - .CaseOf<CallExpr>(valueOperatorCall(IgnorableOptional), - [](const CallExpr *E, const MatchFinder::MatchResult &, - LatticeTransferState &State) { - transferUnwrapCall(E, E->getArg(0), State); - }) + .CaseOfCFGStmt<CallExpr>(valueOperatorCall(IgnorableOptional), + [](const CallExpr *E, + const MatchFinder::MatchResult &, + LatticeTransferState &State) { + transferUnwrapCall(E, E->getArg(0), State); + }) // optional::has_value - .CaseOf<CXXMemberCallExpr>(isOptionalMemberCallWithName("has_value"), - transferOptionalHasValueCall) + .CaseOfCFGStmt<CXXMemberCallExpr>( + isOptionalMemberCallWithName("has_value"), + transferOptionalHasValueCall) // optional::operator bool - .CaseOf<CXXMemberCallExpr>(isOptionalMemberCallWithName("operator bool"), - transferOptionalHasValueCall) + .CaseOfCFGStmt<CXXMemberCallExpr>( + isOptionalMemberCallWithName("operator bool"), + transferOptionalHasValueCall) // optional::emplace - .CaseOf<CXXMemberCallExpr>( + .CaseOfCFGStmt<CXXMemberCallExpr>( isOptionalMemberCallWithName("emplace"), [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &, LatticeTransferState &State) { @@ -625,7 +630,7 @@ auto buildTransferMatchSwitch( }) // optional::reset - .CaseOf<CXXMemberCallExpr>( + .CaseOfCFGStmt<CXXMemberCallExpr>( isOptionalMemberCallWithName("reset"), [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &, LatticeTransferState &State) { @@ -634,21 +639,22 @@ auto buildTransferMatchSwitch( }) // optional::swap - .CaseOf<CXXMemberCallExpr>(isOptionalMemberCallWithName("swap"), - transferSwapCall) + .CaseOfCFGStmt<CXXMemberCallExpr>(isOptionalMemberCallWithName("swap"), + transferSwapCall) // std::swap - .CaseOf<CallExpr>(isStdSwapCall(), transferStdSwapCall) + .CaseOfCFGStmt<CallExpr>(isStdSwapCall(), transferStdSwapCall) // opt.value_or("").empty() - .CaseOf<Expr>(isValueOrStringEmptyCall(), transferValueOrStringEmptyCall) + .CaseOfCFGStmt<Expr>(isValueOrStringEmptyCall(), + transferValueOrStringEmptyCall) // opt.value_or(X) != X - .CaseOf<Expr>(isValueOrNotEqX(), transferValueOrNotEqX) + .CaseOfCFGStmt<Expr>(isValueOrNotEqX(), transferValueOrNotEqX) // returns optional - .CaseOf<CallExpr>(isCallReturningOptional(), - transferCallReturningOptional) + .CaseOfCFGStmt<CallExpr>(isCallReturningOptional(), + transferCallReturningOptional) .Build(); } @@ -677,9 +683,9 @@ auto buildDiagnoseMatchSwitch( // lot of duplicated work (e.g. string comparisons), consider providing APIs // that avoid it through memoization. auto IgnorableOptional = ignorableOptional(Options); - return MatchSwitchBuilder<const Environment, std::vector<SourceLocation>>() + return CFGMatchSwitchBuilder<const Environment, std::vector<SourceLocation>>() // optional::value - .CaseOf<CXXMemberCallExpr>( + .CaseOfCFGStmt<CXXMemberCallExpr>( valueCall(IgnorableOptional), [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &, const Environment &Env) { @@ -687,7 +693,7 @@ auto buildDiagnoseMatchSwitch( }) // optional::operator*, optional::operator-> - .CaseOf<CallExpr>( + .CaseOfCFGStmt<CallExpr>( valueOperatorCall(IgnorableOptional), [](const CallExpr *E, const MatchFinder::MatchResult &, const Environment &Env) { @@ -708,10 +714,10 @@ UncheckedOptionalAccessModel::UncheckedOptionalAccessModel( : DataflowAnalysis<UncheckedOptionalAccessModel, NoopLattice>(Ctx), TransferMatchSwitch(buildTransferMatchSwitch(Options)) {} -void UncheckedOptionalAccessModel::transfer(const Stmt *S, NoopLattice &L, - Environment &Env) { +void UncheckedOptionalAccessModel::transfer(const CFGElement *Elt, + NoopLattice &L, Environment &Env) { LatticeTransferState State(L, Env); - TransferMatchSwitch(*S, getASTContext(), State); + TransferMatchSwitch(*Elt, getASTContext(), State); } bool UncheckedOptionalAccessModel::compareEquivalent(QualType Type, @@ -745,8 +751,8 @@ UncheckedOptionalAccessDiagnoser::UncheckedOptionalAccessDiagnoser( : DiagnoseMatchSwitch(buildDiagnoseMatchSwitch(Options)) {} std::vector<SourceLocation> UncheckedOptionalAccessDiagnoser::diagnose( - ASTContext &Context, const Stmt *Stmt, const Environment &Env) { - return DiagnoseMatchSwitch(*Stmt, Context, Env); + ASTContext &Ctx, const CFGElement *Elt, const Environment &Env) { + return DiagnoseMatchSwitch(*Elt, Ctx, Env); } } // namespace dataflow diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp index c06cd3a173fed..845209b90004d 100644 --- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp @@ -14,10 +14,8 @@ #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" @@ -1248,13 +1246,9 @@ class UncheckedOptionalAccessTest Diagnoser = UncheckedOptionalAccessDiagnoser(Options)]( ASTContext &Ctx, const CFGElement &Elt, const TypeErasedDataflowAnalysisState &State) mutable { - auto Stmt = Elt.getAs<CFGStmt>(); - if (!Stmt) { - return; - } - auto StmtDiagnostics = - Diagnoser.diagnose(Ctx, Stmt->getStmt(), State.Env); - llvm::move(StmtDiagnostics, std::back_inserter(Diagnostics)); + auto EltDiagnostics = + Diagnoser.diagnose(Ctx, &Elt, State.Env); + llvm::move(EltDiagnostics, 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
