[PATCH] D122231: [clang][dataflow] Add support for `value_or` in a comparison.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added inline comments.


================
Comment at: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:275
+      State.Env.getValue(*ValueOrPredExpr, SkipPast::None));
+  if (ExprValue == nullptr) {
+    auto &ExprLoc = State.Env.createStorageLocation(*ValueOrPredExpr);
----------------
sgatev wrote:
> Why do this conditionally? I think we should set a value regardless of whether another model has already done so.
Why? I figured we're agnostic to the underlying value, and only care about relating it via the implication. We're setting it only so we have something to anchor that implication on.  If we always set it, then we're erasing the information from another model.


================
Comment at: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp:1244
               Ctx, UncheckedOptionalAccessModelOptions{
                        /*IgnoreSmartPointerDereference=*/true});
         },
----------------
Note: this came from sync'ing to HEAD and picking up my other patch.


================
Comment at: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp:1811-1812
+    void target() {
+      $ns::$optional<int> opt;
+      auto *opt_p = &opt;
+      if (opt_p->value_or(0) != 0) {
----------------
sgatev wrote:
> These can be combined in a `$ns::$optional<int> *opt` parameter.
Unfortunately, that crashes (which must be why I did this to begin with). But, I did reduce to only one var and one param.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122231/new/

https://reviews.llvm.org/D122231