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