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

Tabs?

[EricWF]

include/chrono

2734

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

Move operations should be noexcept.

2742

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

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

These shouldn't be constexpr.

2765

The inline is redundant.

2765

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

2782

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

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

30

Weird indentation.

[mclow.lists]

include/chrono

2734

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

2748

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

Right. Copy/paste.

2765

You're right.

[mclow.lists]

Update based on Eric's comments.

include/chrono

2748

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 ↗

(On Diff #184786)

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.

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;
+
+    inline explicit link(string __nm, string __tg, __undocumented) : __name{__nm}, __target{__tg} {}
+
+    inline string_view name()   const noexcept { return __name; }
+    inline string_view target() const noexcept { return __target; }
+
+private:
+    string __name;
+    string __target;
+};
+
+inline constexpr bool operator==(const link& __lhs, const link& __rhs) noexcept {return __lhs.name() == __rhs.name();}
+inline constexpr bool operator< (const link& __lhs, const link& __rhs) noexcept {return __lhs.name()  < __rhs.name();}
+inline constexpr bool operator!=(const link& __lhs, const link& __rhs) noexcept {return !(__lhs == __rhs);}
+inline constexpr bool operator> (const link& __lhs, const link& __rhs) noexcept {return   __rhs  < __lhs;}
+inline constexpr bool operator<=(const link& __lhs, const link& __rhs) noexcept {return !(__rhs  < __lhs);}
+inline constexpr bool operator>=(const link& __lhs, const link& __rhs) noexcept {return !(__lhs  < __rhs);}
+
+
+class _LIBCPP_TYPE_VIS leap {
+public:
+    leap           (const leap&) = default;
+    leap& operator=(const leap&) = default;
+
+    inline explicit constexpr leap (const sys_seconds & __d, __undocumented) : __date{__d} /*?? noexcept*/ {}
+
+    inline 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
 

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));
+}

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}};
+    
+    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);
+}

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));
+}

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");
+}

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>();
+}

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>();
+}