[libcxx-commits] [PATCH] D79555: [libc++] [P0415] [C++20] Constexpr for std::complex.

[Marek Kurdej via Phabricator via libcxx-commits]

curdeius planned changes to this revision.
curdeius marked an inline comment as done.
curdeius added inline comments.


================
Comment at: libcxx/include/complex:586-615
+// #if _LIBCPP_STD_VER > 17
+template <class _Tp>
+_LIBCPP_CONSTEXPR_AFTER_CXX17 _Tp __copysign_constexpr(_Tp __lcpp_x, _Tp __lcpp_y);
+
+template <>
+inline _LIBCPP_INLINE_VISIBILITY
+_LIBCPP_CONSTEXPR_AFTER_CXX17 float __copysign_constexpr(float __lcpp_x, float __lcpp_y)
----------------
I guess I need to do the same `#if` dance as below, so as not to change the current runtime behaviour.


================
Comment at: libcxx/include/complex:601
+{
+    return __builtin_copysignl(__lcpp_x, __lcpp_y);
+}
----------------
Oops. Should be `__builtin_copysign` (without `l`).


================
Comment at: libcxx/include/complex:727
 }
+// #endif
 
----------------
Oops. Will remove in next update.


================
Comment at: libcxx/include/complex:921
+complex<_Tp>
+operator/(const complex<_Tp>& __z, const complex<_Tp>& __w)
+{
----------------
ldionne wrote:
> I think we really need to unfork these implementations and use the same one in all standard modes. It looks like the only difference is pretty much that you're calling the `_constexpr` variations when `is_constant_evaluated()` in the C++20 version. Would it be possible to always call e.g. `__libcpp_scalbn`, and then have that function check for `is_constant_evaluated()` and use a constexpr-friendly implementation in that case?
Done. The solution is IMO still a bit ugly (with those #ifdefs), but it's much better than before.
Thanks for the suggestion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79555