[PATCH] D140696: [clang][dataflow] Treat unions as structs.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

In D140696#4019810 <https://reviews.llvm.org/D140696#4019810>, @gribozavr2 wrote:

>> With this change, unions are modeled exactly as structs (as far as I can tell), which is unsound.
>
> Modeling just the storage of the union (but not the value) is sound. The first revision of the patch was a targeted fix that allowed us to continue maintaining the invariant that `this` is always modeled with a non-null `StorageLocation`.
>
> About modeling values of unions as structs (and members of unions as struct members), I can think of some unsoundness but nothing that affects what we are modeling now (e.g., pointers to two struct members should compare unequal, but pointers to two union members should compare equal). If the code writes to one union member, but reads from the other, code has UB. So it generally shouldn't be a problem, for the purpose of modeling, to model more storage in the union that a program free of union-related UB should never use. WDYT?

Good point about UB! So, we can drop the soundness concern. That leaves the "this is a big change" concern. I'm ok with it, but I'd prefer it wait for a thorough test (running over a large variety of TUs). That said, you two can decide how best to proceed -- I don't want to block your progress.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140696