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

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

tbaeder marked 7 inline comments as done.
tbaeder added inline comments.


================
Comment at: clang/lib/AST/Interp/Integral.h:213
 
+  static bool rem(Integral A, Integral B, unsigned OpBits, Integral *R) {
+    *R = Integral(A.V % B.V);
----------------
erichkeane wrote:
> tbaeder wrote:
> > erichkeane wrote:
> > > Its a touch weird we return 'bool' in all these places, rather than an `Optional<Integral>`, right?  Perhaps a refactor idea for a future patch.
> > The really weird part is that `false` here suddenly means success, but I get your point. Does using `Optional` introduce any sort of performance penalty if this is being called a lot?
> 'false' as success is actually pretty common in clang, we use that all over the place. I don't believe Optional would have any negative penalty, at least compared to a pointer-param?
Yeah I know it's being used elsewhere in the codebase like this, but the all the AST visitors in the interpreter code return `false` for failure, so this is kinda confusing to me.

I'll note the refactoring for later.


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

https://reviews.llvm.org/D134744