tbaeder marked an inline comment as done. tbaeder added inline comments.
================ Comment at: clang/test/AST/Interp/literals.cpp:161 + // expected-note {{division by zero}} + + ---------------- aaron.ballman wrote: > tbaeder wrote: > > aaron.ballman wrote: > > > tbaeder wrote: > > > > aaron.ballman wrote: > > > > > Same question here as with div in regards to testing float behavior. > > > > > (If you don't intend to support floats yet, make sure the commit > > > > > message makes that clear.) > > > > Ultimately sure, but as of right now, floats aren't supported at all, > > > > so nope. > > > Okay, fine by me. > > > > > > In terms of the request by @shafik for `INT_MIN % -1`, keep in mind there > > > are arbitrary width integer types like `_BitInt`, so you should add test > > > coverage for code like: > > > ``` > > > constexpr _BitInt(7) Val = -64; > > > static_assert(Val % (_BitInt(7)-1, ""); > > > ``` > > > and similar for division. > > I just checked, `_BitInt` doesn't work at all; it bails out when creating > > the `Descriptor` for the variable. > > > > I already added the suggested note for the `INT_MIN` case in this patch, > > but I wasn't sure how to test is properly. Should I just use a fixed-width > > integer type and copy its `_MIN` value in the test? > Ah, well that's something we should fix! :-D > > > I already added the suggested note for the INT_MIN case in this patch, but > > I wasn't sure how to test is properly. Should I just use a fixed-width > > integer type and copy its _MIN value in the test? > > You should be able to use: > ``` > #define INT_MIN (~__INT_MAX__) > ``` > (and similar for the other MIN macros) which relies on the predefined macro > instead of something from a header. Done, thanks. But I guess this now depends on https://reviews.llvm.org/D134804 since it uses `~` haha CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134744/new/ https://reviews.llvm.org/D134744 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits