From 730db44fc0c60c573111449ac7213303f7ed223d Mon Sep 17 00:00:00 2001 From: Adam Simpkins Date: Fri, 1 Dec 2017 11:18:17 -0800 Subject: [PATCH] implement chrono conversions for unusual duration types Summary: Implement conversions between std::chrono::duration and POSIX-style time structures even when neither the numerator nor the denominator of the duration ratio are 1. Both of these are done by first converting to an intermediate type where the numerator is 1, and then using the conversion routines for that case. Reviewed By: yfeldblum Differential Revision: D6366647 fbshipit-source-id: 8f9495fb4101cac6d8b4cf0353a679107007b298 --- folly/chrono/Conv.h | 94 +++++++++++++++++++++++++--------- folly/chrono/test/ConvTest.cpp | 55 ++++++++++++++++++++ 2 files changed, 125 insertions(+), 24 deletions(-) diff --git a/folly/chrono/Conv.h b/folly/chrono/Conv.h index 688658ec..1cd6a8b0 100644 --- a/folly/chrono/Conv.h +++ b/folly/chrono/Conv.h @@ -187,6 +187,36 @@ static Expected, ConversionCode> durationToPosixTime( return std::pair{sec, subsec}; } +/* + * Helper classes for picking an intermediate duration type to use + * when doing conversions to/from durations where neither the numerator nor + * denominator are 1. + */ +template +struct IntermediateTimeRep {}; +template +struct IntermediateTimeRep { + using type = T; +}; +template +struct IntermediateTimeRep { + using type = intmax_t; +}; +template +struct IntermediateTimeRep { + using type = uintmax_t; +}; +// For IntermediateDuration we always use 1 as the numerator, and the original +// Period denominator. This ensures that we do not lose precision when +// performing the conversion. +template +using IntermediateDuration = std::chrono::duration< + typename IntermediateTimeRep< + Rep, + std::is_floating_point::value, + std::is_signed::value>::type, + std::ratio<1, Period::den>>; + /** * Convert a std::chrono::duration to a pair of (seconds, subseconds) * @@ -201,18 +231,28 @@ Expected, ConversionCode> durationToPosixTime( static_assert( SubsecondRatio::num == 1, "subsecond numerator should always be 1"); - // TODO: We need to implement an overflow-checking tryTo() function for - // duration-to-duration casts for the code above to work. - // - // For now this is unimplemented, and we just have a static_assert that - // will always fail if someone tries to instantiate this. Unusual duration - // types should be extremely rare, and I'm not aware of any code at the - // moment that actually needs this. - static_assert( - Period::num == 1, - "conversion from unusual duration types is not implemented yet"); - (void)duration; - return makeUnexpected(ConversionCode::SUCCESS); + // Perform this conversion by first converting to a duration where the + // numerator is 1, then convert to the output type. + using IntermediateType = IntermediateDuration; + using IntermediateRep = typename IntermediateType::rep; + + // Check to see if we would have overflow converting to the intermediate + // type. + constexpr auto maxInput = + std::numeric_limits::max() / Period::num; + if (duration.count() > maxInput) { + return makeUnexpected(ConversionCode::POSITIVE_OVERFLOW); + } + constexpr auto minInput = + std::numeric_limits::min() / Period::num; + if (duration.count() < minInput) { + return makeUnexpected(ConversionCode::NEGATIVE_OVERFLOW); + } + auto intermediate = + IntermediateType{static_cast(duration.count()) * + static_cast(Period::num)}; + + return durationToPosixTime(intermediate); } /** @@ -489,19 +529,25 @@ auto posixTimeToDuration( static_assert( SubsecondRatio::num == 1, "subsecond numerator should always be 1"); - // TODO: We need to implement an overflow-checking tryTo() function for - // duration-to-duration casts for the code above to work. + // Cast through an intermediate type with subsecond granularity. + // Note that this could fail due to overflow during the initial conversion + // even if the result is representable in the output POSIX-style types. // - // For now this is unimplemented, and we just have a static_assert that - // will always fail if someone tries to instantiate this. Unusual duration - // types should be extremely rare, and I'm not aware of any code at the - // moment that actually needs this. - static_assert( - Tgt::period::num == 1, - "conversion to unusual duration types is not implemented yet"); - (void)seconds; - (void)subseconds; - return makeUnexpected(ConversionCode::SUCCESS); + // Note that for integer type conversions going through this intermediate + // type can result in slight imprecision due to truncating the intermediate + // calculation to an integer. + using IntermediateType = + IntermediateDuration; + auto intermediate = posixTimeToDuration( + seconds, subseconds, IntermediateType{}); + if (intermediate.hasError()) { + return makeUnexpected(intermediate.error()); + } + // Now convert back to the target duration. Use tryTo() to confirm that the + // result fits in the target representation type. + return tryTo( + intermediate.value().count() / Tgt::period::num) + .then([](typename Tgt::rep tgt) { return Tgt{tgt}; }); } template < diff --git a/folly/chrono/test/ConvTest.cpp b/folly/chrono/test/ConvTest.cpp index 172bdd67..73312920 100644 --- a/folly/chrono/test/ConvTest.cpp +++ b/folly/chrono/test/ConvTest.cpp @@ -124,6 +124,24 @@ TEST(Conv, timespecToStdChrono) { ts.tv_nsec = 0; auto doubleMinutes = to>>(ts); EXPECT_EQ(1.5, doubleMinutes.count()); + + // Test with unusual durations where neither the numerator nor denominator + // are 1. + using five_sevenths = std::chrono::duration>; + ts.tv_sec = 1; + ts.tv_nsec = 0; + EXPECT_EQ(1, to(ts).count()); + ts.tv_sec = 1; + ts.tv_nsec = 428571500; + EXPECT_EQ(2, to(ts).count()); + + using thirteen_thirds = std::chrono::duration>; + ts.tv_sec = 39; + ts.tv_nsec = 0; + EXPECT_NEAR(9.0, to(ts).count(), 0.000000001); + ts.tv_sec = 1; + ts.tv_nsec = 0; + EXPECT_NEAR(0.230769230, to(ts).count(), 0.000000001); } TEST(Conv, timespecToStdChronoOverflow) { @@ -241,6 +259,22 @@ TEST(Conv, timespecToStdChronoOverflow) { EXPECT_EQ( std::numeric_limits::max() / 3600, to(ts).count()); + + // Test overflow with an unusual duration where neither the numerator nor + // denominator are 1. + using unusual_time = std::chrono::duration>; + ts.tv_sec = 141994; + ts.tv_nsec = 666666666; + EXPECT_EQ(32767, to(ts).count()); + ts.tv_nsec = 666666667; + EXPECT_THROW(to(ts), std::range_error); + + ts.tv_sec = -141998; + ts.tv_nsec = 999999999; + EXPECT_EQ(-32768, to(ts).count()); + ts.tv_sec = -141999; + ts.tv_nsec = 0; + EXPECT_THROW(to(ts), std::range_error); } TEST(Conv, timevalToStdChrono) { @@ -334,6 +368,20 @@ TEST(Conv, stdChronoToTimespec) { ts = to(createTimePoint(123ns)); EXPECT_EQ(0, ts.tv_sec); EXPECT_EQ(123, ts.tv_nsec); + + // Test with some unusual durations where neither the numerator nor + // denominator are 1. + using five_sevenths = std::chrono::duration>; + ts = to(five_sevenths(7)); + EXPECT_EQ(5, ts.tv_sec); + EXPECT_EQ(0, ts.tv_nsec); + ts = to(five_sevenths(19)); + EXPECT_EQ(13, ts.tv_sec); + EXPECT_EQ(571428571, ts.tv_nsec); + using seven_fifths = std::chrono::duration>; + ts = to(seven_fifths(5)); + EXPECT_EQ(7, ts.tv_sec); + EXPECT_EQ(0, ts.tv_nsec); } TEST(Conv, stdChronoToTimespecOverflow) { @@ -359,6 +407,13 @@ TEST(Conv, stdChronoToTimespecOverflow) { EXPECT_EQ(ts.tv_nsec, 0); EXPECT_THROW( to(hours_i64(2562047788015216LL)), std::range_error); + + // Test overflows from an unusual duration where neither the numerator nor + // denominator are 1. + using three_halves = std::chrono::duration>; + EXPECT_THROW( + to(three_halves(6148914691236517206ULL)), + std::range_error); } // Test for overflow. -- 2.34.1