erichkeane added a comment.

This likely also needs a Release Note.



================
Comment at: clang/lib/AST/ASTContext.cpp:2697
   int64_t FieldSizeInBits =
       Context.toBits(Context.getTypeSizeInChars(Field->getType()));
   if (Field->isBitField()) {
----------------
MitalAshok wrote:
> erichkeane wrote:
> > This answer ends up being wrong in the case of _BitInt, consider 
> > _BitInt(7).  Its field-size would be 7, yet this would result in 8.  I'm 
> > not sure of the fallout of this though.
> The field size should be 8 for _BitInt(7), since it takes 8 bits in the 
> struct (including the padding bit). It is filtered out later with 
> "hasUniqueObjectRepresentations" on line 2709 if it wasn't in a bit field
I see, thanks for the explanation.


================
Comment at: clang/test/SemaCXX/has_unique_object_reps_bitint.cpp:7
+static_assert(__has_unique_object_representations(_BitInt(sizeof(int) * 8u)));
+static_assert(sizeof(_BitInt(24)) != 4 || 
!__has_unique_object_representations(_BitInt(24)));
+
----------------
MitalAshok wrote:
> erichkeane wrote:
> > Whats going on here?  I don't particularly get the condition.
> sizeof(_BitInt(24)) == sizeof(int32_t) (4) since we align to the next larger 
> builtin integer type. But if that weren't the case for some reason (i.e., 
> that changes in the future, I don't know if _BitInt is ABI stable), this 
> wouldn't start failing for unrelated reasons
I see!  Can you clarify that with a comment here?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125802

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

Reply via email to