[clang] 7538b36 - [clang][dataflow] Replace usage of deprecated functions with the optional check
Wei Yi Tee via cfe-commits
cfe-commits at lists.llvm.org
Mon Sep 19 10:33:39 PDT 2022
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"})
More information about the cfe-commits
mailing list