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

[BillyONeal]

Summary

Resolve integer overflow warnings in GCD and LCM tests.

lcm.pass.cpp:
19: Update headers to that actually used in the test.
41: test0 was triggering narrowing warnings for all callers, because the
inputs were always ints, but some of the explicit template arguments were
smaller than that. Instead, have this function accept ints and static_cast
explicitly to the types we want before calling std::lcm.
47: Replace unnecessary ternary.
55: Use foo_t instead of typename foo<>::type
111/116: intX_t were not std::qualified but only <cfoo> headers were included.
141: C1XX has a bug where it interprets 2147483648 as unsigned int. Then the
negation trips "negation of unsigned value, result still unsigned" warnings.
Perma-workaround this issue by saying INT_MIN, which better documents the
intended behavior and avoids triggering warnings on C1XX.

gcd.pass.cpp:
Same changes as lcm.pass.cpp but for GCD.

[BillyONeal]

Stephan had some code review comments :)

[rsmith]

test/std/numerics/numeric.ops/numeric.ops.gcd/gcd.pass.cpp

46

Is there a reason to recompute the type here rather than using Output?

[BillyONeal]

test/std/numerics/numeric.ops/numeric.ops.gcd/gcd.pass.cpp

46

Hmm... I think I was concerned about it looking like the cast o the output type was an assertion that the return type was in fact of that type (which isn't true). But that should be taken care of by the previous static_asserts.

[BillyONeal]

Hi folks, any update on this or is just fixing @rsmith's comment OK?

Thanks!

[CaseyCarter]

test/std/numerics/numeric.ops/numeric.ops.lcm/lcm.pass.cpp

144

INT_MIN is in <climits> .

[EricWF]

test/std/numerics/numeric.ops/numeric.ops.gcd/gcd.pass.cpp

146

std::common_type here please. This test runs in C++03 where we don't have std::foo_t.

[BillyONeal]

test/std/numerics/numeric.ops/numeric.ops.gcd/gcd.pass.cpp

146

C++17 library features in C++03?!? :(

OK, will fix.

[BillyONeal]

Resolved CR comments

[EricWF]

LGTM, preferably with the suggested cleanups.

test/std/numerics/numeric.ops/numeric.ops.gcd/gcd.pass.cpp

41–42

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()

146

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.

test/std/numerics/numeric.ops/numeric.ops.lcm/lcm.pass.cpp

40–50

Same suggestion as on the above test.

[BillyONeal]

test/std/numerics/numeric.ops/numeric.ops.gcd/gcd.pass.cpp

41–42

I was under the impression the abort dance was because assert isn't constexpr.

[EricWF]

test/std/numerics/numeric.ops/numeric.ops.gcd/gcd.pass.cpp

41–42

assert is constexpr for the exact same reason the abort dance was constexpr.

And I think somebody else was under the same misapprehension when they wrote this test.

[BillyONeal]

Do Eric's suggestions.

test/std/numerics/numeric.ops/numeric.ops.gcd/gcd.pass.cpp

 
 // <numeric>
 
 // template<class _M, class _N>
 // constexpr common_type_t<_M,_N> gcd(_M __m, _N __n)
 
 #include <numeric>
 #include <cassert>
+#include <cstdint>
 #include <cstdlib>    // for rand()
-#include <iostream>
+#include <type_traits>
 
 constexpr struct {
   int x;
   int y;
   int expect;
 } Cases[] = {
     {0, 0, 0},
     {1, 0, 1},
     {0, 1, 1},
     {1, 1, 1},
     {2, 3, 1},
     {2, 4, 2},
     {36, 17, 1},
     {36, 18, 18}
 };
 
 
 template <typename Input1, typename Input2, typename Output>
-constexpr bool test0(Input1 in1, Input2 in2, Output out)
+constexpr bool test0(int in1, int in2, int out)
 {
-    static_assert((std::is_same<Output, decltype(std::gcd(in1, in2))>::value), "" );
-    static_assert((std::is_same<Output, decltype(std::gcd(in2, in1))>::value), "" );
-    return out == std::gcd(in1, in2) ? true : (std::abort(), false);
+    static_assert(std::is_same<Output, decltype(
+        std::gcd(static_cast<Input1>(0), static_cast<Input2>(0)))>::value, "");
+    static_assert(std::is_same<Output, decltype(
+        std::gcd(static_cast<Input2>(0), static_cast<Input1>(0)))>::value, "");
+    const bool result = static_cast<std::common_type_t<Input1, Input2>>(out) ==
+        std::gcd(static_cast<Input1>(in1), static_cast<Input2>(in2));
+    if (!result) {
+        std::abort();
+    }
+
+    return result;
 }
 
 
 template <typename Input1, typename Input2 = Input1>
 constexpr bool do_test(int = 0)
 {
-    using S1 = typename std::make_signed<Input1>::type;
-    using S2 = typename std::make_signed<Input2>::type;
-    using U1 = typename std::make_unsigned<Input1>::type;
-    using U2 = typename std::make_unsigned<Input2>::type;
+    using S1 = std::make_signed_t<Input1>;
+    using S2 = std::make_signed_t<Input2>;
+    using U1 = std::make_unsigned_t<Input1>;
+    using U2 = std::make_unsigned_t<Input2>;
     bool accumulate = true;
     for (auto TC : Cases) {
         { // Test with two signed types
             using Output = std::common_type_t<S1, S2>;
             accumulate &= test0<S1, S2, Output>(TC.x, TC.y, TC.expect);
             accumulate &= test0<S1, S2, Output>(-TC.x, TC.y, TC.expect);
             accumulate &= test0<S1, S2, Output>(TC.x, -TC.y, TC.expect);
             accumulate &= test0<S1, S2, Output>(-TC.x, -TC.y, TC.expect);
int main()
     static_assert(do_test<long long>(), "");
 
     assert(do_test<signed char>(non_cce));
     assert(do_test<short>(non_cce));
     assert(do_test<int>(non_cce));
     assert(do_test<long>(non_cce));
     assert(do_test<long long>(non_cce));
 
-    static_assert(do_test< int8_t>(), "");
-    static_assert(do_test<int16_t>(), "");
-    static_assert(do_test<int32_t>(), "");
-    static_assert(do_test<int64_t>(), "");
-
-    assert(do_test< int8_t>(non_cce));
-    assert(do_test<int16_t>(non_cce));
-    assert(do_test<int32_t>(non_cce));
-    assert(do_test<int64_t>(non_cce));
+    static_assert(do_test<std::int8_t>(), "");
+    static_assert(do_test<std::int16_t>(), "");
+    static_assert(do_test<std::int32_t>(), "");
+    static_assert(do_test<std::int64_t>(), "");
+
+    assert(do_test<std::int8_t>(non_cce));
+    assert(do_test<std::int16_t>(non_cce));
+    assert(do_test<std::int32_t>(non_cce));
+    assert(do_test<std::int64_t>(non_cce));
 
     static_assert(do_test<signed char, int>(), "");
     static_assert(do_test<int, signed char>(), "");
     static_assert(do_test<short, int>(), "");
     static_assert(do_test<int, short>(), "");
     static_assert(do_test<int, long>(), "");
     static_assert(do_test<long, int>(), "");
     static_assert(do_test<int, long long>(), "");
     static_assert(do_test<long long, int>(), "");
 
     assert((do_test<signed char, int>(non_cce)));
     assert((do_test<int, signed char>(non_cce)));
     assert((do_test<short, int>(non_cce)));
     assert((do_test<int, short>(non_cce)));
     assert((do_test<int, long>(non_cce)));
     assert((do_test<long, int>(non_cce)));
     assert((do_test<int, long long>(non_cce)));
     assert((do_test<long long, int>(non_cce)));
 
 //  LWG#2837
     {
-        auto res = std::gcd((int64_t)1234, (int32_t)-2147483648);
-        static_assert( std::is_same<decltype(res), std::common_type<int64_t, int32_t>::type>::value, "");
+        auto res = std::gcd(static_cast<std::int64_t>(1234), INT32_MIN);
+        static_assert(std::is_same<decltype(res), std::int64_t>::value, "");
         assert(res == 2);
     }
 }

test/std/numerics/numeric.ops/numeric.ops.lcm/lcm.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is dual licensed under the MIT and the University of Illinois Open
 // Source Licenses. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // UNSUPPORTED: c++98, c++03, c++11, c++14
 // <numeric>
 
 // template<class _M, class _N>
-// constexpr common_type_t<_M,_N> gcd(_M __m, _N __n)
+// constexpr common_type_t<_M,_N> lcm(_M __m, _N __n)
 
 #include <numeric>
 #include <cassert>
+#include <cstdint>
 #include <cstdlib>
-#include <iostream>
+#include <type_traits>
 
 constexpr struct {
   int x;
   int y;
   int expect;
 } Cases[] = {
     {0, 0, 0},
     {1, 0, 0},
     {0, 1, 0},
     {1, 1, 1},
     {2, 3, 6},
     {2, 4, 4},
     {3, 17, 51},
     {36, 18, 36}
 };
 
 template <typename Input1, typename Input2, typename Output>
-constexpr bool test0(Input1 in1, Input2 in2, Output out)
+constexpr bool test0(int in1, int in2, int out)
 {
-    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, "");
+    static_assert(std::is_same<Output, decltype(
+        std::lcm(static_cast<Input2>(0), static_cast<Input1>(0)))>::value, "");
+    const bool result = static_cast<std::common_type_t<Input1, Input2>>(out) ==
+        std::lcm(static_cast<Input1>(in1), static_cast<Input2>(in2));
+    if (!result) {
+        std::abort();
+    }
+
+    return result;
 }
 
 
 template <typename Input1, typename Input2 = Input1>
 constexpr bool do_test(int = 0)
 {
-    using S1 = typename std::make_signed<Input1>::type;
-    using S2 = typename std::make_signed<Input2>::type;
-    using U1 = typename std::make_unsigned<Input1>::type;
-    using U2 = typename std::make_unsigned<Input2>::type;
+    using S1 = std::make_signed_t<Input1>;
+    using S2 = std::make_signed_t<Input2>;
+    using U1 = std::make_unsigned_t<Input1>;
+    using U2 = std::make_unsigned_t<Input2>;
     bool accumulate = true;
     for (auto TC : Cases) {
         { // Test with two signed types
             using Output = std::common_type_t<S1, S2>;
             accumulate &= test0<S1, S2, Output>(TC.x, TC.y, TC.expect);
             accumulate &= test0<S1, S2, Output>(-TC.x, TC.y, TC.expect);
             accumulate &= test0<S1, S2, Output>(TC.x, -TC.y, TC.expect);
             accumulate &= test0<S1, S2, Output>(-TC.x, -TC.y, TC.expect);
int main()
     static_assert(do_test<long long>(), "");
 
     assert(do_test<signed char>(non_cce));
     assert(do_test<short>(non_cce));
     assert(do_test<int>(non_cce));
     assert(do_test<long>(non_cce));
     assert(do_test<long long>(non_cce));
 
-    static_assert(do_test< int8_t>(), "");
-    static_assert(do_test<int16_t>(), "");
-    static_assert(do_test<int32_t>(), "");
-    static_assert(do_test<int64_t>(), "");
-
-    assert(do_test< int8_t>(non_cce));
-    assert(do_test<int16_t>(non_cce));
-    assert(do_test<int32_t>(non_cce));
-    assert(do_test<int64_t>(non_cce));
+    static_assert(do_test<std::int8_t>(), "");
+    static_assert(do_test<std::int16_t>(), "");
+    static_assert(do_test<std::int32_t>(), "");
+    static_assert(do_test<std::int64_t>(), "");
+
+    assert(do_test<std::int8_t>(non_cce));
+    assert(do_test<std::int16_t>(non_cce));
+    assert(do_test<std::int32_t>(non_cce));
+    assert(do_test<std::int64_t>(non_cce));
 
     static_assert(do_test<signed char, int>(), "");
     static_assert(do_test<int, signed char>(), "");
     static_assert(do_test<short, int>(), "");
     static_assert(do_test<int, short>(), "");
     static_assert(do_test<int, long>(), "");
     static_assert(do_test<long, int>(), "");
     static_assert(do_test<int, long long>(), "");
     static_assert(do_test<long long, int>(), "");
 
     assert((do_test<signed char, int>(non_cce)));
     assert((do_test<int, signed char>(non_cce)));
     assert((do_test<short, int>(non_cce)));
     assert((do_test<int, short>(non_cce)));
     assert((do_test<int, long>(non_cce)));
     assert((do_test<long, int>(non_cce)));
     assert((do_test<int, long long>(non_cce)));
     assert((do_test<long long, int>(non_cce)));
 
 //  LWG#2837
     {
-    auto res1 = std::lcm((int64_t)1234, (int32_t)-2147483648);
-    (void) std::lcm<int, unsigned long>(INT_MIN, 2);  // this used to trigger UBSAN
-    static_assert( std::is_same<decltype(res1), std::common_type<int64_t, int32_t>::type>::value, "");
+    auto res1 = std::lcm(static_cast<std::int64_t>(1234), INT32_MIN);
+    (void)std::lcm(INT_MIN, 2UL);  // this used to trigger UBSAN
+    static_assert(std::is_same<decltype(res1), std::int64_t>::value, "");
   assert(res1 == 1324997410816LL);
     }
 }