[PATCH] D32309: [libcxx] [test] Resolve compiler warnings in LCM/GCD tests

[Eric Fiselier via Phabricator via cfe-commits]

EricWF accepted this revision.
EricWF added a comment.
This revision is now accepted and ready to land.

LGTM, preferably with the suggested cleanups.



================
Comment at: test/std/numerics/numeric.ops/numeric.ops.gcd/gcd.pass.cpp:146
+        auto res = std::gcd(static_cast<int64_t>(1234), INT32_MIN);
+        static_assert(std::is_same<decltype(res), std::common_type_t<int64_t, int32_t>>::value, "");
         assert(res == 2);
----------------
BillyONeal wrote:
> EricWF wrote:
> > `std::common_type` here please. This test runs in C++03 where we don't have `std::foo_t`.
> C++17 library features in C++03?!? :(
> 
> OK, will fix.
> 
Sorry my bad. This test is truely C++17 only. I missed the `// UNSUPPORTED` line originally. Feel free to run wild with C++17 features.


================
Comment at: test/std/numerics/numeric.ops/numeric.ops.gcd/gcd.pass.cpp:42
+constexpr bool test0(int in1, int in2, int out)
 {
+    static_assert(std::is_same<Output, decltype(
----------------
Nit but this seems much cleaner and more readable without the casts.

```
auto value1 = static_cast<Input1>(in1);
auto value2 = static_cast<Input2>(in2);
static_assert(std::is_same_v<Output, decltype(std::gcd(value1, value2))>, "");
static_assert(std::is_same_v<Output, decltype(std::gcd(value2, value1))>, "");
assert(static_cast<Output>(out) == std::gcd(value1, value2));
return true;
```

Also just use `assert` instead of `std::abort()`


================
Comment at: test/std/numerics/numeric.ops/numeric.ops.lcm/lcm.pass.cpp:41
 {
-    static_assert((std::is_same<Output, decltype(std::lcm(Input1(0), Input2(0)))>::value), "" );
-    static_assert((std::is_same<Output, decltype(std::lcm(Input2(0), Input1(0)))>::value), "" );
-    return out == std::lcm(in1, in2) ? true : (std::abort(), false);
+    static_assert(std::is_same<Output, decltype(
+        std::lcm(static_cast<Input1>(0), static_cast<Input2>(0)))>::value, "");
----------------
Same suggestion as on the above test.


https://reviews.llvm.org/D32309