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

Reply via email to