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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG8fcdd625856b: [clang][dataflow] Add support for correlated branches to optional model (authored by sgatev). Repository: rG LLVM Github Monorepo

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

sgatev added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:288 +bool isEngagedOptionalValue(const Value , const Environment ) { + auto *HasValueVal = xazax.hun wrote: > I wonder whether `NonEmpty` is

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

sgatev updated this revision to Diff 437088. sgatev marked 3 inline comments as done. sgatev added a comment. Herald added a subscriber: martong. Address comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125931/new/

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

xazax.hun accepted this revision. xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:288 +bool isEngagedOptionalValue(const Value , const Environment ) { + auto *HasValueVal = I wonder

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

ymandel added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:174 +/// property of the optional value `OptionalVal`. +void setHasValue(StructValue , BoolValue ) { + OptionalVal.setProperty("has_value", HasValueVal);

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

sgatev added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:174 +/// property of the optional value `OptionalVal`. +void setHasValue(StructValue , BoolValue ) { + OptionalVal.setProperty("has_value", HasValueVal);

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

sgatev updated this revision to Diff 436710. sgatev marked 4 inline comments as done. sgatev added a comment. Address comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125931/new/ https://reviews.llvm.org/D125931 Files:

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

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:

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

sgatev marked 3 inline comments as done. sgatev added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:596-598 + MergedEnv.makeOr( + MergedEnv.makeAnd(Env1.getFlowConditionToken(), *HasValueVal1), +

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

sgatev updated this revision to Diff 436590. sgatev added a comment. Rebase main. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125931/new/ https://reviews.llvm.org/D125931 Files:

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

xazax.hun added a comment. In D125931#3527584 , @ymandel wrote: > 1. In this patch, we go with a widening operation, but put the relevant logic > in the core, so it can be reused for booleans in general. Depending on how much simplicity are we willing

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

ymandel added a comment. In D125931#3525699 , @xazax.hun wrote: > Actually, I think in most cases we want to be consistent on how to merge bool > values. So I wonder whether instead of reimplementing the merge operation in > this check we should just

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

xazax.hun added a comment. Actually, I think in most cases we want to consistent how to merge bool values. So I wonder whether instead of reimplementing the merge operation in this check we should just call a function that does the work. And the same function should be used within the engine

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

xazax.hun added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:596-598 + MergedEnv.makeOr( + MergedEnv.makeAnd(Env1.getFlowConditionToken(), *HasValueVal1), +

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

ymandel added a comment. Thanks! Looks good overall, just the one question about loops. Comment at: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:596-598 + MergedEnv.makeOr( + MergedEnv.makeAnd(Env1.getFlowConditionToken(),

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

sgatev created this revision. sgatev added reviewers: ymandel, xazax.hun, gribozavr2. Herald added subscribers: tschuett, steakhal, rnkovacs. Herald added a project: All. sgatev requested review of this revision. Herald added a project: clang. Add support for correlated branches to the