[libcxx-commits] [PATCH] D116295: [libc++] Add missing templated version of `std::lerp`

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/include/cmath:639
 
+template <class _A1, class _A2, class _A3>
+inline _LIBCPP_HIDE_FROM_ABI
----------------
Mordante wrote:
> The function should be `constexpr`. Please make sure to also update the test.
Good catch! Updated.


================
Comment at: libcxx/test/std/numerics/c.math/cmath.pass.cpp:1140
     static_assert((std::is_same<decltype(std::hypot((float)0, (int)0, (long double)0)), long double>::value), "");
-    static_assert((std::is_same<decltype(std::hypot((float)0, (unsigned int)0)), double>::value), "");
     static_assert((std::is_same<decltype(std::hypot((float)0, (double)0, (long)0)), double>::value), "");
----------------
Quuxplusone wrote:
> jloser wrote:
> > Why remove this? Seems unrelated? 
> This is a duplicate of old line 1119, erroneously copied down here with the three-argument overload even though it's passing only two arguments. I discovered this error when I copied these lines to use with `lerp` (because `lerp` doesn't have a two-argument overload). I can put this in a separate git commit, for sure.
Merged this removal in eda5bbfb9db4d0dc3861333f7b4adf135fe20af8.


================
Comment at: libcxx/test/std/numerics/c.math/cmath.pass.cpp:1180
+#if TEST_STD_VER > 17
+    static_assert((std::is_same<decltype(std::lerp((float)0, (float)0, (float)0)), float>::value), "");
+    static_assert((std::is_same<decltype(std::lerp((float)0, (bool)0, (float)0)), double>::value), "");
----------------
Mordante wrote:
> Please remove the empty strings and use `std::is_same_v`.
> 
Both here and above, this is for consistency with everything else in this file. Trying to "do something different" by creating a new `c.math.lerp.pass.cpp` is how we ended up missing tests (and code) to begin with; mclow should have just copy-pasted all the stuff for `std::hypot` (as I did here) and then the template overload couldn't have been missed.
(Although, as you see, the `constexpr` can be missed if I copy from `hypot`-which-is-not-constexpr. :))


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116295