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

[Dani Ferreira Franco Moura via Phabricator via cfe-commits]

merrymeerkat added a comment.

@ymandel - thank you for pointing all of this out! And no need to apologise at all :)

I agree that the changes made here are not ideal. But the alternative is also not ideal.

The question becomes: is it better to model unions in a way that is lacking, or to not model them at all?

To model unions properly, we would need to be able to set the storage location of all the members of a union to be the same. But storage location is tied to type, so we can't create the same storage location for members of different types. From what I can tell, correctly modelling unions would require us to do an overhaul of how storage locations are implemented.

Like Dmitri said, I think most of the issues with this simple approach to modelling unions would only be revealed with UB code. So, unless we want to compare the storage locations of pointers to different union members (which I don't think will be a very common use), this modelling should work all right for defined behaviour. I added a documentation comment regarding the lacking implementation of storage location.

Coming back to the question above. Yitzhak, you seem to prefer not modelling union at all rather than having this model that falls short. Dmitri seems to prefer a model that works on most cases than no model. I am a bit torn, but I think I'm leaning a bit on the side of having a model that falls short. I don't think we plan on doing the storage location overhaul anytime soon, and until then it'd be good to at least have some functionality.

Happy new year by the way!!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140696