li.zhe.hua added inline comments.
================
Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:230
+ ///
+ /// Requirements:
+ ///
----------------
sgatev wrote:
> Why add these as requirements instead of skipping past `ExprWithCleanups`
> internally, as we currently do for parens? Should we add similar requirements
> to `createStorageLocation`, `setStorageLocation`, and `getStorageLocation`?
> Would that result in lots of client code sprinkled with `IgnoreParens` and
> some form of `ignoreExprWithCleanups`?
Ah, I missed that we do this automatically for parens, in part because we
//do// unnecessarily sprinkle `IgnoreParens` in a lot of places in
`Transfer.cpp`.
That said, we can't avoid sprinkling them at least in some places for the time
being. Fixing this greatly expands beyond the scope of fixing a `cast` assert
firing. (That is still, aspirationally, the goal of this patch.)
Let me remove the `ParenExpr` requirement (that's incorrect), leave the
`ExprWithCleanups` requirement (with `FIXME`), and leave the holistic cleanup
for a separate patch.
================
Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:429-430
Value *Environment::getValue(const Expr &E, SkipPast SP) const {
+ assert(!isa<ParenExpr>(&E));
+ assert(!isa<ExprWithCleanups>(&E));
auto *Loc = getStorageLocation(E, SP);
----------------
sgatev wrote:
>
Acknowledged. Obviated by removing the `ParenExpr` assert.
================
Comment at:
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:224
+ if (auto *OptionalVal = cast_or_null<StructValue>(State.Env.getValue(
+ *ObjectExpr->IgnoreParens(), SkipPast::ReferenceThenPointer))) {
auto *HasValueVal = getHasValue(OptionalVal);
----------------
sgatev wrote:
> Do we have a test that fails if `IgnoreParens` is missing? If not, let's add
> a test or extend one of the existing. Perhaps
> https://github.com/llvm/llvm-project/blob/main/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp#L1286
I missed that we ignore parens automatically in `getValue`. Removing this.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124807/new/
https://reviews.llvm.org/D124807
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
