[clang] [clang][dataflow] Add test for crash repro and clean up const accessor handling (PR #129930)

[Jan Voung via cfe-commits]

================
@@ -577,57 +580,62 @@ void handleConstMemberCall(const CallExpr *CE,
       auto &ResultLoc = State.Env.getResultObjectLocation(*CE);
       copyRecord(cast<RecordStorageLocation>(Loc), ResultLoc, State.Env);
     }
-    return;
+    return true;
   }
 
   // Cache if the const method returns a reference
-  if (RecordLoc != nullptr && CE->isGLValue()) {
+  if (CE->isGLValue()) {
     const FunctionDecl *DirectCallee = CE->getDirectCallee();
     if (DirectCallee == nullptr)
-      return;
+      return false;
 
     StorageLocation &Loc =
         State.Lattice.getOrCreateConstMethodReturnStorageLocation(
             *RecordLoc, DirectCallee, State.Env, [&](StorageLocation &Loc) {
               // no-op
+              // NOTE: if we want to support const ref to pointers or bools
+              // we should initialize their values here.
             });
 
     State.Env.setStorageLocation(*CE, Loc);
-    return;
-  }
-
-  // Cache if the const method returns a boolean or pointer type.
-  // We may decide to cache other return types in the future.
-  if (RecordLoc != nullptr &&
-      (CE->getType()->isBooleanType() || CE->getType()->isPointerType())) {
+    return true;
+  } else if (CE->getType()->isBooleanType() || CE->getType()->isPointerType()) {
+    // Cache if the const method returns a boolean or pointer type.
     Value *Val = State.Lattice.getOrCreateConstMethodReturnValue(*RecordLoc, CE,
                                                                  State.Env);
     if (Val == nullptr)
-      return;
+      return false;
     State.Env.setValue(*CE, *Val);
-    return;
+    return true;
   }
 
-  // Perform default handling if the call returns an optional
-  // but wasn't handled above (if RecordLoc is nullptr).
-  if (isSupportedOptionalType(CE->getType())) {
-    transferCallReturningOptional(CE, Result, State);
-  }
+  return false;
 }
 
-void transferValue_ConstMemberCall(const CXXMemberCallExpr *MCE,
-                                   const MatchFinder::MatchResult &Result,
-                                   LatticeTransferState &State) {
-  handleConstMemberCall(
-      MCE, dataflow::getImplicitObjectLocation(*MCE, State.Env), Result, State);
+void transferConstMemberCall(const CXXMemberCallExpr *MCE,
+                             const MatchFinder::MatchResult &Result,
+                             LatticeTransferState &State) {
+  if (!handleConstMemberCall(
+          MCE, dataflow::getImplicitObjectLocation(*MCE, State.Env), Result,
+          State)) {
+    // Perform default handling if the call returns an optional,
+    // but wasn't handled.
+    if (isSupportedOptionalType(MCE->getType()))
+      transferCallReturningOptional(MCE, Result, State);
----------------
jvoung wrote:

I was trying not to repeat the default handling too much (in the return false cases of handleConstMemberCall -- or have a lambda or something that I call), but consolidated to one place now that is shared w/ these two transfer functions.

https://github.com/llvm/llvm-project/pull/129930