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

[Yitzhak Mandelbaum 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()) {
----------------
ymand wrote:

incidentally -- these type checks are probably cheaper than isSupportedOptionalType, so I wonder if it would make sense to put them first in the function (though you'd need to rule out isGLValue). That would also allow you to factor out the check of `DirectCallee == nullptr` from the two other branches.

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