More calendar bits for <chrono>

[Mordante]

Summary

This patch adds the types leap and link to std::chrono.
These are "building block" types, that will be used when we get all the time zone stuff in.

Note the new type __undocumented, that denotes an undocumented call.
chrono is probably not the right place for this in the long run, I expect that it will find other uses.

[mclow.lists]

test/std/utilities/time/time.zone/time.zone.leap/types.pass.cpp

30 ↗

(On Diff #181680)

Tabs?

[EricWF]

include/chrono

2734 ↗

(On Diff #181680)

I don't think the name __undocumented describes what it is this does. It's used as a unspellable tag used to make constructors uncallable by the user. Maybe __private_ctor_t is a better name?

That said, I think I would prefer making the constructors private instead. Especially for types like link which should only be created in one or two places.

2739 ↗

(On Diff #181680)

Move operations should be noexcept.

2742 ↗

(On Diff #181680)

This signature smells funky.

I think it should have string_view parameters instead. That way we're not double copying a string.

Of course, if link is only ever constructed from arguments of type string&&, then the parameter type should be string and the arguments should be std::moveed .

2748 ↗

(On Diff #181680)

I think there is a better representation for this.

I have a couple of ideas, but it depends on what the implementation for time_zone and tzdb look like.
For example, the target() string should always correspond to a time_zone with the same name. We should be able to re-use that string instead of allocating another.

And if we can't re-use strings that we already store, I was thinking something like this:

struct Link2 {
    Link2(std::string_view xname, std::string_view xtarget) : name_len(xname.size()), target_len(xtarget.size()) {
        buff.reset(new char[name_len + target_len + 2] );
        memcpy(buff.get(), xname.data(), name_len + 1);
        memcpy(buff.get() + xname.size() + 1, xtarget.data(), target_len + 1);
    }
    
    std::string_view name() { return std::string_view(buff.get(), name_len); }
    std::string_view target() { return std::string_view(buff.get() + name_len + 1, target_len); }
    std::unique_ptr<char[]> buff;
    unsigned short name_len, target_len;
};

This gives us one allocation and a object size of 16 bytes (as opposed to 64), a less expensive move operation, and more consistent invalidation semantics on the string_view.

2752 ↗

(On Diff #181680)

These shouldn't be constexpr.

2765 ↗

(On Diff #181680)

The inline is redundant.

2765 ↗

(On Diff #181680)

This constructor shouldn't throw, so we can probably marke it as noexcept.

2782 ↗

(On Diff #181680)

What's with this indentation? clang-format all the things!

test/std/utilities/time/time.zone/time.zone.link/types.pass.cpp

30 ↗

(On Diff #181680)

Weird indentation.

[mclow.lists]

include/chrono

2734 ↗

(On Diff #181680)

My idea is that this will not be the only place we use __undocumented - and not just in constructors.

2748 ↗

(On Diff #181680)

You know what I like better? store a string and a string_view and construct from a string + time_zone. Note that an overwhelming majority of the names will fit in the short string optimization.

2752 ↗

(On Diff #181680)

Right. Copy/paste.

2765 ↗

(On Diff #181680)

You're right.

[mclow.lists]

Update based on Eric's comments.

include/chrono

2748 ↗

(On Diff #181680)

and more consistent invalidation semantics on the string_view.

The string_view should never be invalidated. Once the vector of links is created in the tzdb, then it never moves/changes/gets deallocated. Before the tzdb is created, no one calls name or target.

As for the allocations, as I said before, "an overwhelming majority of the names will fit in the short string optimization." (~560 out of ~590 across both zones and links). So most of the time, using strings involves NO allocations.

[ldionne]

Marshall, are you ready to commit this? I haven't looked deeply, but it looks like you had addressed Eric's comments?

libcxx/test/std/utilities/time/time.zone/time.zone.link/types.pass.cpp

35

Weird indentation.

[mclow.lists]

I'm fine with rebasing and committing this.

[curdeius]

Shouldn't this patch implement also the renames of leap and link?
https://wg21.link/P1981 Rename leap to leap_second
https://wg21.link/P1982 Rename link to time_zone_link

[mclow.lists]

In D56692#2497567, @curdeius wrote:

Shouldn't this patch implement also the renames of leap and link?

Yes. And re-do the comparisons to use spaceship.

[Mordante]

I'm working on a real implementation of the time zone implementation. Since we want to move to GitHub PRs I'm commandeering and closing this patch to clean up our queue.

libcxx/include/chrono

 #include <type_traits>
 #include <ratio>
 #include <limits>
+#include <string>
 #include <version>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)

 inline constexpr year_month_weekday_last& year_month_weekday_last::operator+=(const years& __dy)  noexcept { *this = *this + __dy; return *this; }
 inline constexpr year_month_weekday_last& year_month_weekday_last::operator-=(const years& __dy)  noexcept { *this = *this - __dy; return *this; }
 
+struct __undocumented { constexpr explicit __undocumented() noexcept = default; };
+
+class _LIBCPP_TYPE_VIS link
+{
+public:
+    link            (link&&) = default;
+    link& operator= (link&&) = default;
+
+    explicit link(string __nm, string __tg, __undocumented) : __name{__nm}, __target{__tg} {}
+
+    string_view name()   const noexcept { return __name; }
+    string_view target() const noexcept { return __target; }
+
+private:
+    string __name;
+    string __target;
+};
+
+inline bool operator==(const link& __lhs, const link& __rhs) noexcept {return __lhs.name() == __rhs.name();}
+inline bool operator< (const link& __lhs, const link& __rhs) noexcept {return __lhs.name()  < __rhs.name();}
+inline bool operator!=(const link& __lhs, const link& __rhs) noexcept {return !(__lhs == __rhs);}
+inline bool operator> (const link& __lhs, const link& __rhs) noexcept {return   __rhs  < __lhs;}
+inline bool operator<=(const link& __lhs, const link& __rhs) noexcept {return !(__rhs  < __lhs);}
+inline bool operator>=(const link& __lhs, const link& __rhs) noexcept {return !(__lhs  < __rhs);}
+
+
+class _LIBCPP_TYPE_VIS leap {
+public:
+    leap           (const leap&) _NOEXCEPT = default;
+    leap& operator=(const leap&) _NOEXCEPT = default;
+
+    explicit constexpr leap (const sys_seconds & __d, __undocumented) _NOEXCEPT : __date{__d} {}
+
+    constexpr sys_seconds date() const noexcept { return __date; }
+
+private:
+    sys_seconds __date;
+};
+
+
+inline constexpr bool operator==(const leap& __lhs, const leap& __rhs) noexcept {return __lhs.date() == __rhs.date();}
+inline constexpr bool operator< (const leap& __lhs, const leap& __rhs) noexcept {return __lhs.date()  < __rhs.date();}
+inline constexpr bool operator!=(const leap& __lhs, const leap& __rhs) noexcept {return !(__lhs == __rhs);}
+inline constexpr bool operator> (const leap& __lhs, const leap& __rhs) noexcept {return   __rhs  < __lhs;}
+inline constexpr bool operator<=(const leap& __lhs, const leap& __rhs) noexcept {return !(__rhs  < __lhs);}
+inline constexpr bool operator>=(const leap& __lhs, const leap& __rhs) noexcept {return !(__lhs  < __rhs);}
+
+template <class _Duration>
+constexpr bool operator==(const leap& __lhs, const sys_time<_Duration>& __rhs) noexcept { return __lhs.date() == __rhs; }
+
+template <class _Duration>
+constexpr bool operator==(const sys_time<_Duration>& __lhs, const leap& __rhs) noexcept { return __lhs == __rhs.date(); }
+  
+template <class _Duration>
+constexpr bool operator< (const leap& __lhs, const sys_time<_Duration>& __rhs) noexcept { return __lhs.date() < __rhs; }
+
+template <class _Duration>
+constexpr bool operator< (const sys_time<_Duration>& __lhs, const leap& __rhs) noexcept { return __lhs < __rhs.date(); }
+
+template <class _Duration>
+constexpr bool operator!=(const leap& __lhs, const sys_time<_Duration>& __rhs) noexcept {return !(__lhs == __rhs);}
+ 
+template <class _Duration>
+constexpr bool operator!=(const sys_time<_Duration>& __lhs, const leap& __rhs) noexcept {return !(__lhs == __rhs);}
+
+template <class _Duration>
+constexpr bool operator> (const leap& __lhs, const sys_time<_Duration>& __rhs) noexcept {return  (__rhs  < __lhs);}
+ 
+template <class _Duration>
+constexpr bool operator> (const sys_time<_Duration>& __lhs, const leap& __rhs) noexcept {return  (__rhs  < __lhs);}
+
+template <class _Duration>
+constexpr bool operator<=(const leap& __lhs, const sys_time<_Duration>& __rhs) noexcept {return !(__rhs < __lhs);}
+ 
+template <class _Duration>
+constexpr bool operator<=(const sys_time<_Duration>& __lhs, const leap& __rhs) noexcept {return !(__rhs < __lhs);}
+
+template <class _Duration>
+constexpr bool operator>=(const leap& __lhs, const sys_time<_Duration>& __rhs) noexcept {return !(__lhs < __rhs);}
+ 
+template <class _Duration>
+constexpr bool operator>=(const sys_time<_Duration>& __lhs, const leap& __rhs) noexcept {return !(__lhs < __rhs);}
+
 #endif // _LIBCPP_STD_VER > 17
 } // chrono
 

libcxx/test/libcxx/utilities/time/time.zone/time.zone.leap/comparisons.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+// UNSUPPORTED: c++98, c++03, c++11, c++14, c++17
+
+// <chrono>
+// class leap;
+
+//  inline explicit constexpr leap (const sys_seconds & __d, __undocumented) : __date{__d} /*?? noexcept*/ {}
+
+// bool operator==(const leap& x, const leap& y);
+// bool operator!=(const leap& x, const leap& y);
+// bool operator< (const leap& x, const leap& y);
+// bool operator> (const leap& x, const leap& y);
+// bool operator<=(const leap& x, const leap& y);
+// bool operator>=(const leap& x, const leap& y);
+// 
+// template<class Duration>
+//   bool operator==(const leap& x, const sys_time<Duration>& y);
+// template<class Duration>
+//   bool operator==(const sys_time<Duration>& x, const leap& y);
+// template<class Duration>
+//   bool operator!=(const leap& x, const sys_time<Duration>& y);
+// template<class Duration>
+//   bool operator!=(const sys_time<Duration>& x, const leap& y);
+// template<class Duration>
+//   bool operator< (const leap& x, const sys_time<Duration>& y);
+// template<class Duration>
+//   bool operator< (const sys_time<Duration>& x, const leap& y);
+// template<class Duration>
+//   bool operator> (const leap& x, const sys_time<Duration>& y);
+// template<class Duration>
+//   bool operator> (const sys_time<Duration>& x, const leap& y);
+// template<class Duration>
+//   bool operator<=(const leap& x, const sys_time<Duration>& y);
+// template<class Duration>
+//   bool operator<=(const sys_time<Duration>& x, const leap& y);
+// template<class Duration>
+//   bool operator>=(const leap& x, const sys_time<Duration>& y);
+// template<class Duration>
+//   bool operator>=(const sys_time<Duration>& x, const leap& y);
+
+ 
+#include <chrono>
+#include <type_traits>
+#include <cassert>
+
+#include "test_macros.h"
+#include "test_comparisons.h"
+
+
+int main()
+{
+    using leap          = std::chrono::leap;
+    using seconds       = std::chrono::seconds;
+    using hours         = std::chrono::hours;
+    using sys_seconds   = std::chrono::sys_seconds;
+    using ud            = std::chrono::__undocumented;
+
+    using sys_hours     = std::chrono::sys_time<hours>;
+
+    AssertComparisons6ReturnBool<leap>();
+    AssertComparisons6AreNoexcept<leap>();
+    
+    constexpr leap zero{sys_seconds{seconds{0}}, ud{}};
+    constexpr leap one {sys_seconds{seconds{1}}, ud{}};
+    constexpr leap two {sys_seconds{seconds{2}}, ud{}};
+    
+    static_assert(testComparisons6(zero, zero, true, false));
+    static_assert(testComparisons6(zero, one,  false, true));
+    assert(testComparisons6(zero, two, false, true));
+    assert(testComparisons6(two,  one, false, false));
+    
+    constexpr sys_hours h0{hours{0}};
+    constexpr sys_hours h1{hours{1}};
+
+    static_assert(testComparisons6(zero, h0,  true, false));
+    static_assert(testComparisons6(zero, h1, false, true));
+    assert(testComparisons6(one, h0, false, false));
+    assert(testComparisons6(one, h1, false, true));
+}

libcxx/test/libcxx/utilities/time/time.zone/time.zone.leap/ctor.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+// UNSUPPORTED: c++98, c++03, c++11, c++14, c++17
+
+// <chrono>
+// class leap;
+
+//  inline explicit constexpr leap (const sys_seconds & __d, __undocumented) : __date{__d} _NOEXCEPT {}
+//  constexpr sys_seconds date() const;
+
+#include <chrono>
+#include <type_traits>
+#include <cassert>
+
+#include "test_macros.h"
+
+int main()
+{
+    using leap        = std::chrono::leap;
+    using seconds     = std::chrono::seconds;
+    using sys_seconds = std::chrono::sys_seconds;
+    using ud          = std::chrono::__undocumented;
+
+    constexpr sys_seconds zero{seconds{0}};
+    constexpr sys_seconds one {seconds{1}};
+    
+    LIBCPP_ASSERT_NOEXCEPT(leap(zero, ud{}));
+    static_assert(leap(zero, ud{}).date() == zero, "");
+    static_assert(leap(one,  ud{}).date() != zero, "");
+    
+    assert(leap(zero, ud{}).date() != one);
+    assert(leap(one,  ud{}).date() == one);
+}

libcxx/test/libcxx/utilities/time/time.zone/time.zone.link/comparisons.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+// UNSUPPORTED: c++98, c++03, c++11, c++14, c++17
+
+// <chrono>
+// class link;
+
+//  explicit link (const string & name, const string & target __undocumented);
+
+// bool operator==(const leap& x, const leap& y) noexcept;
+// bool operator!=(const leap& x, const leap& y) noexcept;
+// bool operator< (const leap& x, const leap& y) noexcept;
+// bool operator> (const leap& x, const leap& y) noexcept;
+// bool operator<=(const leap& x, const leap& y) noexcept;
+// bool operator>=(const leap& x, const leap& y) noexcept;
+// 
+
+ 
+#include <chrono>
+#include <type_traits>
+#include <cassert>
+
+#include "test_macros.h"
+#include "test_comparisons.h"
+
+
+int main()
+{
+    using link        = std::chrono::link;
+    using ud          = std::chrono::__undocumented;
+
+    AssertComparisons6ReturnBool<link>();
+    AssertComparisons6AreNoexcept<link>();
+    
+    link aaa{"aaa", "t0", ud{}};
+    link bbb{"bbb", "t1", ud{}};
+    link ccc{"ccc", "t2", ud{}};
+    
+    assert(testComparisons6(aaa, aaa, true,  true));
+    assert(testComparisons6(aaa, bbb, false, true));
+    assert(testComparisons6(ccc, bbb, false, false));
+}

libcxx/test/libcxx/utilities/time/time.zone/time.zone.link/ctor.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+// UNSUPPORTED: c++98, c++03, c++11, c++14, c++17
+
+// <chrono>
+// class link;
+
+//  explicit link (const string & name, const string & target __undocumented);
+//  string_view name()   const noexcept;
+//  string_view target() const noexcept;
+      
+#include <chrono>
+#include <type_traits>
+#include <cassert>
+
+#include "test_macros.h"
+
+int main()
+{
+    using link        = std::chrono::link;
+    using ud          = std::chrono::__undocumented;
+
+    link zero{"name0", "target0", ud{}};
+    link one {"name1", "target1", ud{}};
+
+    assert(zero.name()   == "name0");
+    assert(zero.target() == "target0");
+    assert(one .name()   == "name1");
+    assert(one .target() == "target1");
+}

libcxx/test/std/utilities/time/time.zone/time.zone.leap/types.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+// UNSUPPORTED: c++98, c++03, c++11, c++14, c++17
+
+// <chrono>
+// class leap;
+
+//  There's not a lot we can do with leap here, since it has no documented
+//    constructors.  We just check the types and the noexcept-ness of the calls.
+
+// unspecified additional constructors
+//   constexpr sys_seconds date() const noexcept;
+
+#include <chrono>
+#include <type_traits>
+#include <cassert>
+
+#include "test_macros.h"
+#include "test_comparisons.h"
+
+int main()
+{
+    using leap        = std::chrono::leap;
+  using sys_seconds = std::chrono::sys_seconds;
+
+    using sys_hours     = std::chrono::sys_time<std::chrono::hours>;
+  
+  ASSERT_NOEXCEPT(                       std::declval<const leap&>().date());
+  ASSERT_SAME_TYPE(sys_seconds, decltype(std::declval<const leap&>().date()));
+
+    AssertComparisons6ReturnBool<leap>();
+    AssertComparisons6AreNoexcept<leap>();
+    AssertComparisons6ReturnBool<leap, sys_hours>();
+    AssertComparisons6AreNoexcept<leap, sys_hours>();
+}

libcxx/test/std/utilities/time/time.zone/time.zone.link/types.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+// UNSUPPORTED: c++98, c++03, c++11, c++14, c++17
+
+// <chrono>
+// class link;
+
+//  There's not a lot we can do with link here, since it has no documented
+//    constructors.  We just check the types and the noexcept-ness of the calls.
+
+// unspecified additional constructors
+//   string_view name()   const noexcept;
+//   string_view target() const noexcept;
+
+#include <chrono>
+#include <type_traits>
+#include <cassert>
+
+#include "test_macros.h"
+#include "test_comparisons.h"
+
+int main()
+{
+    using link        = std::chrono::link;
+
+  ASSERT_NOEXCEPT(                            std::declval<const link&>().name());
+  ASSERT_SAME_TYPE(std::string_view, decltype(std::declval<const link&>().name()));
+
+  ASSERT_NOEXCEPT(                            std::declval<const link&>().target());
+  ASSERT_SAME_TYPE(std::string_view, decltype(std::declval<const link&>().target()));
+
+    AssertComparisons6ReturnBool<link>();
+    AssertComparisons6AreNoexcept<link>();
+}