[PATCH] D155446: [clang][dataflow] Eliminate duplication between `AggregateStorageLocation` and `StructValue`.

[Martin Böhme via Phabricator via cfe-commits]

mboehme marked 3 inline comments as done.
mboehme added inline comments.


================
Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:466
+      return;
+    Env.setStorageLocationStrict(*S, *MemberLoc);
   }
----------------
ymandel wrote:
> This diff makes me very happy. :)
Me too! :)


================
Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:496
+                     Env.getValue(*Arg, SkipPast::Reference))) {
+        if (auto *Val = cast<StructValue>(Env.createValue(S->getType()))) {
+          Env.setValueStrict(*S, *Val);
----------------
ymandel wrote:
> I thought `cast` requires a nonnull argument, so this will always be true?
Ah, good catch! Yes, the condition will always be true, so the if statement is superfluous. (We know `S->getType()` is a record type, and `Env.createValue()` will always create a value for record types.)

Removed the if statement.

(Confusingly, this comment now looks as if it's anchored to the if statement that does the `Env.getValue()`. The `cast_or_null` there _is_ necessary because we might not have a value for the argument.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155446