diff --git a/clang/include/clang/Analysis/FlowSensitive/CFGMatchSwitch.h b/clang/include/clang/Analysis/FlowSensitive/CFGMatchSwitch.h --- a/clang/include/clang/Analysis/FlowSensitive/CFGMatchSwitch.h +++ b/clang/include/clang/Analysis/FlowSensitive/CFGMatchSwitch.h @@ -24,14 +24,47 @@ #include "clang/Analysis/CFG.h" #include "clang/Analysis/FlowSensitive/MatchSwitch.h" #include +#include #include namespace clang { namespace dataflow { -template -using CFGMatchSwitch = - std::function; +template class CFGMatchSwitch { +public: + CFGMatchSwitch(std::unique_ptr CurrentElement, + ASTMatchSwitch StmtMS, + ASTMatchSwitch 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().getInitializer(), Context, + S); + case CFGElement::Statement: + case CFGElement::Constructor: + case CFGElement::CXXRecordTypedCall: + return StmtMS(*Element.castAs().getStmt(), Context, S); + default: + // FIXME: Handle other kinds of CFGElement. + return Result(); + } + } + +private: + std::unique_ptr CurrentElement; + ASTMatchSwitch StmtMS; + ASTMatchSwitch InitMS; +}; + +template +using CFGMatchSwitchAction = + std::function; /// 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 CFGMatchSwitchBuilder && CaseOfCFGStmt(MatchSwitchMatcher M, - MatchSwitchAction A) && { + CFGMatchSwitchAction A) && { + std::move(StmtBuilder) + .template CaseOf( + 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 + CFGMatchSwitchBuilder &&CaseOfCFGStmt( + MatchSwitchMatcher M, + std::function + A) && { std::move(StmtBuilder).template CaseOf(M, A); return std::move(*this); } @@ -62,34 +114,41 @@ template CFGMatchSwitchBuilder && CaseOfCFGInit(MatchSwitchMatcher M, - MatchSwitchAction A) && { + CFGMatchSwitchAction A) && { + std::move(InitBuilder) + .template CaseOf( + 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 + CFGMatchSwitchBuilder &&CaseOfCFGInit( + MatchSwitchMatcher M, + std::function + A) && { std::move(InitBuilder).template CaseOf(M, A); return std::move(*this); } CFGMatchSwitch 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().getInitializer(), - Context, S); - case CFGElement::Statement: - case CFGElement::Constructor: - case CFGElement::CXXRecordTypedCall: - return StmtMS(*Element.castAs().getStmt(), Context, S); - default: - // FIXME: Handle other kinds of CFGElement. - return Result(); - } - }; + return CFGMatchSwitch(std::move(CurrentElement), + std::move(StmtBuilder).Build(), + std::move(InitBuilder).Build()); } private: ASTMatchSwitchBuilder StmtBuilder; ASTMatchSwitchBuilder InitBuilder; + std::unique_ptr CurrentElement = + std::make_unique(nullptr); }; } // namespace dataflow diff --git a/clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h b/clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h --- a/clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h +++ b/clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h @@ -52,7 +52,7 @@ using MatchSwitchMatcher = ast_matchers::internal::Matcher; template -using MatchSwitchAction = std::function; template @@ -94,8 +94,9 @@ /// /// `NodeT` should be derived from `BaseT`. template - ASTMatchSwitchBuilder &&CaseOf(MatchSwitchMatcher M, - MatchSwitchAction A) && { + ASTMatchSwitchBuilder && + CaseOf(MatchSwitchMatcher M, + ASTMatchSwitchAction A) && { static_assert(std::is_base_of::value, "NodeT must be derived from BaseT."); Matchers.push_back(std::move(M)); @@ -158,7 +159,7 @@ } std::vector Matchers; - std::vector> Actions; + std::vector> Actions; }; // FIXME: Remove this alias when all usages of `MatchSwitchBuilder` are updated diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp --- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp +++ b/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( 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( 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( 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(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( 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( 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( 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( 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(); diff --git a/clang/unittests/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp b/clang/unittests/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp --- a/clang/unittests/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp @@ -34,11 +34,13 @@ .CaseOfCFGStmt( 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(withInitializer(integerLiteral(equals(42)))), - [](const CXXCtorInitializer *, const MatchFinder::MatchResult &, + [](const CFGElement &Element, const CXXCtorInitializer *, + const MatchFinder::MatchResult &, CFGElementMatches &Counter) { Counter.InitializerMatches++; }) .Build(); } diff --git a/clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp b/clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp --- a/clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp @@ -84,7 +84,7 @@ dyn_cast_or_null(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(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); diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp --- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp @@ -1235,6 +1235,7 @@ UncheckedOptionalAccessModelOptions Options{ /*IgnoreSmartPointerDereference=*/true}; std::vector Diagnostics; + UncheckedOptionalAccessDiagnoser Diagnoser(Options); llvm::Error Error = checkDataflow( AnalysisInputs( 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 =