sdesmalen added inline comments.

================
Comment at: clang/include/clang/AST/Type.h:3956
     /// [implimits] 8 bits would be enough here.
-    unsigned NumExceptionType = 0;
+    unsigned NumExceptionType : 16;
+
----------------
erichkeane wrote:
> sdesmalen wrote:
> > erichkeane wrote:
> > > tbaeder wrote:
> > > > Isn't using a `uint16_t` preferred over a bitfield?
> > > Ah, thats a great point, yes, I think that is probably better here!  I 
> > > like the assert on setting it (particularly because we're shrinking it!), 
> > > but that idea makes this patch much smaller.
> > The reason for making this a bitfield rather than uint16_t is because 
> > D127762 extends the struct with another (bit)field:
> > 
> >     /// Any AArch64 SME ACLE type attributes that need to be propagated
> >     /// on declarations and function pointers.
> >     unsigned AArch64SMEAttributes : 6;
> > 
> > I thought having this all be bitfields made more sense, but I'm happy to 
> > change it to a uint16_t if that is preferred.
> > 
> > > that idea makes this patch much smaller
> > I suspect that the assert is still required to ensure that the value stored 
> > fits the value? Using `uint16_t` doesn't change that, or did I miss 
> > something? :)
> We don't need them all to be bitfields, they'll take up the same amount of 
> space with out it, and save us a bit of oddity.
> 
> I like the assert still as well, but since this size is set in 1 place, an 
> assert there would be fine as well.
Okay that makes sense, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152140

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to