<div dir="auto"><div><div class="gmail_extra"><div class="gmail_quote">On 17 Nov 2016 8:56 am, "Reid Kleckner" <<a href="mailto:rnk@google.com">rnk@google.com</a>> wrote:<br type="attribution"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">rnk added a comment.<br>
<div class="quoted-text"><br>
In <a href="https://reviews.llvm.org/D24289#598169" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D24289#598169</a>, @rsmith wrote:<br>
<br>
> This is causing warnings to fire for headers shared between C and C++, where the "give the enum an unsigned underlying type" advice doesn't work, and where the code in question will never be built for the MS ABI. It seems really hard to justify this being on by default.<br>
><br>
> I'm going to turn it off by default for now, but we should probably consider turning it back on by default when targeting the MS ABI (as a "your code is wrong" warning rather than a "your code is not portable" warning).<br>
<br>
<br>
</div>Seems reasonable. I asked to make it off by default, but got argued down to putting it under -Wall.<br>
<div class="quoted-text"><br>
> Yeah, suggesting adding an underlying type to the enum to solve this problem seems like a bad idea, since that fundamentally changes the nature of the enum -- typically allowing it to store a lot more values, and making putting it in a bitfield a bad idea.<br>
<br>
</div>Any time you use a bitfield it stores fewer values than the original integer type. I don't see how enums are special here. Even if I can store values in the enum not representable as a bitwise combination of enumerators, people usually don't, and adding an underlying type doesn't change the conventions of normal C++ code.</blockquote></div></div></div><div dir="auto"><br></div><div dir="auto">The range of representable values for a bitfield with no fixed underlying type is actually smaller than that of its underlying type. See <a href="http://eel.is/c++draft/dcl.enum#8">http://eel.is/c++draft/dcl.enum#8</a></div><div dir="auto"><br></div><div dir="auto">So a bitfield of width equal to the number of bits needed to store any enumerator does not have fewer values than the original type.</div><div dir="auto"><br></div><div dir="auto"></div><div dir="auto"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Do you have any better suggestions for people that want this code to do the right thing when built with MSVC?<br>
<br>
  enum E /* : unsigned */ { E0, E1, E2, E3 };<br>
  struct A { E b : 2; };<br>
  int main() {<br>
    A a;<br>
    a.b = E3;<br>
    return a.b; // comes out as -1 without the underlying type<br>
  }<br>
<br>
Widening the bitfield wastes a bit. Making the bitfield a plain integer and cast in and out of the enum type, but that obscures the true type of the bitfield. So, I still support this suggestion.</blockquote></div></div></div><div dir="auto"><br></div><div dir="auto">How about we consider msvc's behaviour to just be a bug, and zero-extend loads from enum bitfields with no negative enumerators? Since loading such a negative value results in UB, this wouldn't change the meaning of any program with defined behaviour, and still respects the MS ABI.</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="elided-text">
Repository:<br>
  rL LLVM<br>
<br>
<a href="https://reviews.llvm.org/D24289" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D24289</a><br>
<br>
<br>
<br>
</div></blockquote></div><br></div></div></div>