[libcxx-commits] [PATCH] D75960: [libc++] Implement C++20's P0476r2: std::bit_cast

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a subscriber: erik.pilkington.
ldionne added inline comments.


================
Comment at: libcxx/include/bit:366
+_LIBCPP_INLINE_VISIBILITY
+constexpr _ToType bit_cast(_FromType const& __from) noexcept
+{
----------------
EricWF wrote:
> We should probably mark this nodiscard. 
Marked it with `_LIBCPP_NODISCARD_EXT`, which we use for nodiscard-as-an-extension.


================
Comment at: libcxx/include/bit:368
+{
+    static_assert(sizeof(_ToType) == sizeof(_FromType));
+    static_assert(is_trivially_copyable_v<_ToType>);
----------------
Question: the spec has these as `Constraints:` -- I think it means these should be used to SFINAE out, not hard-error -- right?


================
Comment at: libcxx/test/std/numerics/bit/bit.cast/bit_cast.pass.cpp:24
+    // TODO fun stuff
+    return true;
+}
----------------
EricWF wrote:
> What's the fun stuff?
Writing the tests.


================
Comment at: libcxx/test/std/numerics/bit/bit.cast/bit_cast.pass.cpp:153
+
+// For some reason, __builtin_memcmp of the floating point types doesn't appear
+// to work inside constexpr.
----------------
@erik.pilkington Any idea why those don't work inside constexpr?


================
Comment at: libcxx/test/std/numerics/bit/bit.cast/bit_cast.pass.cpp:196
+    };
+    for (long double d : long_doubles) {
+        test_nested_roundtrip(d);
----------------
@erik.pilkington All the tests for `long double` are failing (at runtime) -- any idea why? Did you implement support for long double in `__builtin_bit_cast`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75960