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

[Yitzhak Mandelbaum via cfe-commits]

================
@@ -551,91 +551,92 @@ void transferCallReturningOptional(const CallExpr *E,
   setHasValue(*Loc, State.Env.makeAtomicBoolValue(), State.Env);
 }
 
+// Returns true if the const accessor is handled by caching.
+// Returns false if we could not cache. We should perform default handling
+// in that case.
 bool handleConstMemberCall(const CallExpr *CE,
                            dataflow::RecordStorageLocation *RecordLoc,
                            const MatchFinder::MatchResult &Result,
                            LatticeTransferState &State) {
   if (RecordLoc == nullptr)
     return false;
 
-  // If the const method returns an optional or reference to an optional.
-  if (isSupportedOptionalType(CE->getType())) {
-    const FunctionDecl *DirectCallee = CE->getDirectCallee();
-    if (DirectCallee == nullptr)
-      return false;
-    StorageLocation &Loc =
-        State.Lattice.getOrCreateConstMethodReturnStorageLocation(
-            *RecordLoc, DirectCallee, State.Env, [&](StorageLocation &Loc) {
-              setHasValue(cast<RecordStorageLocation>(Loc),
-                          State.Env.makeAtomicBoolValue(), State.Env);
-            });
-    if (CE->isGLValue()) {
-      // If the call to the const method returns a reference to an optional,
-      // link the call expression to the cached StorageLocation.
-      State.Env.setStorageLocation(*CE, Loc);
-    } else {
-      // If the call to the const method returns an optional by value, we
-      // need to use CopyRecord to link the optional to the result object
-      // of the call expression.
-      auto &ResultLoc = State.Env.getResultObjectLocation(*CE);
-      copyRecord(cast<RecordStorageLocation>(Loc), ResultLoc, State.Env);
-    }
-    return true;
-  }
-
-  // Cache if the const method returns a reference
+  // Cache if the const method returns a reference.
   if (CE->isGLValue()) {
     const FunctionDecl *DirectCallee = CE->getDirectCallee();
     if (DirectCallee == nullptr)
       return false;
 
+    // Initialize the optional's "has_value" property to true if the type is
+    // optional, otherwise no-op. If we want to support const ref to pointers or
+    // bools we should initialize their values here too.
+    auto Init = [&](StorageLocation &Loc) {
+      if (isSupportedOptionalType(CE->getType()))
+        setHasValue(cast<RecordStorageLocation>(Loc),
+                    State.Env.makeAtomicBoolValue(), State.Env);
+    };
     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.
-            });
+            *RecordLoc, DirectCallee, State.Env, Init);
 
     State.Env.setStorageLocation(*CE, Loc);
     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 false;
-    State.Env.setValue(*CE, *Val);
-    return true;
+  } else {
----------------
ymand wrote:

nit: drop the `else` because the bloc ends in `return`?

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