[PATCH] D156672: [clang][dataflow] Use `Strict` accessors where we weren't using them yet.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel accepted this revision.
ymandel added a comment.

SGTM



================
Comment at: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:545
+          &cast<AggregateStorageLocation>(State.Env.createStorageLocation(*E));
+      State.Env.setStorageLocationStrict(*E, *Loc);
+    }
----------------
mboehme wrote:
> 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`.
Right, I was not paying enough attention. I was thinking specifically about the value. basically, i confused line 545 for line 549.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156672