Apply all suggestions from GitHub, except for replacing the utility subsecond_helper class with a function

This commit is contained in:
matrackif 2021-12-05 16:03:07 +01:00
parent 75444670c1
commit 3d860b7cf5
2 changed files with 23 additions and 19 deletions

View File

@ -1393,12 +1393,11 @@ template <class Duration> class subsecond_helper {
// Returns the amount of digits according to the c++ 20 spec // Returns the amount of digits according to the c++ 20 spec
// In the range [0, 18], if more than 18 fractional digits are required, // In the range [0, 18], if more than 18 fractional digits are required,
// then we return 6 for microseconds precision // then we return 6 for microseconds precision
static constexpr int num_digits(std::intmax_t num, std::intmax_t den, static constexpr int num_digits(long long num, long long den, int n = 0) {
int n = 0) {
return num % den == 0 ? n : (n > 18 ? 6 : num_digits(num * 10, den, n + 1)); return num % den == 0 ? n : (n > 18 ? 6 : num_digits(num * 10, den, n + 1));
} }
static constexpr std::intmax_t pow10(std::uint32_t n) { static constexpr long long pow10(std::uint32_t n) {
return n == 0 ? 1 : 10 * pow10(n - 1); return n == 0 ? 1 : 10 * pow10(n - 1);
} }
@ -1406,7 +1405,13 @@ template <class Duration> class subsecond_helper {
FMT_ENABLE_IF(std::numeric_limits<Rep>::is_signed)> FMT_ENABLE_IF(std::numeric_limits<Rep>::is_signed)>
static constexpr std::chrono::duration<Rep, Period> abs( static constexpr std::chrono::duration<Rep, Period> abs(
std::chrono::duration<Rep, Period> d) { std::chrono::duration<Rep, Period> d) {
return d >= d.zero() ? d : -d; // We need to compare the duration using the count() method directly
// due to a compiler bug in clang-11 regarding the spaceship operator,
// when -Wzero-as-null-pointer-constant is enabled.
// In clang-12 the bug has been fixed. See
// https://bugs.llvm.org/show_bug.cgi?id=46235 and the reproducible example:
// https://www.godbolt.org/z/Knbb5joYx
return d.count() >= d.zero().count() ? d : -d;
} }
template <class Rep, class Period, template <class Rep, class Period,
@ -1591,8 +1596,8 @@ struct chrono_formatter {
template <typename RepType> void write(RepType value, int width) { template <typename RepType> void write(RepType value, int width) {
write_sign(); write_sign();
if (isnan(value)) return write_nan(); if (isnan(value)) return write_nan();
uint32_or_64_or_128_t<std::intmax_t> n = uint32_or_64_or_128_t<long long> n =
to_unsigned(to_nonnegative_int(value, max_value<std::intmax_t>())); to_unsigned(to_nonnegative_int(value, max_value<long long>()));
int num_digits = detail::count_digits(n); int num_digits = detail::count_digits(n);
if (width > num_digits) out = std::fill_n(out, width - num_digits, '0'); if (width > num_digits) out = std::fill_n(out, width - num_digits, '0');
out = format_decimal<char_type>(out, n, num_digits).end; out = format_decimal<char_type>(out, n, num_digits).end;

View File

@ -543,9 +543,10 @@ TEST(chrono_test, negative_durations) {
} }
TEST(chrono_test, special_durations) { TEST(chrono_test, special_durations) {
EXPECT_EQ( auto value = fmt::format("{:%S}", std::chrono::duration<double>(1e20));
"40", // No decimal point is printed so size() is 2.
fmt::format("{:%S}", std::chrono::duration<double>(1e20)).substr(0, 2)); EXPECT_EQ(value.size(), 2);
EXPECT_EQ("40", value.substr(0, 2));
auto nan = std::numeric_limits<double>::quiet_NaN(); auto nan = std::numeric_limits<double>::quiet_NaN();
EXPECT_EQ( EXPECT_EQ(
"nan nan nan nan nan:nan nan", "nan nan nan nan nan:nan nan",
@ -584,8 +585,8 @@ TEST(chrono_test, weekday) {
} }
TEST(chrono_test, cpp20_duration_subsecond_support) { TEST(chrono_test, cpp20_duration_subsecond_support) {
using attoseconds = std::chrono::duration<std::intmax_t, std::atto>; using attoseconds = std::chrono::duration<long long, std::atto>;
// Check that 18 digits of subsecond precision are supported // Check that 18 digits of subsecond precision are supported.
EXPECT_EQ(fmt::format("{:%S}", attoseconds{999999999999999999}), EXPECT_EQ(fmt::format("{:%S}", attoseconds{999999999999999999}),
"00.999999999999999999"); "00.999999999999999999");
EXPECT_EQ(fmt::format("{:%S}", attoseconds{673231113420148734}), EXPECT_EQ(fmt::format("{:%S}", attoseconds{673231113420148734}),
@ -598,29 +599,27 @@ TEST(chrono_test, cpp20_duration_subsecond_support) {
"-13.420148734"); "-13.420148734");
EXPECT_EQ(fmt::format("{:%S}", std::chrono::milliseconds{1234}), "01.234"); EXPECT_EQ(fmt::format("{:%S}", std::chrono::milliseconds{1234}), "01.234");
{ {
// Check that {:%H:%M:%S} is equivalent to {:%T} // Check that {:%H:%M:%S} is equivalent to {:%T}.
auto dur = std::chrono::milliseconds{3601234}; auto dur = std::chrono::milliseconds{3601234};
auto formatted_dur = fmt::format("{:%T}", dur); auto formatted_dur = fmt::format("{:%T}", dur);
EXPECT_EQ(formatted_dur, "01:00:01.234"); EXPECT_EQ(formatted_dur, "01:00:01.234");
EXPECT_EQ(fmt::format("{:%H:%M:%S}", dur), formatted_dur); EXPECT_EQ(fmt::format("{:%H:%M:%S}", dur), formatted_dur);
} }
using nanoseconds_dbl = std::chrono::duration<double, std::nano>; using nanoseconds_dbl = std::chrono::duration<double, std::nano>;
EXPECT_EQ(fmt::format("{:%S}", nanoseconds_dbl{-123456789.123456789}), EXPECT_EQ(fmt::format("{:%S}", nanoseconds_dbl{-123456789}), "-00.123456789");
"-00.123456789"); EXPECT_EQ(fmt::format("{:%S}", nanoseconds_dbl{9123456789}), "09.123456789");
EXPECT_EQ(fmt::format("{:%S}", nanoseconds_dbl{9123456789.123456789}), // Verify that only the seconds part is extracted and printed.
"09.123456789");
// Only seconds part is printed
EXPECT_EQ(fmt::format("{:%S}", nanoseconds_dbl{99123456789}), "39.123456789"); EXPECT_EQ(fmt::format("{:%S}", nanoseconds_dbl{99123456789}), "39.123456789");
EXPECT_EQ(fmt::format("{:%S}", nanoseconds_dbl{99123000000}), "39.123000000"); EXPECT_EQ(fmt::format("{:%S}", nanoseconds_dbl{99123000000}), "39.123000000");
{ {
// Now the hour is printed, and we also test if negative doubles work // Now the hour is printed, and we also test if negative doubles work.
auto dur = nanoseconds_dbl{-99123456789}; auto dur = nanoseconds_dbl{-99123456789};
auto formatted_dur = fmt::format("{:%T}", dur); auto formatted_dur = fmt::format("{:%T}", dur);
EXPECT_EQ(formatted_dur, "-00:01:39.123456789"); EXPECT_EQ(formatted_dur, "-00:01:39.123456789");
EXPECT_EQ(fmt::format("{:%H:%M:%S}", dur), formatted_dur); EXPECT_EQ(fmt::format("{:%H:%M:%S}", dur), formatted_dur);
} }
// Check that durations with precision greater than std::chrono::seconds have // Check that durations with precision greater than std::chrono::seconds have
// fixed precision and empty zeros // fixed precision, and print zeros even if there is no fractional part.
EXPECT_EQ(fmt::format("{:%S}", std::chrono::microseconds{7000000}), EXPECT_EQ(fmt::format("{:%S}", std::chrono::microseconds{7000000}),
"07.000000"); "07.000000");
} }