1commit db5f2b6e74d6d2e2a9c6f530199067c1d6eff7b6 2Author: Max Morin <[email protected]> 3Date: Mon Aug 27 09:45:24 2018 +0000 4 5 Make TimeDelta TriviallyCopyable for std::atomic. 6 7 We're having some awkwardness with working around the issue with stuff 8 like std::atomic<int64_t> and conversions back and forth. It would be 9 preferable to use std::atomic<TimeDelta> directly. 10 11 Removes the workaround added for bug 635974, since that was a bug in 12 vs 2015 and we use clang now. 13 14 Also fixes a couple of lint issues in time.h. 15 16 Bug: 635974, 851959, 761570 17 Change-Id: I4683f960b0c348748c5f0aaf222da4dda40256ec 18 Reviewed-on: https://chromium-review.googlesource.com/1184781 19 Commit-Queue: Max Morin <[email protected]> 20 Reviewed-by: Yuri Wiitala <[email protected]> 21 Reviewed-by: Bruce Dawson <[email protected]> 22 Cr-Commit-Position: refs/heads/master@{#586219} 23 24diff --git a/base/time/time.h b/base/time/time.h 25index f4c2f93f30b4..7d4f308545c9 100644 26--- a/base/time/time.h 27+++ b/base/time/time.h 28@@ -199,11 +199,6 @@ class BASE_EXPORT TimeDelta { 29 double InMicrosecondsF() const; 30 int64_t InNanoseconds() const; 31 32- constexpr TimeDelta& operator=(TimeDelta other) { 33- delta_ = other.delta_; 34- return *this; 35- } 36- 37 // Computations with other deltas. Can easily be made constexpr with C++17 but 38 // hard to do until then per limitations around 39 // __builtin_(add|sub)_overflow in safe_math_clang_gcc_impl.h : 40@@ -283,11 +278,6 @@ class BASE_EXPORT TimeDelta { 41 return delta_ >= other.delta_; 42 } 43 44-#if defined(OS_WIN) 45- // This works around crbug.com/635974 46- constexpr TimeDelta(const TimeDelta& other) : delta_(other.delta_) {} 47-#endif 48- 49 private: 50 friend int64_t time_internal::SaturatedAdd(TimeDelta delta, int64_t value); 51 friend int64_t time_internal::SaturatedSub(TimeDelta delta, int64_t value); 52@@ -375,7 +365,7 @@ class TimeBase { 53 // 54 // DEPRECATED - Do not use in new code. For serializing Time values, prefer 55 // Time::ToDeltaSinceWindowsEpoch().InMicroseconds(). http://crbug.com/634507 56- int64_t ToInternalValue() const { return us_; } 57+ constexpr int64_t ToInternalValue() const { return us_; } 58 59 // The amount of time since the origin (or "zero") point. This is a syntactic 60 // convenience to aid in code readability, mainly for debugging/testing use 61@@ -799,7 +789,7 @@ constexpr TimeDelta TimeDelta::FromDouble(double value) { 62 // static 63 constexpr TimeDelta TimeDelta::FromProduct(int64_t value, 64 int64_t positive_value) { 65- DCHECK(positive_value > 0); 66+ DCHECK(positive_value > 0); // NOLINT, DCHECK_GT isn't constexpr. 67 return value > std::numeric_limits<int64_t>::max() / positive_value 68 ? Max() 69 : value < std::numeric_limits<int64_t>::min() / positive_value 70@@ -903,8 +893,8 @@ class BASE_EXPORT TimeTicks : public time_internal::TimeBase<TimeTicks> { 71 return TimeTicks(us); 72 } 73 74-#if defined(OS_WIN) 75 protected: 76+#if defined(OS_WIN) 77 typedef DWORD (*TickFunctionType)(void); 78 static TickFunctionType SetMockTickFunction(TickFunctionType ticker); 79 #endif 80@@ -926,8 +916,7 @@ BASE_EXPORT std::ostream& operator<<(std::ostream& os, TimeTicks time_ticks); 81 // thread is running. 82 class BASE_EXPORT ThreadTicks : public time_internal::TimeBase<ThreadTicks> { 83 public: 84- ThreadTicks() : TimeBase(0) { 85- } 86+ constexpr ThreadTicks() : TimeBase(0) {} 87 88 // Returns true if ThreadTicks::Now() is supported on this system. 89 static bool IsSupported() WARN_UNUSED_RESULT { 90diff --git a/base/time/time_unittest.cc b/base/time/time_unittest.cc 91index 5dc8888a3297..ce8cc39cbaec 100644 92--- a/base/time/time_unittest.cc 93+++ b/base/time/time_unittest.cc 94@@ -1542,6 +1542,70 @@ TEST(TimeDelta, Overflows) { 95 EXPECT_EQ(kOneSecond, (ticks_now + kOneSecond) - ticks_now); 96 } 97 98+constexpr TimeTicks TestTimeTicksConstexprCopyAssignment() { 99+ TimeTicks a = TimeTicks::FromInternalValue(12345); 100+ TimeTicks b; 101+ b = a; 102+ return b; 103+} 104+ 105+TEST(TimeTicks, ConstexprAndTriviallyCopiable) { 106+ // "Trivially copyable" is necessary for use in std::atomic<TimeTicks>. 107+ static_assert(std::is_trivially_copyable<TimeTicks>(), ""); 108+ 109+ // Copy ctor. 110+ constexpr TimeTicks a = TimeTicks::FromInternalValue(12345); 111+ constexpr TimeTicks b{a}; 112+ static_assert(a.ToInternalValue() == b.ToInternalValue(), ""); 113+ 114+ // Copy assignment. 115+ static_assert(a.ToInternalValue() == 116+ TestTimeTicksConstexprCopyAssignment().ToInternalValue(), 117+ ""); 118+} 119+ 120+constexpr ThreadTicks TestThreadTicksConstexprCopyAssignment() { 121+ ThreadTicks a = ThreadTicks::FromInternalValue(12345); 122+ ThreadTicks b; 123+ b = a; 124+ return b; 125+} 126+ 127+TEST(ThreadTicks, ConstexprAndTriviallyCopiable) { 128+ // "Trivially copyable" is necessary for use in std::atomic<ThreadTicks>. 129+ static_assert(std::is_trivially_copyable<ThreadTicks>(), ""); 130+ 131+ // Copy ctor. 132+ constexpr ThreadTicks a = ThreadTicks::FromInternalValue(12345); 133+ constexpr ThreadTicks b{a}; 134+ static_assert(a.ToInternalValue() == b.ToInternalValue(), ""); 135+ 136+ // Copy assignment. 137+ static_assert(a.ToInternalValue() == 138+ TestThreadTicksConstexprCopyAssignment().ToInternalValue(), 139+ ""); 140+} 141+ 142+constexpr TimeDelta TestTimeDeltaConstexprCopyAssignment() { 143+ TimeDelta a = TimeDelta::FromSeconds(1); 144+ TimeDelta b; 145+ b = a; 146+ return b; 147+} 148+ 149+TEST(TimeDelta, ConstexprAndTriviallyCopiable) { 150+ // "Trivially copyable" is necessary for use in std::atomic<TimeDelta>. 151+ static_assert(std::is_trivially_copyable<TimeDelta>(), ""); 152+ 153+ // Copy ctor. 154+ constexpr TimeDelta a = TimeDelta::FromSeconds(1); 155+ constexpr TimeDelta b{a}; 156+ static_assert(a == b, ""); 157+ 158+ // Copy assignment. 159+ static_assert(a == TestTimeDeltaConstexprCopyAssignment(), ""); 160+} 161+ 162 TEST(TimeDeltaLogging, DCheckEqCompiles) { 163 DCHECK_EQ(TimeDelta(), TimeDelta()); 164 } 165