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

[Mark de Wever via Phabricator via libcxx-commits]

Mordante accepted this revision as: Mordante.
Mordante added a comment.

Thanks, a few minor points remaining. Otherwise then that LGTM!



================
Comment at: libcxx/test/std/numerics/c.math/cmath.pass.cpp:1177
 
+void test_lerp()
+{
----------------
Please make the test `constexpr` and test it that way.


================
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), "");
----------------
Quuxplusone wrote:
> 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. :))
I've no strong opinion whether `lerp` gets it's own file or becomes part of this test. I object against having the tests for `lerp` in two different files. If you want to keep it in this file, please make add the current `lerp` tests to this file in a separate commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116295