From ab0d757bcfc486ac6ed8ba69d4e630feda2b6ab7 Mon Sep 17 00:00:00 2001 From: Mark de Wever Date: Mon, 17 Jul 2023 20:25:01 +0200 Subject: [PATCH] [libc++][chrono] Fixes month inc and dec operations. The operator++, operator++(int), operator--, and operator--(int) need to change the month to a valid value. The wording is specified in terms of operator+(const month& x, const months& y) noexcept; which has the correct behavior. The aforementioned operators instead used ++/-- on the internal value direction, resulting in incorrect behaviour. As a drive-by improve the unit tests: - use the typical constexpr test method - test whether the month is valid after the operations - format the tests Fixes: https://llvm.org/PR63912 Reviewed By: #libc, ldionne Differential Revision: https://reviews.llvm.org/D155504 --- libcxx/include/__chrono/month.h | 4 +- .../time.cal.month.members/decrement.pass.cpp | 60 ++++++++++-------- .../time.cal.month.members/increment.pass.cpp | 59 ++++++++++------- .../plus_minus_equal.pass.cpp | 73 ++++++++++------------ .../time.cal.month.nonmembers/minus.pass.cpp | 3 +- .../time.cal.month.nonmembers/plus.pass.cpp | 64 +++++++++---------- 6 files changed, 135 insertions(+), 128 deletions(-) diff --git a/libcxx/include/__chrono/month.h b/libcxx/include/__chrono/month.h index 5dff7ce..7566e4e 100644 --- a/libcxx/include/__chrono/month.h +++ b/libcxx/include/__chrono/month.h @@ -31,9 +31,9 @@ private: public: month() = default; _LIBCPP_HIDE_FROM_ABI explicit inline constexpr month(unsigned __val) noexcept : __m_(static_cast(__val)) {} - _LIBCPP_HIDE_FROM_ABI inline constexpr month& operator++() noexcept { ++__m_; return *this; } + _LIBCPP_HIDE_FROM_ABI inline constexpr month& operator++() noexcept { *this += months{1}; return *this; } _LIBCPP_HIDE_FROM_ABI inline constexpr month operator++(int) noexcept { month __tmp = *this; ++(*this); return __tmp; } - _LIBCPP_HIDE_FROM_ABI inline constexpr month& operator--() noexcept { --__m_; return *this; } + _LIBCPP_HIDE_FROM_ABI inline constexpr month& operator--() noexcept { *this -= months{1}; return *this; } _LIBCPP_HIDE_FROM_ABI inline constexpr month operator--(int) noexcept { month __tmp = *this; --(*this); return __tmp; } _LIBCPP_HIDE_FROM_ABI constexpr month& operator+=(const months& __m1) noexcept; _LIBCPP_HIDE_FROM_ABI constexpr month& operator-=(const months& __m1) noexcept; diff --git a/libcxx/test/std/time/time.cal/time.cal.month/time.cal.month.members/decrement.pass.cpp b/libcxx/test/std/time/time.cal/time.cal.month/time.cal.month.members/decrement.pass.cpp index 5671122..9299d9e 100644 --- a/libcxx/test/std/time/time.cal/time.cal.month/time.cal.month.members/decrement.pass.cpp +++ b/libcxx/test/std/time/time.cal/time.cal.month/time.cal.month.members/decrement.pass.cpp @@ -13,42 +13,52 @@ // constexpr month& operator--() noexcept; // constexpr month operator--(int) noexcept; - #include #include #include #include "test_macros.h" -template -constexpr bool testConstexpr() -{ - M m1{10}; - if (static_cast(--m1) != 9) return false; - if (static_cast(m1--) != 9) return false; - if (static_cast(m1) != 8) return false; - return true; -} +constexpr bool test() { + using month = std::chrono::month; + for (unsigned i = 0; i <= 15; ++i) { + month m1(i); + month m2 = m1--; + assert(m1.ok()); + assert(m1 != m2); + + unsigned exp = i == 0 ? 11 : i == 1 ? 12 : i - 1; + while (exp > 12) + exp -= 12; + assert(static_cast(m1) == exp); + } + for (unsigned i = 0; i <= 15; ++i) { + month m1(i); + month m2 = --m1; + assert(m1.ok()); + assert(m2.ok()); + assert(m1 == m2); -int main(int, char**) -{ - using month = std::chrono::month; + unsigned exp = i == 0 ? 11 : i == 1 ? 12 : i - 1; + while (exp > 12) + exp -= 12; + assert(static_cast(m1) == exp); + } + + return true; +} - ASSERT_NOEXCEPT(--(std::declval()) ); - ASSERT_NOEXCEPT( (std::declval())--); +int main(int, char**) { + using month = std::chrono::month; - ASSERT_SAME_TYPE(month , decltype( std::declval()--)); - ASSERT_SAME_TYPE(month&, decltype(--std::declval() )); + ASSERT_NOEXCEPT(--(std::declval())); + ASSERT_NOEXCEPT((std::declval())--); - static_assert(testConstexpr(), ""); + ASSERT_SAME_TYPE(month, decltype(std::declval()--)); + ASSERT_SAME_TYPE(month&, decltype(--std::declval())); - for (unsigned i = 10; i <= 20; ++i) - { - month m(i); - assert(static_cast(--m) == i - 1); - assert(static_cast(m--) == i - 1); - assert(static_cast(m) == i - 2); - } + test(); + static_assert(test()); return 0; } diff --git a/libcxx/test/std/time/time.cal/time.cal.month/time.cal.month.members/increment.pass.cpp b/libcxx/test/std/time/time.cal/time.cal.month/time.cal.month.members/increment.pass.cpp index af61d34..1f8f741 100644 --- a/libcxx/test/std/time/time.cal/time.cal.month/time.cal.month.members/increment.pass.cpp +++ b/libcxx/test/std/time/time.cal/time.cal.month/time.cal.month.members/increment.pass.cpp @@ -13,41 +13,52 @@ // constexpr month& operator++() noexcept; // constexpr month operator++(int) noexcept; - #include #include #include #include "test_macros.h" -template -constexpr bool testConstexpr() -{ - M m1{1}; - if (static_cast(++m1) != 2) return false; - if (static_cast(m1++) != 2) return false; - if (static_cast(m1) != 3) return false; - return true; +constexpr bool test() { + using month = std::chrono::month; + for (unsigned i = 0; i <= 15; ++i) { + month m1(i); + month m2 = m1++; + assert(m1.ok()); + assert(m1 != m2); + + unsigned exp = i + 1; + while (exp > 12) + exp -= 12; + assert(static_cast(m1) == exp); + } + for (unsigned i = 0; i <= 15; ++i) { + month m1(i); + month m2 = ++m1; + assert(m1.ok()); + assert(m2.ok()); + assert(m1 == m2); + + unsigned exp = i + 1; + while (exp > 12) + exp -= 12; + assert(static_cast(m1) == exp); + } + + return true; } -int main(int, char**) -{ - using month = std::chrono::month; - ASSERT_NOEXCEPT(++(std::declval()) ); - ASSERT_NOEXCEPT( (std::declval())++); +int main(int, char**) { + using month = std::chrono::month; - ASSERT_SAME_TYPE(month , decltype( std::declval()++)); - ASSERT_SAME_TYPE(month&, decltype(++std::declval() )); + ASSERT_NOEXCEPT(++(std::declval())); + ASSERT_NOEXCEPT((std::declval())++); - static_assert(testConstexpr(), ""); + ASSERT_SAME_TYPE(month, decltype(std::declval()++)); + ASSERT_SAME_TYPE(month&, decltype(++std::declval())); - for (unsigned i = 0; i <= 10; ++i) - { - month m(i); - assert(static_cast(++m) == i + 1); - assert(static_cast(m++) == i + 1); - assert(static_cast(m) == i + 2); - } + test(); + static_assert(test()); return 0; } diff --git a/libcxx/test/std/time/time.cal/time.cal.month/time.cal.month.members/plus_minus_equal.pass.cpp b/libcxx/test/std/time/time.cal/time.cal.month/time.cal.month.members/plus_minus_equal.pass.cpp index 848a96a..d825ef7 100644 --- a/libcxx/test/std/time/time.cal/time.cal.month/time.cal.month.members/plus_minus_equal.pass.cpp +++ b/libcxx/test/std/time/time.cal/time.cal.month/time.cal.month.members/plus_minus_equal.pass.cpp @@ -19,50 +19,43 @@ #include "test_macros.h" -template -constexpr bool testConstexpr() -{ - M m1{1}; - if (static_cast(m1 += Ms{ 1}) != 2) return false; - if (static_cast(m1 += Ms{ 2}) != 4) return false; - if (static_cast(m1 += Ms{ 8}) != 12) return false; - if (static_cast(m1 -= Ms{ 1}) != 11) return false; - if (static_cast(m1 -= Ms{ 2}) != 9) return false; - if (static_cast(m1 -= Ms{ 8}) != 1) return false; - return true; +constexpr bool test() { + using month = std::chrono::month; + using months = std::chrono::months; + + for (unsigned i = 1; i <= 10; ++i) { + month m(i); + int exp = i + 10; + while (exp > 12) + exp -= 12; + assert(static_cast(m += months{10}) == static_cast(exp)); + assert(static_cast(m) == static_cast(exp)); + assert(m.ok()); + } + + for (unsigned i = 1; i <= 10; ++i) { + month m(i); + int exp = i - 9; + while (exp < 1) + exp += 12; + assert(static_cast(m -= months{9}) == static_cast(exp)); + assert(static_cast(m) == static_cast(exp)); + assert(m.ok()); + } + return true; } -int main(int, char**) -{ - using month = std::chrono::month; - using months = std::chrono::months; +int main(int, char**) { + using month = std::chrono::month; + using months = std::chrono::months; - ASSERT_NOEXCEPT(std::declval() += std::declval()); - ASSERT_NOEXCEPT(std::declval() -= std::declval()); - ASSERT_SAME_TYPE(month&, decltype(std::declval() += std::declval())); - ASSERT_SAME_TYPE(month&, decltype(std::declval() -= std::declval())); + ASSERT_NOEXCEPT(std::declval() += std::declval()); + ASSERT_NOEXCEPT(std::declval() -= std::declval()); + ASSERT_SAME_TYPE(month&, decltype(std::declval() += std::declval())); + ASSERT_SAME_TYPE(month&, decltype(std::declval() -= std::declval())); - static_assert(testConstexpr(), ""); - - for (unsigned i = 1; i <= 10; ++i) - { - month m(i); - int exp = i + 10; - while (exp > 12) - exp -= 12; - assert(static_cast(m += months{10}) == static_cast(exp)); - assert(static_cast(m) == static_cast(exp)); - } - - for (unsigned i = 1; i <= 10; ++i) - { - month m(i); - int exp = i - 9; - while (exp < 1) - exp += 12; - assert(static_cast(m -= months{ 9}) == static_cast(exp)); - assert(static_cast(m) == static_cast(exp)); - } + test(); + static_assert(test()); return 0; } diff --git a/libcxx/test/std/time/time.cal/time.cal.month/time.cal.month.nonmembers/minus.pass.cpp b/libcxx/test/std/time/time.cal/time.cal.month/time.cal.month.nonmembers/minus.pass.cpp index f9ef5b6..2041afc 100644 --- a/libcxx/test/std/time/time.cal/time.cal.month/time.cal.month.nonmembers/minus.pass.cpp +++ b/libcxx/test/std/time/time.cal/time.cal.month/time.cal.month.nonmembers/minus.pass.cpp @@ -58,12 +58,11 @@ int main(int, char**) for (unsigned i = 1; i <= 12; ++i) { month m1 = m - months{i}; - // months off = m - month {i}; + assert(m1.ok()); int exp = 6 - i; if (exp < 1) exp += 12; assert(static_cast(m1) == static_cast(exp)); - // assert(off.count() == static_cast(exp)); } return 0; diff --git a/libcxx/test/std/time/time.cal/time.cal.month/time.cal.month.nonmembers/plus.pass.cpp b/libcxx/test/std/time/time.cal/time.cal.month/time.cal.month.nonmembers/plus.pass.cpp index 297df13..ee7a33e 100644 --- a/libcxx/test/std/time/time.cal/time.cal.month/time.cal.month.nonmembers/plus.pass.cpp +++ b/libcxx/test/std/time/time.cal/time.cal.month/time.cal.month.nonmembers/plus.pass.cpp @@ -23,51 +23,45 @@ // holding a value in the range [1, 12] even if !x.ok(). -end note] // [Example: February + months{11} == January. -end example] - - #include #include #include #include "test_macros.h" -template -constexpr bool testConstexpr() -{ - M m{1}; - Ms offset{4}; - assert(m + offset == M{5}); - assert(offset + m == M{5}); - // Check the example - assert(M{2} + Ms{11} == M{1}); - return true; -} +constexpr bool test() { + using month = std::chrono::month; + using months = std::chrono::months; -int main(int, char**) -{ - using month = std::chrono::month; - using months = std::chrono::months; + month my{2}; + for (unsigned i = 0; i <= 15; ++i) { + month m1 = my + months{i}; + month m2 = months{i} + my; + assert(m1.ok()); + assert(m2.ok()); + assert(m1 == m2); + unsigned exp = i + 2; + while (exp > 12) + exp -= 12; + assert(static_cast(m1) == exp); + assert(static_cast(m2) == exp); + } + + return true; +} - ASSERT_NOEXCEPT(std::declval() + std::declval()); - ASSERT_NOEXCEPT(std::declval() + std::declval()); +int main(int, char**) { + using month = std::chrono::month; + using months = std::chrono::months; - ASSERT_SAME_TYPE(month, decltype(std::declval() + std::declval())); - ASSERT_SAME_TYPE(month, decltype(std::declval() + std::declval() )); + ASSERT_NOEXCEPT(std::declval() + std::declval()); + ASSERT_NOEXCEPT(std::declval() + std::declval()); - static_assert(testConstexpr(), ""); + ASSERT_SAME_TYPE(month, decltype(std::declval() + std::declval())); + ASSERT_SAME_TYPE(month, decltype(std::declval() + std::declval())); - month my{2}; - for (unsigned i = 0; i <= 15; ++i) - { - month m1 = my + months{i}; - month m2 = months{i} + my; - assert(m1 == m2); - unsigned exp = i + 2; - while (exp > 12) - exp -= 12; - assert(static_cast(m1) == exp); - assert(static_cast(m2) == exp); - } + test(); + static_assert(test()); - return 0; + return 0; } -- 2.7.4