[PATCH] D134859: [clang][Interp] Implement basic support for floating point values

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added inline comments.


================
Comment at: clang/lib/AST/Interp/Boolean.h:59
   explicit operator bool() const { return V; }
+  explicit operator double() const { return V; }
 
----------------
Is there any reason why `operator double` exists, but `operator float` does not?


================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:144
+
+    if (!this->emitCast(*FromT, PT_Float, CE))
+      return false;
----------------
Does this two-stage conversion make sense? In contrast to things like `PT_Sint8` `PT_Float` is not a real type, it designates a set (potentially open) of all floating-point types. What is the meaning of this conversion? Why `emitCastFP` is not enough?

BTW which classes implement `emitCast` and `emitCastFP`? Usually such casts depend on rounding mode, how these methods get it?


================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:479
+  case PT_Float:
+    return this->emitZeroFloat(E);
   }
----------------
Should this method get floating-point semantic as a parameter?


================
Comment at: clang/lib/AST/Interp/Floating.h:37
+  /// Zero-initializes a Floating.
+  Floating() : F(0.0f) {}
+  Floating(APFloat F) : F(F) {}
----------------
This constructor creates a value of particular floating-point type, which generally is not compatible with floating-point values of other types. Does it need a semantic argument?


================
Comment at: clang/lib/AST/Interp/Floating.h:44
+  }
+  static Floating zero() { return Floating(0.0f); }
+
----------------
This method requires semantic just as getInf.


================
Comment at: clang/lib/AST/Interp/Floating.h:54-62
+  explicit operator int8_t() const { return toAPSInt().getExtValue(); }
+  explicit operator uint8_t() const { return toAPSInt().getExtValue(); }
+  explicit operator int16_t() const { return toAPSInt().getExtValue(); }
+  explicit operator uint16_t() const { return toAPSInt().getExtValue(); }
+  explicit operator int32_t() const { return toAPSInt().getExtValue(); }
+  explicit operator uint32_t() const { return toAPSInt().getExtValue(); }
+  explicit operator int64_t() const { return toAPSInt().getExtValue(); }
----------------
Conversions to integers are bitcasts+truncation but the conversion to bool is different. What semantics have these operations?


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

https://reviews.llvm.org/D134859