mboehme marked 2 inline comments as done.
mboehme added inline comments.
================
Comment at:
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:545
+ &cast<AggregateStorageLocation>(State.Env.createStorageLocation(*E));
+ State.Env.setStorageLocationStrict(*E, *Loc);
+ }
----------------
ymandel wrote:
> Should this line instead be outside of this `if`? So that the flow is
> ```
> Loc = get
> if (!Loc) Loc = create
> set(Loc)
> ```
I don't understand -- wouldn't this just be doing unnecessary work?
If
```
Loc = cast_or_null<AggregateStorageLocation>(
State.Env.getStorageLocationStrict(*E));
```
returns a non-null `Loc`, we don't create a new `Loc`, hence
```
State.Env.setStorageLocationStrict(*E, *Loc);
```
is a no-op, as `E` is already associated with `Loc`.
================
Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:473-474
if (S->isElidable()) {
- Env.setStorageLocation(*S, *ArgLoc);
- } else if (auto *ArgVal = cast_or_null<StructValue>(Env.getValue(*Arg)))
{
+ if (Value *Val = Env.getValue(*ArgLoc))
+ Env.setValueStrict(*S, *Val);
+ } else {
----------------
ymandel wrote:
> What happens otherwise? I gather that `S` just isn't associated with anything?
Correct. The idea being that if `S` is elidable, i.e. just a pass-through, we
want to pass through any `StructValue` that we got, and so if we didn't get a
`StructValue`, then we shouldn't pass one through.
I guess we could also just create a `StructValue` from scratch and associate it
with `ArgLoc`, but I'm not sure that it makes much of a difference in practice.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D156672/new/
https://reviews.llvm.org/D156672
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
