[clang] 356ca40 - [FlowSensitive] [StatusOr] cache return values for all accessors

[Florian Mayer via cfe-commits]

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");