[clang] [clang][dataflow] Disallow setting properties on `RecordValue`s. (PR #76042)

[Dmitri Gribenko via cfe-commits]

================
@@ -636,40 +636,37 @@ class OptionalIntAnalysis final
     if (!CS)
       return;
     const Stmt *S = CS->getStmt();
-    auto OptionalIntRecordDecl = recordDecl(hasName("OptionalInt"));
-    auto HasOptionalIntType = hasType(OptionalIntRecordDecl);
-
-    SmallVector<BoundNodes, 1> Matches = match(
-        stmt(anyOf(cxxConstructExpr(HasOptionalIntType).bind("construct"),
-                   cxxOperatorCallExpr(
-                       callee(cxxMethodDecl(ofClass(OptionalIntRecordDecl))))
-                       .bind("operator"))),
-        *S, getASTContext());
-    if (const auto *E = selectFirst<CXXConstructExpr>(
-            "construct", Matches)) {
-      cast<RecordValue>(Env.getValue(*E))
-          ->setProperty("has_value", Env.getBoolLiteralValue(false));
-    } else if (const auto *E =
-                   selectFirst<CXXOperatorCallExpr>("operator", Matches)) {
-      assert(E->getNumArgs() > 0);
-      auto *Object = E->getArg(0);
-      assert(Object != nullptr);
-
-      refreshRecordValue(*Object, Env)
-          .setProperty("has_value", Env.getBoolLiteralValue(true));
+    const Expr *E = dyn_cast<Expr>(S);
+    if (!E)
+      return;
+
+    if (!E->getType()->isPointerType())
+      return;
+
+    // Make sure we have a `PointerValue` for `E`.
+    auto *PtrVal = cast_or_null<PointerValue>(Env.getValue(*E));
----------------
gribozavr wrote:

There are a lot of things that a poorly-written check could do, so is this the one to spend our time protecting against? Maybe. I wish the framework helped more with convergence. Convergence issues are indeed difficult, and we ourselves have fully solved the problem and haven't found a good tradeoff re: convergence for nullability. As we deploy this check internally, I expect us to tweak things in this area, hopefully we will get a better understanding, or even ideas for framework-level features. There is also non-trivial work we already know needs to be done that is convergence-related, like a generic "Top" value.

So I think at this point I'd prefer to keep the extensibility with sharp edges, rather than reduce expressivity.

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