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

Eric Fiselier via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon May 8 14:28:34 PDT 2017

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.


More information about the cfe-commits mailing list