[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

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:18886
+          InitVal.getActiveBits() ? InitVal.getActiveBits() : 1;
+      NumPositiveBits = std::max(NumPositiveBits, ActiveBits);
+    } else
----------------
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?


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


================
Comment at: compiler-rt/test/ubsan/TestCases/Misc/enum.cpp:10
+enum class EBool : bool { a = 1 } e3;
+enum EEmpty {};
 
----------------
can you do a test on:

`enum ENeg { a = -1 }`
?

That should ALSO have only a single bit.


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

https://reviews.llvm.org/D130301