[PATCH] D125931: [clang][dataflow] Add support for correlated branches to optional model

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun accepted this revision.
xazax.hun added inline comments.


================
Comment at: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:288
 
+bool isEngagedOptionalValue(const Value &OptionalVal, const Environment &env) {
+  auto *HasValueVal =
----------------
I wonder whether `NonEmpty` is clearer than `Engaged`. I think `Engaged` can also be OK, but we probably want to have a comment somewhere to explain the terminology.


================
Comment at: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:689
+  }
+  setHasValue(MergedVal, HasValueVal);
+  return true;
----------------
Would it make sense to have the converese? I.e., when both optionals are empty, create an empty optional here. While currently this modeling does not distinguish between unchecked and empty optionals it might be handy in the future. Although I am also happy with this as is, not adding any code that does not have any utility as of today.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125931