[PATCH] D47953: [builtin] Add bitfield support for __builtin_dump_struct

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: echristo, rsmith, dblaikie.
aaron.ballman added a comment.

In https://reviews.llvm.org/D47953#1137375, @paulsemel wrote:

> I will for sure add tests @lebedev.ri . Fact is that, for the moment, this is not working as expected.
>  This is why I am asking for a bit of help about this bitfield handling :)


Adding a few more reviewers to see if they have ideas on the bitfield handling. Can you describe a bit more about what's incorrect with it when the structures are packed? Code examples might help.



================
Comment at: lib/CodeGen/CGBuiltin.cpp:1250
+      if (Info.IsSigned) {
+        unsigned HighBits = Info.StorageSize - Info.Offset - Info.Size;
+        if (HighBits)
----------------
paulsemel wrote:
> aaron.ballman wrote:
> > What happens if this overflows due to being < 0?
> How could this be < 0 ?
> If it is, it means that there was a problem during bitfield construction isn't it ?
> I mean StorageSize is always greater that Offset + Size
I don't think it should happen in the normal course of things. I'd probably just put in an assert to ensure that the high bit is never set in `HighBits` unless it's also set in `StorageSize`.


Repository:
  rC Clang

https://reviews.llvm.org/D47953