[PATCH] D124221: Add new builtin __builtin_reflect_struct.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: clang/lib/Sema/SemaChecking.cpp:420
+    auto *FD = IFD ? IFD->getAnonField() : dyn_cast<FieldDecl>(D);
+    if (!FD || (FD->isUnnamedBitfield() || FD->isAnonymousStructOrUnion()))
+      continue;
----------------
rsmith wrote:
> erichkeane wrote:
> > rsmith wrote:
> > > erichkeane wrote:
> > > > Losing the unnamed bitfield is unfortunate, and I the 'dump struct' handles.  As is losing the anonymous struct/union.
> > > > 
> > > > Also, how does this handle 'record type' members?  Does the user have to know the inner type already?  If they already know that much information about the type, they wouldn't really need this, right?
> > > If `__builtin_dump_struct` includes unnamed bitfields, that's a bug in that builtin.
> > > 
> > > In general, the user function gets given the bases and members with the right types, so they can use an overload set or a template to dispatch based on that type. See the SemaCXX test for a basic example of that.
> > I thought it did?  For the use case I see `__builtin_dump_struct` having, I would think printing the existence of unnamed bitfields to be quite useful.
> > 
> > 
> Why? They're not part of the value, they're just padding.
Looks like `__builtin_dump_struct` currently includes them *and* prints a value (whatever garbage happens to be in those padding bits). That's obviously a bug.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124221