Author: Florian Mayer Date: 2026-02-05T10:21:07-08:00 New Revision: 356ca4036da62fe06c109704d8042764f489cb73
URL: https://github.com/llvm/llvm-project/commit/356ca4036da62fe06c109704d8042764f489cb73 DIFF: https://github.com/llvm/llvm-project/commit/356ca4036da62fe06c109704d8042764f489cb73.diff LOG: [FlowSensitive] [StatusOr] cache return values for all accessors This is important for iterators that often return pairs containing StatusOr. Pull Request: https://github.com/llvm/llvm-project/pull/179807 Added: Modified: clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp Removed: ################################################################################ diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp index 6aa7efe176c72..7dc3f5872f4d8 100644 --- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp +++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp @@ -238,18 +238,18 @@ static auto possiblyReferencedStatusOrType() { return anyOf(statusOrType(), referenceType(pointee(statusOrType()))); } -static auto isConstStatusOrAccessorMemberCall() { +static auto isConstAccessorMemberCall() { using namespace ::clang::ast_matchers; // NOLINT: Too many names - return cxxMemberCallExpr(callee( - cxxMethodDecl(parameterCountIs(0), isConst(), - returns(qualType(possiblyReferencedStatusOrType()))))); + return cxxMemberCallExpr(callee(cxxMethodDecl( + parameterCountIs(0), isConst(), + returns(hasCanonicalType(anyOf(referenceType(), recordType())))))); } -static auto isConstStatusOrAccessorMemberOperatorCall() { +static auto isConstAccessorMemberOperatorCall() { using namespace ::clang::ast_matchers; // NOLINT: Too many names - return cxxOperatorCallExpr( - callee(cxxMethodDecl(parameterCountIs(0), isConst(), - returns(possiblyReferencedStatusOrType())))); + return cxxOperatorCallExpr(callee(cxxMethodDecl( + parameterCountIs(0), isConst(), + returns(hasCanonicalType(anyOf(referenceType(), recordType())))))); } static auto isConstPointerAccessorMemberCall() { @@ -872,10 +872,9 @@ static void transferStatusOrReturningCall(const CallExpr *Expr, initializeStatusOr(*StatusOrLoc, State.Env); } -static bool doHandleConstStatusOrAccessorMemberCall( +static bool doHandleConstAccessorMemberCall( const CallExpr *Expr, RecordStorageLocation *RecordLoc, const MatchFinder::MatchResult &Result, LatticeTransferState &State) { - assert(isStatusOrType(Expr->getType())); if (RecordLoc == nullptr) return false; const FunctionDecl *DirectCallee = Expr->getDirectCallee(); @@ -884,7 +883,8 @@ static bool doHandleConstStatusOrAccessorMemberCall( StorageLocation &Loc = State.Lattice.getOrCreateConstMethodReturnStorageLocation( *RecordLoc, DirectCallee, State.Env, [&](StorageLocation &Loc) { - initializeStatusOr(cast<RecordStorageLocation>(Loc), State.Env); + if (isStatusOrType(Expr->getType())) + initializeStatusOr(cast<RecordStorageLocation>(Loc), State.Env); }); if (Expr->isPRValue()) { auto &ResultLoc = State.Env.getResultObjectLocation(*Expr); @@ -895,10 +895,11 @@ static bool doHandleConstStatusOrAccessorMemberCall( return true; } -static void handleConstStatusOrAccessorMemberCall( +static void handleConstAccessorMemberCall( const CallExpr *Expr, RecordStorageLocation *RecordLoc, const MatchFinder::MatchResult &Result, LatticeTransferState &State) { - if (!doHandleConstStatusOrAccessorMemberCall(Expr, RecordLoc, Result, State)) + if (!doHandleConstAccessorMemberCall(Expr, RecordLoc, Result, State) && + isStatusOrType(Expr->getType())) transferStatusOrReturningCall(Expr, State); } static void handleConstPointerAccessorMemberCall( @@ -912,19 +913,26 @@ static void handleConstPointerAccessorMemberCall( } static void -transferConstStatusOrAccessorMemberCall(const CXXMemberCallExpr *Expr, - const MatchFinder::MatchResult &Result, - LatticeTransferState &State) { - handleConstStatusOrAccessorMemberCall( +transferConstAccessorMemberCall(const CXXMemberCallExpr *Expr, + const MatchFinder::MatchResult &Result, + LatticeTransferState &State) { + auto Type = Expr->getType(); + if (!Type->isRecordType() && !Type->isReferenceType()) + return; + handleConstAccessorMemberCall( Expr, getImplicitObjectLocation(*Expr, State.Env), Result, State); } -static void transferConstStatusOrAccessorMemberOperatorCall( - const CXXOperatorCallExpr *Expr, const MatchFinder::MatchResult &Result, - LatticeTransferState &State) { +static void +transferConstAccessorMemberOperatorCall(const CXXOperatorCallExpr *Expr, + const MatchFinder::MatchResult &Result, + LatticeTransferState &State) { + auto Type = Expr->getArg(0)->getType(); + if (!Type->isRecordType() && !Type->isReferenceType()) + return; auto *RecordLoc = cast_or_null<RecordStorageLocation>( State.Env.getStorageLocation(*Expr->getArg(0))); - handleConstStatusOrAccessorMemberCall(Expr, RecordLoc, Result, State); + handleConstAccessorMemberCall(Expr, RecordLoc, Result, State); } static void @@ -1264,11 +1272,11 @@ buildTransferMatchSwitch(ASTContext &Ctx, [](StorageLocation &Loc) {}); }) // const accessor calls - .CaseOfCFGStmt<CXXMemberCallExpr>(isConstStatusOrAccessorMemberCall(), - transferConstStatusOrAccessorMemberCall) + .CaseOfCFGStmt<CXXMemberCallExpr>(isConstAccessorMemberCall(), + transferConstAccessorMemberCall) .CaseOfCFGStmt<CXXOperatorCallExpr>( - isConstStatusOrAccessorMemberOperatorCall(), - transferConstStatusOrAccessorMemberOperatorCall) + isConstAccessorMemberOperatorCall(), + transferConstAccessorMemberOperatorCall) .CaseOfCFGStmt<CXXMemberCallExpr>(isConstPointerAccessorMemberCall(), transferConstPointerAccessorMemberCall) .CaseOfCFGStmt<CXXOperatorCallExpr>( diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp index cdc870315016f..fd0c6f13c0031 100644 --- a/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp +++ b/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp @@ -3986,8 +3986,7 @@ TEST_P(UncheckedStatusOrAccessModelTest, PairIteratorRef) { }; void target() { if (auto it = Make<iterator>(); (*it).second.ok()) { - // This is a false positive. Fix and remove the unsafe. - (*it).second.value(); // [[unsafe]] + (*it).second.value(); } } )cc"); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
