[PATCH] D130301: [Clang] Fix how we set the NumPositiveBits on an E numDecl to cover the case of single enumerator with value zero or an empty enum

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:18886
+          InitVal.getActiveBits() ? InitVal.getActiveBits() : 1;
+      NumPositiveBits = std::max(NumPositiveBits, ActiveBits);
+    } else
----------------
erichkeane wrote:
> What about:
> 
> `std::max({NumPositiveBits, ActiveBits, 1})` (which uses the init-list version of std::max).
> 
> Also, not completely following this: BUT we don't need a positive bit IF we have a negative bit, right?  So this is only necessary if BOTH NumPositiveBits and NumNegativeBits would have been 0?
Good call on the `std::max` alternative, I completely forgot about it.

If we look at the description of `getNumPositiveBits()` in `Decl.h` it says:

> Returns the width in bits required to store all the non-negative enumerators of this enum.

So since `0` is non-negative we should update the positive bits. Make sense?


================
Comment at: clang/lib/Sema/SemaDecl.cpp:18888
+    } else
       NumNegativeBits = std::max(NumNegativeBits,
                                  (unsigned)InitVal.getMinSignedBits());
----------------
erichkeane wrote:
> By coding standard, we need curleys around the 'else' as well if we have it on the 'if'.
Good catch.


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

https://reviews.llvm.org/D130301