erichkeane added inline comments.
================
Comment at: clang/lib/AST/ExprConstant.cpp:13527
+ llvm::APInt Min;
+ llvm::APInt End;
+
----------------
Why "End" instead of max?
================
Comment at: clang/lib/AST/ExprConstant.cpp:13533
+
+ if (NumNegativeBits) {
+ unsigned NumBits = std::max(NumNegativeBits, NumPositiveBits + 1);
----------------
shafik wrote:
> erichkeane wrote:
> > 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.
> So I used ubsan to validate my checks e.g. https://godbolt.org/z/1j43465K7
> but perhaps ubsan is getting it wrong as well?
Playing with that link, I THINK this is right? I couldn't get it to be the
'wrong' message there. It DID take a while to figure out what is going on here.
I DO like the idea of abstracting this into some sort of `InRangeOfValues`
thing on EnumDecl so that UBSan and this can share it.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130058/new/
https://reviews.llvm.org/D130058
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
