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

Yitzhak Mandelbaum via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jun 14 04:38:48 PDT 2022


ymandel added inline comments.


================
Comment at: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:174
+/// property of the optional value `OptionalVal`.
+void setHasValue(StructValue &OptionalVal, BoolValue &HasValueVal) {
+  OptionalVal.setProperty("has_value", HasValueVal);
----------------
sgatev wrote:
> ymandel wrote:
> > I believe you can relax this to `Value` because `setProperty` is no longer specific to `StructValue`.
> I did that intentionally because I still think it makes sense to assert that an optional is modeled as a `StructValue`. I haven't thought about where and how it'd be best to assert that though so I'll happily remove the casts for now.
Sounds good. I had the same thought. But, it occurs to me that at this point, are we using the `StructValue` at all? If not, then the only reason we expect it to be a `StructValue` is because we know the optional type is a record type. But, that's not an assumption of our model. So, maybe we should be agnostic to the underlying representation?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125931



More information about the cfe-commits mailing list