[PATCH] D134744: [clang][Interp] Implement rem opcode

[Timm Bäder via Phabricator via cfe-commits]

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:
> > > 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?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134744/new/

https://reviews.llvm.org/D134744