[PATCH] D157596: [clang][Interp] Handle mixed floating/integral compound assign operators

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

tbaeder marked an inline comment as done.
tbaeder added inline comments.


================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:2659-2664
+  if (FromT == PT_Float) {
+    // Floating to floating.
+    if (ToT == PT_Float) {
+      const llvm::fltSemantics *ToSem = &Ctx.getFloatSemantics(ToQT);
+      return this->emitCastFP(ToSem, getRoundingMode(E), E);
+    }
----------------
aaron.ballman wrote:
> Should we be early returning if we're casting from float->float like we do for int->int?
Heh, I was thinking the exact same thing when writing this, but 

a) That means we also have to pass a `QualTyp FromQT`
b) I don't think we'd ever run into this problem since the AST would have to contain such a cast from/to the same floating type.


================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:2667
+    // Float to integral.
+    if (isIntegralType(ToT) || ToT == PT_Bool)
+      return this->emitCastFloatingIntegral(ToT, E);
----------------
aaron.ballman wrote:
> It weirds me out that `isIntegralType()` returns false for a boolean type; that is an integral type as well: http://eel.is/c++draft/basic.types#basic.fundamental-11
I don't think there's a good reason for it to return `false` for bools; before this patch, the function is only used for an assertion in `Neg()`.


================
Comment at: clang/test/AST/Interp/floats.cpp:106
+
+  constexpr float intPlusDouble() {
+   int a = 0;
----------------
aaron.ballman wrote:
> Is it intentional that this is returning a float but then comparing below as an integer?
Kinda? What we compare against doesn't really matter for the compound assignment in the function, does it?


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

https://reviews.llvm.org/D157596