commit db5f2b6e74d6d2e2a9c6f530199067c1d6eff7b6 Author: Max Morin Date: Mon Aug 27 09:45:24 2018 +0000 Make TimeDelta TriviallyCopyable for std::atomic. We're having some awkwardness with working around the issue with stuff like std::atomic and conversions back and forth. It would be preferable to use std::atomic directly. Removes the workaround added for bug 635974, since that was a bug in vs 2015 and we use clang now. Also fixes a couple of lint issues in time.h. Bug: 635974, 851959, 761570 Change-Id: I4683f960b0c348748c5f0aaf222da4dda40256ec Reviewed-on: https://chromium-review.googlesource.com/1184781 Commit-Queue: Max Morin Reviewed-by: Yuri Wiitala Reviewed-by: Bruce Dawson Cr-Commit-Position: refs/heads/master@{#586219} diff --git a/base/time/time.h b/base/time/time.h index f4c2f93f30b4..7d4f308545c9 100644 --- a/base/time/time.h +++ b/base/time/time.h @@ -199,11 +199,6 @@ class BASE_EXPORT TimeDelta { double InMicrosecondsF() const; int64_t InNanoseconds() const; - constexpr TimeDelta& operator=(TimeDelta other) { - delta_ = other.delta_; - return *this; - } - // Computations with other deltas. Can easily be made constexpr with C++17 but // hard to do until then per limitations around // __builtin_(add|sub)_overflow in safe_math_clang_gcc_impl.h : @@ -283,11 +278,6 @@ class BASE_EXPORT TimeDelta { return delta_ >= other.delta_; } -#if defined(OS_WIN) - // This works around crbug.com/635974 - constexpr TimeDelta(const TimeDelta& other) : delta_(other.delta_) {} -#endif - private: friend int64_t time_internal::SaturatedAdd(TimeDelta delta, int64_t value); friend int64_t time_internal::SaturatedSub(TimeDelta delta, int64_t value); @@ -375,7 +365,7 @@ class TimeBase { // // DEPRECATED - Do not use in new code. For serializing Time values, prefer // Time::ToDeltaSinceWindowsEpoch().InMicroseconds(). http://crbug.com/634507 - int64_t ToInternalValue() const { return us_; } + constexpr int64_t ToInternalValue() const { return us_; } // The amount of time since the origin (or "zero") point. This is a syntactic // convenience to aid in code readability, mainly for debugging/testing use @@ -799,7 +789,7 @@ constexpr TimeDelta TimeDelta::FromDouble(double value) { // static constexpr TimeDelta TimeDelta::FromProduct(int64_t value, int64_t positive_value) { - DCHECK(positive_value > 0); + DCHECK(positive_value > 0); // NOLINT, DCHECK_GT isn't constexpr. return value > std::numeric_limits::max() / positive_value ? Max() : value < std::numeric_limits::min() / positive_value @@ -903,8 +893,8 @@ class BASE_EXPORT TimeTicks : public time_internal::TimeBase { return TimeTicks(us); } -#if defined(OS_WIN) protected: +#if defined(OS_WIN) typedef DWORD (*TickFunctionType)(void); static TickFunctionType SetMockTickFunction(TickFunctionType ticker); #endif @@ -926,8 +916,7 @@ BASE_EXPORT std::ostream& operator<<(std::ostream& os, TimeTicks time_ticks); // thread is running. class BASE_EXPORT ThreadTicks : public time_internal::TimeBase { public: - ThreadTicks() : TimeBase(0) { - } + constexpr ThreadTicks() : TimeBase(0) {} // Returns true if ThreadTicks::Now() is supported on this system. static bool IsSupported() WARN_UNUSED_RESULT { diff --git a/base/time/time_unittest.cc b/base/time/time_unittest.cc index 5dc8888a3297..ce8cc39cbaec 100644 --- a/base/time/time_unittest.cc +++ b/base/time/time_unittest.cc @@ -1542,6 +1542,70 @@ TEST(TimeDelta, Overflows) { EXPECT_EQ(kOneSecond, (ticks_now + kOneSecond) - ticks_now); } +constexpr TimeTicks TestTimeTicksConstexprCopyAssignment() { + TimeTicks a = TimeTicks::FromInternalValue(12345); + TimeTicks b; + b = a; + return b; +} + +TEST(TimeTicks, ConstexprAndTriviallyCopiable) { + // "Trivially copyable" is necessary for use in std::atomic. + static_assert(std::is_trivially_copyable(), ""); + + // Copy ctor. + constexpr TimeTicks a = TimeTicks::FromInternalValue(12345); + constexpr TimeTicks b{a}; + static_assert(a.ToInternalValue() == b.ToInternalValue(), ""); + + // Copy assignment. + static_assert(a.ToInternalValue() == + TestTimeTicksConstexprCopyAssignment().ToInternalValue(), + ""); +} + +constexpr ThreadTicks TestThreadTicksConstexprCopyAssignment() { + ThreadTicks a = ThreadTicks::FromInternalValue(12345); + ThreadTicks b; + b = a; + return b; +} + +TEST(ThreadTicks, ConstexprAndTriviallyCopiable) { + // "Trivially copyable" is necessary for use in std::atomic. + static_assert(std::is_trivially_copyable(), ""); + + // Copy ctor. + constexpr ThreadTicks a = ThreadTicks::FromInternalValue(12345); + constexpr ThreadTicks b{a}; + static_assert(a.ToInternalValue() == b.ToInternalValue(), ""); + + // Copy assignment. + static_assert(a.ToInternalValue() == + TestThreadTicksConstexprCopyAssignment().ToInternalValue(), + ""); +} + +constexpr TimeDelta TestTimeDeltaConstexprCopyAssignment() { + TimeDelta a = TimeDelta::FromSeconds(1); + TimeDelta b; + b = a; + return b; +} + +TEST(TimeDelta, ConstexprAndTriviallyCopiable) { + // "Trivially copyable" is necessary for use in std::atomic. + static_assert(std::is_trivially_copyable(), ""); + + // Copy ctor. + constexpr TimeDelta a = TimeDelta::FromSeconds(1); + constexpr TimeDelta b{a}; + static_assert(a == b, ""); + + // Copy assignment. + static_assert(a == TestTimeDeltaConstexprCopyAssignment(), ""); +} + TEST(TimeDeltaLogging, DCheckEqCompiles) { DCHECK_EQ(TimeDelta(), TimeDelta()); }