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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
