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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
