<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Sep 10, 2015 at 2:17 PM, Rachel Craik <span dir="ltr"><<a href="mailto:rcraik@ca.ibm.com" target="_blank">rcraik@ca.ibm.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">rcraik added inline comments.<br>
<span class=""><br>
================<br>
Comment at: lib/Sema/SemaDecl.cpp:12586<br>
@@ -12585,3 +12585,3 @@<br>
   if (!FieldTy->isDependentType()) {<br>
     uint64_t TypeSize = Context.getTypeSize(FieldTy);<br>
     if (Value.getZExtValue() > TypeSize) {<br>
----------------<br>
</span><span class="">hubert.reinterpretcast wrote:<br>
> rsmith wrote:<br>
> > 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).<br>
> The implementation of `getIntWidth` currently makes this consideration moot at this time, but should this extend to C89 (aside from the `_Bool` extension)?<br>
</span>I think we have three options (the special case for _Bool bitfields being removed in each case):<br>
  # change `getTypeSize` to `getIntWidth` and leave the rest of the checks as-is<br>
  # change `getTypeSize` to `getIntWidth` and update the C/MS diagnostic to either `ExtWarn` or `Warning`  (for some or all language levels)<br>
  # leave as `getTypeSize` for lower language levels<br>
<br>
Opinions?</blockquote><div><br></div><div>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`.)</div><div><br></div><div>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).</div></div></div></div>