erichkeane added inline comments.
================ Comment at: clang/lib/AST/ExprConstant.cpp:13533 + + if (NumNegativeBits) { + unsigned NumBits = std::max(NumNegativeBits, NumPositiveBits + 1); ---------------- erichkeane wrote: > So my reading of 'within the range of the enumeration values' is different > here. Given: > > `enum F { -4, 4};`, the valid values for assignment I would say are -4, -3, > -2, -1, 0, 1, 2, 3, 4. This error seems to be a bit more permissive here by > making sure it is represent-able by the number of bits required to represent > the enum. > > Aaron pointed out off-line that I'm incorrect in what 'range of enumeration values' means, and that the values comes from https://eel.is/c++draft/dcl.enum#8.sentence-2. So the logic here needs to be to find the smallest integer (regardless of the power-of-2-ness of its bit-size) that can represent all of the values (including a 1 bit value for empty I think?), and diagnose based on that. ================ Comment at: clang/test/SemaCXX/constant-expression-cxx2a.cpp:1477 + +enum E { e1=-4, e2=4}; +void testValueInRangeOfEnumerationValues() { ---------------- erichkeane wrote: > Theres likely quite a few more tests that need to be done. We should have > one where it validates against a fixed underlying type, AND probably a pair > of ones to check against scoped enums. > > I wouldn't expect a scoped enum to diagnose, but a test to make sure we're > checking the right thing is likely valuable. Another good test is the empty-enum case. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130058/new/ https://reviews.llvm.org/D130058 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits