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

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik commandeered this revision.
philnik added a reviewer: curdeius.
philnik added a comment.
Herald added a project: All.

Commandeering to finish this, since it's been laying around for over a year and it seems like it's almost finished.



================
Comment at: libcxx/include/complex:585
 
+inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX17 float
+__copysign_constexpr(float __lcpp_x, float __lcpp_y) {
----------------
ldionne wrote:
> We already jump through some hoops in `<math.h>` to provide `copysign`. Would it make sense to try and unify those in some way? Maybe create a `__copysign` function that is `constexpr` whenever it can be, and then call that from `copysign`. Here, we'd call `__copysign` directly, which has the right constexpr-ness. WDYT? I'm trying to avoid proliferating helper functions that might be useful elsewhere in just this header, since it means we'll reinvent the wheel in the future eventually.
> 
> Also, I don't understand why we're guarding the use of the builtin on `_LIBCPP_STD_VER` and not `__has_builtin`.
> 
> Same comment applies to other general functions below like `fabs` & friends. I think it may be a good idea to tackle those as a separate patch, then this patch would be a lot simpler.
> We already jump through some hoops in `<math.h>` to provide `copysign`. Would it make sense to try and unify those in some way? Maybe create a `__copysign` function that is `constexpr` whenever it can be, and then call that from `copysign`. Here, we'd call `__copysign` directly, which has the right constexpr-ness. WDYT? I'm trying to avoid proliferating helper functions that might be useful elsewhere in just this header, since it means we'll reinvent the wheel in the future eventually.
> 
> Also, I don't understand why we're guarding the use of the builtin on `_LIBCPP_STD_VER` and not `__has_builtin`.
> 
> Same comment applies to other general functions below like `fabs` & friends. I think it may be a good idea to tackle those as a separate patch, then this patch would be a lot simpler.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79555