[libcxx-commits] [libcxx] ab0d757 - [libc++][chrono] Fixes month inc and dec operations.

Mark de Wever via libcxx-commits libcxx-commits at lists.llvm.org
Tue Jul 18 08:33:01 PDT 2023


Author: Mark de Wever
Date: 2023-07-18T17:32:11+02:00
New Revision: ab0d757bcfc486ac6ed8ba69d4e630feda2b6ab7

URL: https://github.com/llvm/llvm-project/commit/ab0d757bcfc486ac6ed8ba69d4e630feda2b6ab7
DIFF: https://github.com/llvm/llvm-project/commit/ab0d757bcfc486ac6ed8ba69d4e630feda2b6ab7.diff

LOG: [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

Added: 
    

Modified: 
    libcxx/include/__chrono/month.h
    libcxx/test/std/time/time.cal/time.cal.month/time.cal.month.members/decrement.pass.cpp
    libcxx/test/std/time/time.cal/time.cal.month/time.cal.month.members/increment.pass.cpp
    libcxx/test/std/time/time.cal/time.cal.month/time.cal.month.members/plus_minus_equal.pass.cpp
    libcxx/test/std/time/time.cal/time.cal.month/time.cal.month.nonmembers/minus.pass.cpp
    libcxx/test/std/time/time.cal/time.cal.month/time.cal.month.nonmembers/plus.pass.cpp

Removed: 
    


################################################################################
diff  --git a/libcxx/include/__chrono/month.h b/libcxx/include/__chrono/month.h
index 5dff7ce3309f37..7566e4ed29983f 100644
--- a/libcxx/include/__chrono/month.h
+++ b/libcxx/include/__chrono/month.h
@@ -31,9 +31,9 @@ class month {
 public:
     month() = default;
     _LIBCPP_HIDE_FROM_ABI explicit inline constexpr month(unsigned __val) noexcept : __m_(static_cast<unsigned char>(__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 56711227771d6f..9299d9e297218f 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 <chrono>
 #include <type_traits>
 #include <cassert>
 
 #include "test_macros.h"
 
-template <typename M>
-constexpr bool testConstexpr()
-{
-    M m1{10};
-    if (static_cast<unsigned>(--m1) != 9) return false;
-    if (static_cast<unsigned>(m1--) != 9) return false;
-    if (static_cast<unsigned>(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<unsigned>(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<unsigned>(m1) == exp);
+  }
+
+  return true;
+}
 
-    ASSERT_NOEXCEPT(--(std::declval<month&>())  );
-    ASSERT_NOEXCEPT(  (std::declval<month&>())--);
+int main(int, char**) {
+  using month = std::chrono::month;
 
-    ASSERT_SAME_TYPE(month , decltype(  std::declval<month&>()--));
-    ASSERT_SAME_TYPE(month&, decltype(--std::declval<month&>()  ));
+  ASSERT_NOEXCEPT(--(std::declval<month&>()));
+  ASSERT_NOEXCEPT((std::declval<month&>())--);
 
-    static_assert(testConstexpr<month>(), "");
+  ASSERT_SAME_TYPE(month, decltype(std::declval<month&>()--));
+  ASSERT_SAME_TYPE(month&, decltype(--std::declval<month&>()));
 
-    for (unsigned i = 10; i <= 20; ++i)
-    {
-        month m(i);
-        assert(static_cast<unsigned>(--m) == i - 1);
-        assert(static_cast<unsigned>(m--) == i - 1);
-        assert(static_cast<unsigned>(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 af61d3442c3af3..1f8f7411eb2ebb 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 <chrono>
 #include <type_traits>
 #include <cassert>
 
 #include "test_macros.h"
 
-template <typename M>
-constexpr bool testConstexpr()
-{
-    M m1{1};
-    if (static_cast<unsigned>(++m1) != 2) return false;
-    if (static_cast<unsigned>(m1++) != 2) return false;
-    if (static_cast<unsigned>(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<unsigned>(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<unsigned>(m1) == exp);
+  }
+
+  return true;
 }
 
-int main(int, char**)
-{
-    using month = std::chrono::month;
-    ASSERT_NOEXCEPT(++(std::declval<month&>())  );
-    ASSERT_NOEXCEPT(  (std::declval<month&>())++);
+int main(int, char**) {
+  using month = std::chrono::month;
 
-    ASSERT_SAME_TYPE(month , decltype(  std::declval<month&>()++));
-    ASSERT_SAME_TYPE(month&, decltype(++std::declval<month&>()  ));
+  ASSERT_NOEXCEPT(++(std::declval<month&>()));
+  ASSERT_NOEXCEPT((std::declval<month&>())++);
 
-    static_assert(testConstexpr<month>(), "");
+  ASSERT_SAME_TYPE(month, decltype(std::declval<month&>()++));
+  ASSERT_SAME_TYPE(month&, decltype(++std::declval<month&>()));
 
-    for (unsigned i = 0; i <= 10; ++i)
-    {
-        month m(i);
-        assert(static_cast<unsigned>(++m) == i + 1);
-        assert(static_cast<unsigned>(m++) == i + 1);
-        assert(static_cast<unsigned>(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 848a96a9b73f7c..d825ef7d6cefa3 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 <typename M, typename Ms>
-constexpr bool testConstexpr()
-{
-    M m1{1};
-    if (static_cast<unsigned>(m1 += Ms{ 1}) !=  2) return false;
-    if (static_cast<unsigned>(m1 += Ms{ 2}) !=  4) return false;
-    if (static_cast<unsigned>(m1 += Ms{ 8}) != 12) return false;
-    if (static_cast<unsigned>(m1 -= Ms{ 1}) != 11) return false;
-    if (static_cast<unsigned>(m1 -= Ms{ 2}) !=  9) return false;
-    if (static_cast<unsigned>(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<unsigned>(m += months{10}) == static_cast<unsigned>(exp));
+    assert(static_cast<unsigned>(m) == static_cast<unsigned>(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<unsigned>(m -= months{9}) == static_cast<unsigned>(exp));
+    assert(static_cast<unsigned>(m) == static_cast<unsigned>(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<month&>() += std::declval<months&>());
-    ASSERT_NOEXCEPT(std::declval<month&>() -= std::declval<months&>());
-    ASSERT_SAME_TYPE(month&, decltype(std::declval<month&>() += std::declval<months&>()));
-    ASSERT_SAME_TYPE(month&, decltype(std::declval<month&>() -= std::declval<months&>()));
+  ASSERT_NOEXCEPT(std::declval<month&>() += std::declval<months&>());
+  ASSERT_NOEXCEPT(std::declval<month&>() -= std::declval<months&>());
+  ASSERT_SAME_TYPE(month&, decltype(std::declval<month&>() += std::declval<months&>()));
+  ASSERT_SAME_TYPE(month&, decltype(std::declval<month&>() -= std::declval<months&>()));
 
-    static_assert(testConstexpr<month, months>(), "");
-
-    for (unsigned i = 1; i <= 10; ++i)
-    {
-        month m(i);
-        int exp = i + 10;
-        while (exp > 12)
-            exp -= 12;
-        assert(static_cast<unsigned>(m += months{10}) == static_cast<unsigned>(exp));
-        assert(static_cast<unsigned>(m)               == static_cast<unsigned>(exp));
-    }
-
-    for (unsigned i = 1; i <= 10; ++i)
-    {
-        month m(i);
-        int exp = i - 9;
-        while (exp < 1)
-            exp += 12;
-        assert(static_cast<unsigned>(m -= months{ 9}) == static_cast<unsigned>(exp));
-        assert(static_cast<unsigned>(m)               == static_cast<unsigned>(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 f9ef5b60a86efb..2041afca5414b4 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<unsigned>(m1) == static_cast<unsigned>(exp));
-        // assert(off.count()            == static_cast<unsigned>(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 297df138ffc5e5..ee7a33e9e0b860 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 <chrono>
 #include <type_traits>
 #include <cassert>
 
 #include "test_macros.h"
 
-template <typename M, typename Ms>
-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<unsigned>(m1) == exp);
+    assert(static_cast<unsigned>(m2) == exp);
+  }
+
+  return true;
+}
 
-    ASSERT_NOEXCEPT(std::declval<month>() + std::declval<months>());
-    ASSERT_NOEXCEPT(std::declval<months>() + std::declval<month>());
+int main(int, char**) {
+  using month  = std::chrono::month;
+  using months = std::chrono::months;
 
-    ASSERT_SAME_TYPE(month, decltype(std::declval<month>()  + std::declval<months>()));
-    ASSERT_SAME_TYPE(month, decltype(std::declval<months>() + std::declval<month>() ));
+  ASSERT_NOEXCEPT(std::declval<month>() + std::declval<months>());
+  ASSERT_NOEXCEPT(std::declval<months>() + std::declval<month>());
 
-    static_assert(testConstexpr<month, months>(), "");
+  ASSERT_SAME_TYPE(month, decltype(std::declval<month>() + std::declval<months>()));
+  ASSERT_SAME_TYPE(month, decltype(std::declval<months>() + std::declval<month>()));
 
-    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<unsigned>(m1) == exp);
-        assert(static_cast<unsigned>(m2) == exp);
-    }
+  test();
+  static_assert(test());
 
-    return 0;
+  return 0;
 }


        


More information about the libcxx-commits mailing list