[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
Mon Jun 13 19:17:26 PDT 2022


ymandel accepted this revision.
ymandel added a comment.
This revision is now accepted and ready to land.

I love the tests (pretty cool examples of what can be handled). Thanks for the thoroughness!



================
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);
----------------
I believe you can relax this to `Value` because `setProperty` is no longer specific to `StructValue`.


================
Comment at: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:289-291
+  auto *OptionalVal = dyn_cast_or_null<StructValue>(&Val);
+  if (OptionalVal == nullptr)
+    return false;
----------------
I think this is no longer necessary now that we don't rely on `StructValue` for the `getProperty` call.


================
Comment at: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:696
+  }
+  setHasValue(*cast<StructValue>(&MergedVal), HasValueVal);
+  return true;
----------------
`MergedVal` is enough here, assuming the changes suggested above.


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