merrymeerkat created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
merrymeerkat requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
For example, previously, if the dataflow analysis wanted to warn the user about
a CXXCtorInitializer, it could not show a precise warning because diagnosers
only accepted Stmts.
Now, diagnosers receive a CFGElement and can thus warn about any type of C++
construct.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D137432
Files:
clang/include/clang/Analysis/FlowSensitive/CFGMatchSwitch.h
clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
clang/unittests/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp
clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp
clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -1235,6 +1235,7 @@
UncheckedOptionalAccessModelOptions Options{
/*IgnoreSmartPointerDereference=*/true};
std::vector<SourceLocation> Diagnostics;
+ UncheckedOptionalAccessDiagnoser Diagnoser(Options);
llvm::Error Error = checkDataflow<UncheckedOptionalAccessModel>(
AnalysisInputs<UncheckedOptionalAccessModel>(
SourceCode, std::move(FuncMatcher),
@@ -1242,8 +1243,7 @@
return UncheckedOptionalAccessModel(Ctx, Options);
})
.withPostVisitCFG(
- [&Diagnostics,
- Diagnoser = UncheckedOptionalAccessDiagnoser(Options)](
+ [&Diagnostics, &Diagnoser](
ASTContext &Ctx, const CFGElement &Elt,
const TypeErasedDataflowAnalysisState &State) mutable {
auto EltDiagnostics =
Index: clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp
@@ -84,7 +84,7 @@
dyn_cast_or_null<BoolValue>(Val.getProperty("pos"))};
}
-void transferUninitializedInt(const DeclStmt *D,
+void transferUninitializedInt(const CFGElement &Element, const DeclStmt *D,
const MatchFinder::MatchResult &M,
LatticeTransferState &State) {
const auto *Var = M.Nodes.getNodeAs<clang::VarDecl>(kVar);
@@ -133,7 +133,8 @@
return {UnaryOpValue, UnaryOpProps, OperandProps};
}
-void transferBinary(const BinaryOperator *BO, const MatchFinder::MatchResult &M,
+void transferBinary(const CFGElement &Element, const BinaryOperator *BO,
+ const MatchFinder::MatchResult &M,
LatticeTransferState &State) {
StorageLocation *Loc = State.Env.getStorageLocation(*BO, SkipPast::None);
if (!Loc) {
@@ -202,7 +203,7 @@
}
}
-void transferUnaryMinus(const UnaryOperator *UO,
+void transferUnaryMinus(const CFGElement &Element, const UnaryOperator *UO,
const MatchFinder::MatchResult &M,
LatticeTransferState &State) {
auto [UnaryOpValue, UnaryOpProps, OperandProps] =
@@ -221,7 +222,7 @@
State.Env.makeImplication(*OperandProps.Zero, *UnaryOpProps.Zero));
}
-void transferUnaryNot(const UnaryOperator *UO,
+void transferUnaryNot(const CFGElement &Element, const UnaryOperator *UO,
const MatchFinder::MatchResult &M,
LatticeTransferState &State) {
auto [UnaryOpValue, UnaryOpProps, OperandProps] =
@@ -246,7 +247,8 @@
}
}
-void transferExpr(const Expr *E, const MatchFinder::MatchResult &M,
+void transferExpr(const CFGElement &Element, const Expr *E,
+ const MatchFinder::MatchResult &M,
LatticeTransferState &State) {
const ASTContext &Context = *M.Context;
StorageLocation *Loc = State.Env.getStorageLocation(*E, SkipPast::None);
Index: clang/unittests/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp
@@ -34,11 +34,13 @@
.CaseOfCFGStmt<DeclStmt>(
declStmt(hasSingleDecl(
varDecl(hasInitializer(integerLiteral(equals(42)))))),
- [](const DeclStmt *, const MatchFinder::MatchResult &,
+ [](const CFGElement &Element, const DeclStmt *,
+ const MatchFinder::MatchResult &,
CFGElementMatches &Counter) { Counter.StmtMatches++; })
.CaseOfCFGInit<CXXCtorInitializer>(
cxxCtorInitializer(withInitializer(integerLiteral(equals(42)))),
- [](const CXXCtorInitializer *, const MatchFinder::MatchResult &,
+ [](const CFGElement &Element, const CXXCtorInitializer *,
+ const MatchFinder::MatchResult &,
CFGElementMatches &Counter) { Counter.InitializerMatches++; })
.Build();
}
Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -277,7 +277,8 @@
return &ValueLoc;
}
-void initializeOptionalReference(const Expr *OptionalExpr,
+void initializeOptionalReference(const CFGElement &Element,
+ const Expr *OptionalExpr,
const MatchFinder::MatchResult &,
LatticeTransferState &State) {
if (auto *OptionalVal =
@@ -316,7 +317,7 @@
}
}
-void transferMakeOptionalCall(const CallExpr *E,
+void transferMakeOptionalCall(const CFGElement &Element, const CallExpr *E,
const MatchFinder::MatchResult &,
LatticeTransferState &State) {
auto &Loc = State.Env.createStorageLocation(*E);
@@ -325,7 +326,8 @@
Loc, createOptionalValue(State.Env, State.Env.getBoolLiteralValue(true)));
}
-void transferOptionalHasValueCall(const CXXMemberCallExpr *CallExpr,
+void transferOptionalHasValueCall(const CFGElement &Element,
+ const CXXMemberCallExpr *CallExpr,
const MatchFinder::MatchResult &,
LatticeTransferState &State) {
if (auto *HasValueVal = getHasValue(
@@ -369,7 +371,8 @@
Env.addToFlowCondition(ModelPred(Env, *ExprValue, *HasValueVal));
}
-void transferValueOrStringEmptyCall(const clang::Expr *ComparisonExpr,
+void transferValueOrStringEmptyCall(const CFGElement &Element,
+ const clang::Expr *ComparisonExpr,
const MatchFinder::MatchResult &Result,
LatticeTransferState &State) {
return transferValueOrImpl(ComparisonExpr, Result, State,
@@ -386,7 +389,8 @@
});
}
-void transferValueOrNotEqX(const Expr *ComparisonExpr,
+void transferValueOrNotEqX(const CFGElement &Element,
+ const Expr *ComparisonExpr,
const MatchFinder::MatchResult &Result,
LatticeTransferState &State) {
transferValueOrImpl(ComparisonExpr, Result, State,
@@ -399,7 +403,7 @@
});
}
-void transferCallReturningOptional(const CallExpr *E,
+void transferCallReturningOptional(const CFGElement &Element, const CallExpr *E,
const MatchFinder::MatchResult &Result,
LatticeTransferState &State) {
if (State.Env.getStorageLocation(*E, SkipPast::None) != nullptr)
@@ -448,8 +452,8 @@
}
void transferValueOrConversionConstructor(
- const CXXConstructExpr *E, const MatchFinder::MatchResult &MatchRes,
- LatticeTransferState &State) {
+ const CFGElement &Element, const CXXConstructExpr *E,
+ const MatchFinder::MatchResult &MatchRes, LatticeTransferState &State) {
assert(E->getNumArgs() > 0);
assignOptionalValue(*E, State,
@@ -474,8 +478,8 @@
}
void transferValueOrConversionAssignment(
- const CXXOperatorCallExpr *E, const MatchFinder::MatchResult &MatchRes,
- LatticeTransferState &State) {
+ const CFGElement &Element, const CXXOperatorCallExpr *E,
+ const MatchFinder::MatchResult &MatchRes, LatticeTransferState &State) {
assert(E->getNumArgs() > 1);
transferAssignment(E,
value_orConversionHasValue(*E->getDirectCallee(),
@@ -483,7 +487,8 @@
State);
}
-void transferNulloptAssignment(const CXXOperatorCallExpr *E,
+void transferNulloptAssignment(const CFGElement &Element,
+ const CXXOperatorCallExpr *E,
const MatchFinder::MatchResult &,
LatticeTransferState &State) {
transferAssignment(E, State.Env.getBoolLiteralValue(false), State);
@@ -502,7 +507,7 @@
State.Env.setValue(OptionalLoc2, *OptionalVal1);
}
-void transferSwapCall(const CXXMemberCallExpr *E,
+void transferSwapCall(const CFGElement &Element, const CXXMemberCallExpr *E,
const MatchFinder::MatchResult &,
LatticeTransferState &State) {
assert(E->getNumArgs() == 1);
@@ -518,7 +523,8 @@
transferSwap(*OptionalLoc1, *OptionalLoc2, State);
}
-void transferStdSwapCall(const CallExpr *E, const MatchFinder::MatchResult &,
+void transferStdSwapCall(const CFGElement &Element, const CallExpr *E,
+ const MatchFinder::MatchResult &,
LatticeTransferState &State) {
assert(E->getNumArgs() == 2);
@@ -573,14 +579,14 @@
// optional::optional
.CaseOfCFGStmt<CXXConstructExpr>(
isOptionalInPlaceConstructor(),
- [](const CXXConstructExpr *E, const MatchFinder::MatchResult &,
- LatticeTransferState &State) {
+ [](const CFGElement &Element, const CXXConstructExpr *E,
+ const MatchFinder::MatchResult &, LatticeTransferState &State) {
assignOptionalValue(*E, State, State.Env.getBoolLiteralValue(true));
})
.CaseOfCFGStmt<CXXConstructExpr>(
isOptionalNulloptConstructor(),
- [](const CXXConstructExpr *E, const MatchFinder::MatchResult &,
- LatticeTransferState &State) {
+ [](const CFGElement &Element, const CXXConstructExpr *E,
+ const MatchFinder::MatchResult &, LatticeTransferState &State) {
assignOptionalValue(*E, State,
State.Env.getBoolLiteralValue(false));
})
@@ -597,14 +603,14 @@
// optional::value
.CaseOfCFGStmt<CXXMemberCallExpr>(
valueCall(IgnorableOptional),
- [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &,
- LatticeTransferState &State) {
+ [](const CFGElement &Element, const CXXMemberCallExpr *E,
+ const MatchFinder::MatchResult &, LatticeTransferState &State) {
transferUnwrapCall(E, E->getImplicitObjectArgument(), State);
})
// optional::operator*, optional::operator->
.CaseOfCFGStmt<CallExpr>(valueOperatorCall(IgnorableOptional),
- [](const CallExpr *E,
+ [](const CFGElement &Element, const CallExpr *E,
const MatchFinder::MatchResult &,
LatticeTransferState &State) {
transferUnwrapCall(E, E->getArg(0), State);
@@ -623,8 +629,8 @@
// optional::emplace
.CaseOfCFGStmt<CXXMemberCallExpr>(
isOptionalMemberCallWithName("emplace"),
- [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &,
- LatticeTransferState &State) {
+ [](const CFGElement &Element, const CXXMemberCallExpr *E,
+ const MatchFinder::MatchResult &, LatticeTransferState &State) {
assignOptionalValue(*E->getImplicitObjectArgument(), State,
State.Env.getBoolLiteralValue(true));
})
@@ -632,8 +638,8 @@
// optional::reset
.CaseOfCFGStmt<CXXMemberCallExpr>(
isOptionalMemberCallWithName("reset"),
- [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &,
- LatticeTransferState &State) {
+ [](const CFGElement &Element, const CXXMemberCallExpr *E,
+ const MatchFinder::MatchResult &, LatticeTransferState &State) {
assignOptionalValue(*E->getImplicitObjectArgument(), State,
State.Env.getBoolLiteralValue(false));
})
@@ -687,16 +693,16 @@
// optional::value
.CaseOfCFGStmt<CXXMemberCallExpr>(
valueCall(IgnorableOptional),
- [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &,
- const Environment &Env) {
+ [](const CFGElement &Element, const CXXMemberCallExpr *E,
+ const MatchFinder::MatchResult &, const Environment &Env) {
return diagnoseUnwrapCall(E, E->getImplicitObjectArgument(), Env);
})
// optional::operator*, optional::operator->
.CaseOfCFGStmt<CallExpr>(
valueOperatorCall(IgnorableOptional),
- [](const CallExpr *E, const MatchFinder::MatchResult &,
- const Environment &Env) {
+ [](const CFGElement &Element, const CallExpr *E,
+ const MatchFinder::MatchResult &, const Environment &Env) {
return diagnoseUnwrapCall(E, E->getArg(0), Env);
})
.Build();
Index: clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
+++ clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
@@ -52,7 +52,7 @@
using MatchSwitchMatcher = ast_matchers::internal::Matcher<T>;
template <typename T, typename State, typename Result = void>
-using MatchSwitchAction = std::function<Result(
+using ASTMatchSwitchAction = std::function<Result(
const T *, const ast_matchers::MatchFinder::MatchResult &, State &)>;
template <typename BaseT, typename State, typename Result = void>
@@ -94,8 +94,9 @@
///
/// `NodeT` should be derived from `BaseT`.
template <typename NodeT>
- ASTMatchSwitchBuilder &&CaseOf(MatchSwitchMatcher<BaseT> M,
- MatchSwitchAction<NodeT, State, Result> A) && {
+ ASTMatchSwitchBuilder &&
+ CaseOf(MatchSwitchMatcher<BaseT> M,
+ ASTMatchSwitchAction<NodeT, State, Result> A) && {
static_assert(std::is_base_of<BaseT, NodeT>::value,
"NodeT must be derived from BaseT.");
Matchers.push_back(std::move(M));
@@ -158,7 +159,7 @@
}
std::vector<ast_matchers::internal::DynTypedMatcher> Matchers;
- std::vector<MatchSwitchAction<BaseT, State, Result>> Actions;
+ std::vector<ASTMatchSwitchAction<BaseT, State, Result>> Actions;
};
// FIXME: Remove this alias when all usages of `MatchSwitchBuilder` are updated
Index: clang/include/clang/Analysis/FlowSensitive/CFGMatchSwitch.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/CFGMatchSwitch.h
+++ clang/include/clang/Analysis/FlowSensitive/CFGMatchSwitch.h
@@ -24,14 +24,47 @@
#include "clang/Analysis/CFG.h"
#include "clang/Analysis/FlowSensitive/MatchSwitch.h"
#include <functional>
+#include <memory>
#include <utility>
namespace clang {
namespace dataflow {
-template <typename State, typename Result = void>
-using CFGMatchSwitch =
- std::function<Result(const CFGElement &, ASTContext &, State &)>;
+template <typename State, typename Result = void> class CFGMatchSwitch {
+public:
+ CFGMatchSwitch(std::unique_ptr<const CFGElement *> CurrentElement,
+ ASTMatchSwitch<Stmt, State, Result> StmtMS,
+ ASTMatchSwitch<CXXCtorInitializer, State, Result> InitMS)
+ : CurrentElement(std::move(CurrentElement)), StmtMS(std::move(StmtMS)),
+ InitMS(std::move(InitMS)) {}
+
+ Result operator()(const CFGElement &Element, ASTContext &Context, State &S) {
+ *CurrentElement = ∈
+ switch (Element.getKind()) {
+ case CFGElement::Initializer:
+ return InitMS(*Element.castAs<CFGInitializer>().getInitializer(), Context,
+ S);
+ case CFGElement::Statement:
+ case CFGElement::Constructor:
+ case CFGElement::CXXRecordTypedCall:
+ return StmtMS(*Element.castAs<CFGStmt>().getStmt(), Context, S);
+ default:
+ // FIXME: Handle other kinds of CFGElement.
+ return Result();
+ }
+ }
+
+private:
+ std::unique_ptr<const CFGElement *> CurrentElement;
+ ASTMatchSwitch<Stmt, State, Result> StmtMS;
+ ASTMatchSwitch<CXXCtorInitializer, State, Result> InitMS;
+};
+
+template <typename T, typename State, typename Result = void>
+using CFGMatchSwitchAction =
+ std::function<Result(const CFGElement &, const T *,
+ const ast_matchers::MatchFinder::MatchResult &,
+ State &)>;
/// Collects cases of a "match switch": a collection of matchers paired with
/// callbacks, which together define a switch that can be applied to an AST node
@@ -47,7 +80,26 @@
template <typename NodeT>
CFGMatchSwitchBuilder &&
CaseOfCFGStmt(MatchSwitchMatcher<Stmt> M,
- MatchSwitchAction<NodeT, State, Result> A) && {
+ CFGMatchSwitchAction<NodeT, State, Result> A) && {
+ std::move(StmtBuilder)
+ .template CaseOf<NodeT>(
+ M,
+ [A, CurrentElement = CurrentElement.get()](
+ const NodeT *N,
+ const ast_matchers::MatchFinder::MatchResult &MR,
+ State &S) -> Result { return A(**CurrentElement, N, MR, S); });
+ return std::move(*this);
+ }
+
+ // TODO: Refactor all callers to call the overload above and delete this
+ // function.
+ template <typename NodeT>
+ CFGMatchSwitchBuilder &&CaseOfCFGStmt(
+ MatchSwitchMatcher<Stmt> M,
+ std::function<Result(const NodeT *,
+ const ast_matchers::MatchFinder::MatchResult &,
+ State &)>
+ A) && {
std::move(StmtBuilder).template CaseOf<NodeT>(M, A);
return std::move(*this);
}
@@ -62,34 +114,41 @@
template <typename NodeT>
CFGMatchSwitchBuilder &&
CaseOfCFGInit(MatchSwitchMatcher<CXXCtorInitializer> M,
- MatchSwitchAction<NodeT, State, Result> A) && {
+ CFGMatchSwitchAction<NodeT, State, Result> A) && {
+ std::move(InitBuilder)
+ .template CaseOf<NodeT>(
+ M,
+ [A, CurrentElement = CurrentElement.get()](
+ const NodeT *N,
+ const ast_matchers::MatchFinder::MatchResult &MR,
+ State &S) -> Result { return A(**CurrentElement, N, MR, S); });
+ return std::move(*this);
+ }
+
+ // TODO: Refactor all callers to call the overload above and delete this
+ // function.
+ template <typename NodeT>
+ CFGMatchSwitchBuilder &&CaseOfCFGInit(
+ MatchSwitchMatcher<CXXCtorInitializer> M,
+ std::function<Result(const NodeT *,
+ const ast_matchers::MatchFinder::MatchResult &,
+ State &)>
+ A) && {
std::move(InitBuilder).template CaseOf<NodeT>(M, A);
return std::move(*this);
}
CFGMatchSwitch<State, Result> Build() && {
- return [StmtMS = std::move(StmtBuilder).Build(),
- InitMS = std::move(InitBuilder).Build()](const CFGElement &Element,
- ASTContext &Context,
- State &S) -> Result {
- switch (Element.getKind()) {
- case CFGElement::Initializer:
- return InitMS(*Element.castAs<CFGInitializer>().getInitializer(),
- Context, S);
- case CFGElement::Statement:
- case CFGElement::Constructor:
- case CFGElement::CXXRecordTypedCall:
- return StmtMS(*Element.castAs<CFGStmt>().getStmt(), Context, S);
- default:
- // FIXME: Handle other kinds of CFGElement.
- return Result();
- }
- };
+ return CFGMatchSwitch<State, Result>(std::move(CurrentElement),
+ std::move(StmtBuilder).Build(),
+ std::move(InitBuilder).Build());
}
private:
ASTMatchSwitchBuilder<Stmt, State, Result> StmtBuilder;
ASTMatchSwitchBuilder<CXXCtorInitializer, State, Result> InitBuilder;
+ std::unique_ptr<const CFGElement *> CurrentElement =
+ std::make_unique<const CFGElement *>(nullptr);
};
} // namespace dataflow
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
