[PATCH] D10018: C11 _Bool bitfield diagnostic

[Richard Smith via cfe-commits]

On Thu, Sep 10, 2015 at 2:17 PM, Rachel Craik <rcraik at ca.ibm.com> wrote:

> rcraik added inline comments.
>
> ================
> Comment at: lib/Sema/SemaDecl.cpp:12586
> @@ -12585,3 +12585,3 @@
>    if (!FieldTy->isDependentType()) {
>      uint64_t TypeSize = Context.getTypeSize(FieldTy);
>      if (Value.getZExtValue() > TypeSize) {
> ----------------
> hubert.reinterpretcast wrote:
> > rsmith wrote:
> > > I think the right way to fix this is to call `getIntWidth` here
> instead of `getTypeSize`, and finesse our error message to clarify that
> we're talking about the width of the type (the number of value bits) rather
> than the size of the type (the number of storage bits).
> > The implementation of `getIntWidth` currently makes this consideration
> moot at this time, but should this extend to C89 (aside from the `_Bool`
> extension)?
> I think we have three options (the special case for _Bool bitfields being
> removed in each case):
>   # change `getTypeSize` to `getIntWidth` and leave the rest of the checks
> as-is
>   # change `getTypeSize` to `getIntWidth` and update the C/MS diagnostic
> to either `ExtWarn` or `Warning`  (for some or all language levels)
>   # leave as `getTypeSize` for lower language levels
>
> Opinions?


All of our standard modes are "language standard plus all applicable defect
reports", and the relevant constraint was added by C DR262, so it should
apply in C89 mode too, and we should pick either the first option or the
second option with an `ExtWarn`. (But this is academic, because our C89
mode does not support any integral types with padding bits other than
`_Bool`.)

I have no particular preference between the first and second (`ExtWarn`)
options. This seems like a completely reasonable extension, but we
shouldn't add an extension here unless there's actually some benefit from
doing so (some existing nontrivial codebase relying on bool bitfields with
width > 1).
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150910/383431c6/attachment.html>