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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
