[llvm-branch-commits] [libcxx] [libc++][TZDB] Implements time_zone::to_sys. (PR #90394)

2024-06-04 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne approved this pull request.

LGTM w/ comments applied (in particular the missing tests).

https://github.com/llvm/llvm-project/pull/90394
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Implements time_zone::to_sys. (PR #90394)

2024-06-04 Thread Louis Dionne via llvm-branch-commits


@@ -0,0 +1,20 @@
+//===--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include 
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+namespace chrono {
+
+_LIBCPP_AVAILABILITY_TZDB _LIBCPP_EXPORTED_FROM_ABI 
nonexistent_local_time::~nonexistent_local_time() = default;
+_LIBCPP_AVAILABILITY_TZDB _LIBCPP_EXPORTED_FROM_ABI 
ambiguous_local_time::~ambiguous_local_time() = default;

ldionne wrote:

```suggestion
_LIBCPP_AVAILABILITY_TZDB _LIBCPP_EXPORTED_FROM_ABI 
nonexistent_local_time::~nonexistent_local_time() = default; // key function
_LIBCPP_AVAILABILITY_TZDB _LIBCPP_EXPORTED_FROM_ABI 
ambiguous_local_time::~ambiguous_local_time() = default; // key function
```

https://github.com/llvm/llvm-project/pull/90394
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Implements time_zone::to_sys. (PR #90394)

2024-06-04 Thread Louis Dionne via llvm-branch-commits


@@ -0,0 +1,33 @@
+//===--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+
+// XFAIL: libcpp-has-no-experimental-tzdb
+// XFAIL: availability-tzdb-missing
+
+// 
+
+// class nonexistent_local_time : public runtime_error {
+// public:
+//   template
+// nonexistent_local_time(const local_time& tp, const 
local_info& i);
+// };
+
+#include 
+#include 
+#include 
+
+// Basic properties
+static_assert(std::is_base_of_v);
+static_assert(!std::is_default_constructible_v);
+static_assert(std::is_destructible_v);
+static_assert(std::is_copy_constructible_v);
+static_assert(std::is_move_constructible_v);
+static_assert(std::is_copy_assignable_v);
+static_assert(std::is_move_assignable_v);

ldionne wrote:

Instead of just asserting those traits, you should have actual runtime tests 
that exercise these functions.

For example if we declared the copy constructor as non-inline in the headers 
and never defined it in the dylib, this test would pass but we'd want it to 
fail.

https://github.com/llvm/llvm-project/pull/90394
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Implements time_zone::to_sys. (PR #90394)

2024-06-04 Thread Louis Dionne via llvm-branch-commits


@@ -70,6 +72,30 @@ class _LIBCPP_AVAILABILITY_TZDB time_zone {
 return __get_info(chrono::time_point_cast(__time));
   }
 
+  // Since the interface promisses throwing, don't add nodiscard.

ldionne wrote:

I don't think this comment is necessary -- we assume no `nodiscard` by default 
anyway, and I think anyone pausing to think whether this should be made 
`nodiscard` will easily come to the same conclusion.

Or if you want to keep the comment, I would reword to `We don't apply nodiscard 
here since this function throws on many inputs, so it could be used as a 
validation`.

https://github.com/llvm/llvm-project/pull/90394
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Implements time_zone::to_sys. (PR #90394)

2024-06-04 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne edited 
https://github.com/llvm/llvm-project/pull/90394
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Implements time_zone::to_sys. (PR #90394)

2024-06-04 Thread Louis Dionne via llvm-branch-commits


@@ -339,6 +339,9 @@ if (LIBCXX_ENABLE_LOCALIZATION AND LIBCXX_ENABLE_FILESYSTEM 
AND LIBCXX_ENABLE_TI
 include/tzdb/types_private.h
 include/tzdb/tzdb_list_private.h
 include/tzdb/tzdb_private.h
+# TODO TZDB The exception could be moved in chrono once the TZDB library
+# is no longer experimental.
+chrono_exception.cpp
 time_zone.cpp
 tzdb.cpp
 tzdb_list.cpp

ldionne wrote:

Those should all move to `libcxx/src/experimental/`. Can be done in a separate 
patch.

https://github.com/llvm/llvm-project/pull/90394
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Implements time_zone::to_sys. (PR #90901)

2024-06-04 Thread Louis Dionne via llvm-branch-commits


@@ -0,0 +1,37 @@
+//===--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+// UNSUPPORTED: no-filesystem, no-localization, no-tzdb
+
+// XFAIL: libcpp-has-no-experimental-tzdb
+// XFAIL: availability-tzdb-missing
+
+// 
+
+// enaum class choose;

ldionne wrote:

```suggestion
// enum class choose;
```

https://github.com/llvm/llvm-project/pull/90901
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Implements time_zone::to_sys. (PR #90901)

2024-06-04 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne edited 
https://github.com/llvm/llvm-project/pull/90901
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Implements time_zone::to_sys. (PR #90901)

2024-06-04 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne approved this pull request.


https://github.com/llvm/llvm-project/pull/90901
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Implements time_zone::to_local. (PR #91003)

2024-06-04 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne edited 
https://github.com/llvm/llvm-project/pull/91003
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Implements time_zone::to_local. (PR #91003)

2024-06-04 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne approved this pull request.


https://github.com/llvm/llvm-project/pull/91003
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Implements time_zone::to_local. (PR #91003)

2024-06-04 Thread Louis Dionne via llvm-branch-commits


@@ -0,0 +1,66 @@
+//===--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+// UNSUPPORTED: no-filesystem, no-localization, no-tzdb
+
+// XFAIL: libcpp-has-no-experimental-tzdb
+// XFAIL: availability-tzdb-missing
+
+// 
+
+// class time_zone;
+
+// template 
+// local_time>
+//   to_local(const sys_time& tp) const;
+
+#include 
+#include 
+#include 
+#include 
+
+#include "test_macros.h"
+#include "assert_macros.h"
+#include "concat_macros.h"
+
+int main(int, char**) {
+  // To make sure the test does not depend on changes in the database it uses a
+  // time zone with a fixed offset.
+  using namespace std::literals::chrono_literals;
+
+  const std::chrono::time_zone* tz = std::chrono::locate_zone("Etc/GMT+1");
+
+  assert(tz->to_local(std::chrono::sys_time{-1ns}) ==
+ std::chrono::local_time{-1ns - 1h});
+
+  assert(tz->to_local(std::chrono::sys_time{0us}) ==
+ std::chrono::local_time{0us - 1h});
+
+  assert(tz->to_local(
+ 
std::chrono::sys_time{std::chrono::sys_days{std::chrono::January
 / 1 / -21970}}) ==
+ std::chrono::local_time{
+ (std::chrono::sys_days{std::chrono::January / 1 / 
-21970}).time_since_epoch() - 1h});
+
+  assert(
+  
tz->to_local(std::chrono::sys_time{std::chrono::sys_days{std::chrono::January
 / 1 / 21970}}) ==
+  std::chrono::local_time{
+  (std::chrono::sys_days{std::chrono::January / 1 / 
21970}).time_since_epoch() - 1h});
+
+  assert(tz->to_local(std::chrono::sys_time{}) ==
+ std::chrono::local_time{
+ (std::chrono::sys_days{std::chrono::January / 1 / 
1970}).time_since_epoch() - 1h});
+
+  assert(tz->to_local(std::chrono::sys_time{}) ==
+ std::chrono::local_time{
+ (std::chrono::sys_days{std::chrono::January / 1 / 
1970}).time_since_epoch() - 1h});
+
+  assert(tz->to_local(std::chrono::sys_time{}) ==
+ std::chrono::local_time{
+ (std::chrono::sys_days{std::chrono::January / 1 / 
1970}).time_since_epoch() - 1h});
+}

ldionne wrote:

`return 0;`

https://github.com/llvm/llvm-project/pull/91003
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Implements zoned_traits. (PR #91059)

2024-06-04 Thread Louis Dionne via llvm-branch-commits


@@ -0,0 +1,32 @@
+//===--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+// UNSUPPORTED: no-filesystem, no-localization, no-tzdb
+
+// XFAIL: libcpp-has-no-experimental-tzdb
+// XFAIL: availability-tzdb-missing
+
+// 
+
+// template struct zoned_traits {};
+//
+// A specialization for const time_zone* is provided by the implementation:
+// template<> struct zoned_traits { ... }
+
+#include 
+#include 
+
+// This test test whether non-specialized versions exhibit the expected
+// behavior. (Note these specializations are not really useful.)
+static_assert(std::is_trivial_v>);

ldionne wrote:

I think we actually need to check `is_empty_v && is_trivial_v` here. Otherwise 
a type like this would pass the `is_trivial` test:

```
struct foo {
foo();
};
```

https://github.com/llvm/llvm-project/pull/91059
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Implements zoned_traits. (PR #91059)

2024-06-04 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne edited 
https://github.com/llvm/llvm-project/pull/91059
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Implements zoned_traits. (PR #91059)

2024-06-04 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne approved this pull request.

LGTM w/ minor comment.

https://github.com/llvm/llvm-project/pull/91059
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][chrono] Fixes leap seconds. (PR #90070)

2024-06-04 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne edited 
https://github.com/llvm/llvm-project/pull/90070
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][chrono] Fixes leap seconds. (PR #90070)

2024-06-04 Thread Louis Dionne via llvm-branch-commits


@@ -626,29 +626,49 @@ static void __parse_leap_seconds(vector& 
__leap_seconds, istream&&
   // seconds since 1 January 1970.
   constexpr auto __offset = sys_days{1970y / January / 1} - sys_days{1900y / 
January / 1};
 
-  while (true) {
-switch (__input.peek()) {
-case istream::traits_type::eof():
-  return;
-
-case ' ':
-case '\t':
-case '\n':
-  __input.get();
-  continue;
+  struct __entry {
+sys_seconds __timestamp;
+seconds __value;
+  };
+  vector<__entry> __entries;
+  [&] {
+while (true) {
+  switch (__input.peek()) {
+  case istream::traits_type::eof():
+return;
+
+  case ' ':
+  case '\t':
+  case '\n':
+__input.get();
+continue;
+
+  case '#':
+chrono::__skip_line(__input);
+continue;
+  }
 
-case '#':
+  sys_seconds __date = 
sys_seconds{seconds{chrono::__parse_integral(__input, false)}} - __offset;
+  chrono::__skip_mandatory_whitespace(__input);
+  seconds __value{chrono::__parse_integral(__input, false)};
   chrono::__skip_line(__input);
-  continue;
-}
 
-sys_seconds __date = sys_seconds{seconds{chrono::__parse_integral(__input, 
false)}} - __offset;
-chrono::__skip_mandatory_whitespace(__input);
-seconds __value{chrono::__parse_integral(__input, false)};
-chrono::__skip_line(__input);
-
-__leap_seconds.emplace_back(std::__private_constructor_tag{}, __date, 
__value);
-  }
+  __entries.emplace_back(__date, __value);
+}
+  }();
+  // The Standard requires the leap seconds to be sorted. The file
+  // leap-seconds.list usually provides them in sorted order, but that is not
+  // guaranteed so we ensure it here.
+  std::ranges::sort(__entries, {}, &__entry::__timestamp);

ldionne wrote:

```suggestion
  ranges::sort(__entries, {}, &__entry::__timestamp);
```

https://github.com/llvm/llvm-project/pull/90070
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][chrono] Fixes leap seconds. (PR #90070)

2024-06-04 Thread Louis Dionne via llvm-branch-commits


@@ -626,29 +626,49 @@ static void __parse_leap_seconds(vector& 
__leap_seconds, istream&&
   // seconds since 1 January 1970.
   constexpr auto __offset = sys_days{1970y / January / 1} - sys_days{1900y / 
January / 1};
 
-  while (true) {
-switch (__input.peek()) {
-case istream::traits_type::eof():
-  return;
-
-case ' ':
-case '\t':
-case '\n':
-  __input.get();
-  continue;
+  struct __entry {
+sys_seconds __timestamp;
+seconds __value;
+  };
+  vector<__entry> __entries;
+  [&] {
+while (true) {
+  switch (__input.peek()) {
+  case istream::traits_type::eof():
+return;
+
+  case ' ':
+  case '\t':
+  case '\n':
+__input.get();
+continue;
+
+  case '#':
+chrono::__skip_line(__input);
+continue;
+  }
 
-case '#':
+  sys_seconds __date = 
sys_seconds{seconds{chrono::__parse_integral(__input, false)}} - __offset;
+  chrono::__skip_mandatory_whitespace(__input);
+  seconds __value{chrono::__parse_integral(__input, false)};
   chrono::__skip_line(__input);
-  continue;
-}
 
-sys_seconds __date = sys_seconds{seconds{chrono::__parse_integral(__input, 
false)}} - __offset;
-chrono::__skip_mandatory_whitespace(__input);
-seconds __value{chrono::__parse_integral(__input, false)};
-chrono::__skip_line(__input);
-
-__leap_seconds.emplace_back(std::__private_constructor_tag{}, __date, 
__value);
-  }
+  __entries.emplace_back(__date, __value);
+}
+  }();
+  // The Standard requires the leap seconds to be sorted. The file
+  // leap-seconds.list usually provides them in sorted order, but that is not
+  // guaranteed so we ensure it here.
+  std::ranges::sort(__entries, {}, &__entry::__timestamp);
+
+  // The database should contain the number of seconds inserted by a leap
+  // second (1 or -1). So the difference between the two elements are stored.

ldionne wrote:

```suggestion
  // second (1 or -1). So the difference between the two elements is stored.
```

https://github.com/llvm/llvm-project/pull/90070
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][chrono] Fixes leap seconds. (PR #90070)

2024-06-04 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne approved this pull request.


https://github.com/llvm/llvm-project/pull/90070
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++] Implement std::move_only_function (P0288R9) (PR #94670)

2024-06-11 Thread Louis Dionne via llvm-branch-commits


@@ -0,0 +1,233 @@
+//===--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+// This header is unguarded on purpose. This header is an implementation 
detail of move_only_function.h
+// and generates multiple versions of std::move_only_function
+
+#include <__config>
+#include <__functional/invoke.h>
+#include <__functional/move_only_function_common.h>
+#include <__type_traits/is_trivially_destructible.h>
+#include <__utility/exchange.h>
+#include <__utility/forward.h>
+#include <__utility/in_place.h>
+#include <__utility/move.h>
+#include <__utility/pointer_int_pair.h>
+#include <__utility/small_buffer.h>
+#include <__utility/swap.h>
+#include 
+#include 
+#include 
+#include 
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#  pragma GCC system_header
+#endif
+
+#ifndef _LIBCPP_IN_MOVE_ONLY_FUNCTION_H
+#  error This header should only be included from move_only_function.h
+#endif
+
+#ifndef _LIBCPP_MOVE_ONLY_FUNCTION_CV
+#  define _LIBCPP_MOVE_ONLY_FUNCTION_CV
+#endif
+
+#ifndef _LIBCPP_MOVE_ONLY_FUNCTION_REF
+#  define _LIBCPP_MOVE_ONLY_FUNCTION_REF
+#  define _LIBCPP_MOVE_ONLY_FUNCTION_INVOKE_QUALS 
_LIBCPP_MOVE_ONLY_FUNCTION_CV&
+#else
+#  define _LIBCPP_MOVE_ONLY_FUNCTION_INVOKE_QUALS 
_LIBCPP_MOVE_ONLY_FUNCTION_CV _LIBCPP_MOVE_ONLY_FUNCTION_REF
+#endif
+
+#ifndef _LIBCPP_MOVE_ONLY_FUNCTION_NOEXCEPT
+#  define _LIBCPP_MOVE_ONLY_FUNCTION_NOEXCEPT false
+#endif
+
+#define _LIBCPP_MOVE_ONLY_FUNCTION_CVREF _LIBCPP_MOVE_ONLY_FUNCTION_CV 
_LIBCPP_MOVE_ONLY_FUNCTION_REF
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+#ifdef _LIBCPP_ABI_MOVE_ONLY_FUNCTION_TRIVIAL_ABI
+#  define _LIBCPP_MOVE_ONLY_FUNCTION_TRIVIAL_ABI [[_Clang::__trivial_abi__]]
+#else
+#  define _LIBCPP_MOVE_ONLY_FUNCTION_TRIVIAL_ABI
+#endif
+
+template 
+class move_only_function;
+
+template 
+class _LIBCPP_MOVE_ONLY_FUNCTION_TRIVIAL_ABI move_only_function<_ReturnT(
+_ArgTypes...) _LIBCPP_MOVE_ONLY_FUNCTION_CVREF 
noexcept(_LIBCPP_MOVE_ONLY_FUNCTION_NOEXCEPT)> {
+private:
+  static constexpr size_t __buffer_size_  = 3 * sizeof(void*);
+  static constexpr size_t __buffer_alignment_ = alignof(void*);
+  using _BufferT  = __small_buffer<__buffer_size_, 
__buffer_alignment_>;
+
+  using _TrivialVTable= _MoveOnlyFunctionTrivialVTable<_BufferT, _ReturnT, 
_ArgTypes...>;
+  using _NonTrivialVTable = _MoveOnlyFunctionNonTrivialVTable<_BufferT, 
_ReturnT, _ArgTypes...>;
+
+  template 
+  static constexpr _TrivialVTable __trivial_vtable_ = {
+  .__call_ = [](_BufferT& __buffer, _ArgTypes... __args) 
noexcept(_LIBCPP_MOVE_ONLY_FUNCTION_NOEXCEPT) -> _ReturnT {
+return std::invoke_r<_ReturnT>(
+static_cast<_Functor 
_LIBCPP_MOVE_ONLY_FUNCTION_INVOKE_QUALS>(*__buffer.__get<_Functor>()),
+std::forward<_ArgTypes>(__args)...);
+  }};
+
+  template 
+  static constexpr _NonTrivialVTable __non_trivial_vtable_{
+  __trivial_vtable_<_Functor>,
+  [](_BufferT& __buffer) noexcept -> void {
+std::destroy_at(__buffer.__get<_Functor>());
+__buffer.__dealloc<_Functor>();
+  },
+  };
+
+  template 
+  _LIBCPP_HIDE_FROM_ABI __pointer_bool_pair 
__get_vptr() {
+if constexpr (_BufferT::__fits_in_buffer<_Functor> && 
is_trivially_destructible_v<_Functor>) {
+  return {&__trivial_vtable_<_Functor>, false};
+} else {
+  return {&__non_trivial_vtable_<_Functor>, true};
+}
+  }
+
+  template 
+  static constexpr bool __is_callable_from = [] {
+using _DVT = decay_t<_VT>;
+if (_LIBCPP_MOVE_ONLY_FUNCTION_NOEXCEPT) {
+  return is_nothrow_invocable_r_v<_ReturnT, _DVT 
_LIBCPP_MOVE_ONLY_FUNCTION_CVREF, _ArgTypes...> &&
+ is_nothrow_invocable_r_v<_ReturnT, _DVT 
_LIBCPP_MOVE_ONLY_FUNCTION_INVOKE_QUALS, _ArgTypes...>;
+} else {
+  return is_invocable_r_v<_ReturnT, _DVT _LIBCPP_MOVE_ONLY_FUNCTION_CVREF, 
_ArgTypes...> &&
+ is_invocable_r_v<_ReturnT, _DVT 
_LIBCPP_MOVE_ONLY_FUNCTION_INVOKE_QUALS, _ArgTypes...>;
+}
+  }();
+
+  template 
+  _LIBCPP_HIDE_FROM_ABI void __construct(_Args&&... __args) {
+static_assert(is_constructible_v, _Func>);
+
+using _StoredFunc = decay_t<_Func>;
+__vtable_ = __get_vptr<_StoredFunc>();
+__buffer_.__construct<_StoredFunc>(std::forward<_Args>(__args)...);
+  }
+
+  _LIBCPP_HIDE_FROM_ABI void __reset() {
+if (__vtable_.__get_value())

ldionne wrote:

> I've provided a complete & test-passing implementation to compare against. 
> It's the baseline I used for this review. I'm confident my results are 
> repeatable.

Sorry, I'm trying to follow the discussion after the fact but I can't find 
where you posted that implem

[llvm-branch-commits] [libcxx] [libc++][TZDB] Finishes zoned_time constructors. (PR #95010)

2024-06-18 Thread Louis Dionne via llvm-branch-commits


@@ -76,12 +81,71 @@ class zoned_time {
 
   _LIBCPP_HIDE_FROM_ABI explicit zoned_time(string_view __name)
 requires(requires { __traits::locate_zone(string_view{}); } &&
- // constructible_from
- // would create a dependency on itself. Instead depend on the fact
- // a constructor taking a _TimeZonePtr exists.
  constructible_from<_TimeZonePtr, 
decltype(__traits::locate_zone(string_view{}))>)
   : __zone_{__traits::locate_zone(__name)}, __tp_{} {}
 
+  template 
+  _LIBCPP_HIDE_FROM_ABI zoned_time(const zoned_time<_Duration2, _TimeZonePtr>& 
__zt)
+requires is_convertible_v, sys_time<_Duration>>
+  : __zone_{__zt.get_time_zone()}, __tp_{__zt.get_sys_time()} {}
+
+  _LIBCPP_HIDE_FROM_ABI zoned_time(_TimeZonePtr __zone, const 
sys_time<_Duration>& __tp)
+  : __zone_{std::move(__zone)}, __tp_{__tp} {}
+
+  _LIBCPP_HIDE_FROM_ABI zoned_time(string_view __name, const 
sys_time<_Duration>& __tp)
+requires requires { _TimeZonePtr{__traits::locate_zone(string_view{})}; }
+  : zoned_time{__traits::locate_zone(__name), __tp} {}
+
+  _LIBCPP_HIDE_FROM_ABI zoned_time(_TimeZonePtr __zone, const 
local_time<_Duration>& __tp)
+requires(is_convertible_v() -> 
to_sys(local_time<_Duration>{})),
+  sys_time>)
+  : __zone_{std::move(__zone)}, __tp_{__zone_->to_sys(__tp)} {}
+
+  _LIBCPP_HIDE_FROM_ABI zoned_time(string_view __name, const 
local_time<_Duration>& __tp)
+requires(requires {
+  _TimeZonePtr{__traits::locate_zone(string_view{})};
+} && is_convertible_v() -> 
to_sys(local_time<_Duration>{})),

ldionne wrote:

```suggestion
requires is_constructible && 
is_convertible_v() -> 
to_sys(local_time<_Duration>{})),
```

The constraint in http://eel.is/c++draft/time.zone#zonedtime.ctor-18.sentence-1 
is:
> Constraints: zoned_time is constructible from the return type of 
> `traits​::​locate_zone(name)` and `tp`.

https://github.com/llvm/llvm-project/pull/95010
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Finishes zoned_time constructors. (PR #95010)

2024-06-18 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne requested changes to this pull request.

This generally LGTM but the constraints seem wrong, so I'd like to see again 
once that is figured out.

https://github.com/llvm/llvm-project/pull/95010
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Finishes zoned_time constructors. (PR #95010)

2024-06-18 Thread Louis Dionne via llvm-branch-commits


@@ -76,12 +81,71 @@ class zoned_time {
 
   _LIBCPP_HIDE_FROM_ABI explicit zoned_time(string_view __name)
 requires(requires { __traits::locate_zone(string_view{}); } &&
- // constructible_from
- // would create a dependency on itself. Instead depend on the fact
- // a constructor taking a _TimeZonePtr exists.
  constructible_from<_TimeZonePtr, 
decltype(__traits::locate_zone(string_view{}))>)
   : __zone_{__traits::locate_zone(__name)}, __tp_{} {}
 
+  template 
+  _LIBCPP_HIDE_FROM_ABI zoned_time(const zoned_time<_Duration2, _TimeZonePtr>& 
__zt)
+requires is_convertible_v, sys_time<_Duration>>
+  : __zone_{__zt.get_time_zone()}, __tp_{__zt.get_sys_time()} {}
+
+  _LIBCPP_HIDE_FROM_ABI zoned_time(_TimeZonePtr __zone, const 
sys_time<_Duration>& __tp)
+  : __zone_{std::move(__zone)}, __tp_{__tp} {}
+
+  _LIBCPP_HIDE_FROM_ABI zoned_time(string_view __name, const 
sys_time<_Duration>& __tp)
+requires requires { _TimeZonePtr{__traits::locate_zone(string_view{})}; }

ldionne wrote:

Similar comment about the constraint here.

https://github.com/llvm/llvm-project/pull/95010
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Finishes zoned_time constructors. (PR #95010)

2024-06-18 Thread Louis Dionne via llvm-branch-commits


@@ -0,0 +1,105 @@
+//===--===/
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+// UNSUPPORTED: no-filesystem, no-localization, no-tzdb
+
+// XFAIL: libcpp-has-no-experimental-tzdb
+// XFAIL: availability-tzdb-missing
+
+// 
+
+// template
+// class zoned_time;
+//
+// zoned_time(string_view name, const local_time& st, choose c);
+
+#include 
+#include 
+#include 
+
+#include "test_offset_time_zone.h"
+
+int main(int, char**) {
+  // Tests unique conversions. To make sure the test is does not depend on 
changes
+  // in the database it uses a time zone with a fixed offset.

ldionne wrote:

```suggestion
  // Tests unique conversions. To make sure the test does not depend on changes
  // in the database it uses a time zone with a fixed offset.
```

https://github.com/llvm/llvm-project/pull/95010
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Finishes zoned_time constructors. (PR #95010)

2024-06-18 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne edited 
https://github.com/llvm/llvm-project/pull/95010
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Finishes zoned_time constructors. (PR #95010)

2024-06-18 Thread Louis Dionne via llvm-branch-commits


@@ -76,12 +81,71 @@ class zoned_time {
 
   _LIBCPP_HIDE_FROM_ABI explicit zoned_time(string_view __name)
 requires(requires { __traits::locate_zone(string_view{}); } &&
- // constructible_from
- // would create a dependency on itself. Instead depend on the fact
- // a constructor taking a _TimeZonePtr exists.
  constructible_from<_TimeZonePtr, 
decltype(__traits::locate_zone(string_view{}))>)
   : __zone_{__traits::locate_zone(__name)}, __tp_{} {}
 
+  template 
+  _LIBCPP_HIDE_FROM_ABI zoned_time(const zoned_time<_Duration2, _TimeZonePtr>& 
__zt)
+requires is_convertible_v, sys_time<_Duration>>
+  : __zone_{__zt.get_time_zone()}, __tp_{__zt.get_sys_time()} {}
+
+  _LIBCPP_HIDE_FROM_ABI zoned_time(_TimeZonePtr __zone, const 
sys_time<_Duration>& __tp)
+  : __zone_{std::move(__zone)}, __tp_{__tp} {}
+
+  _LIBCPP_HIDE_FROM_ABI zoned_time(string_view __name, const 
sys_time<_Duration>& __tp)
+requires requires { _TimeZonePtr{__traits::locate_zone(string_view{})}; }
+  : zoned_time{__traits::locate_zone(__name), __tp} {}
+
+  _LIBCPP_HIDE_FROM_ABI zoned_time(_TimeZonePtr __zone, const 
local_time<_Duration>& __tp)
+requires(is_convertible_v() -> 
to_sys(local_time<_Duration>{})),
+  sys_time>)
+  : __zone_{std::move(__zone)}, __tp_{__zone_->to_sys(__tp)} {}
+
+  _LIBCPP_HIDE_FROM_ABI zoned_time(string_view __name, const 
local_time<_Duration>& __tp)
+requires(requires {
+  _TimeZonePtr{__traits::locate_zone(string_view{})};
+} && is_convertible_v() -> 
to_sys(local_time<_Duration>{})),
+  sys_time>)
+  : zoned_time{__traits::locate_zone(__name), __tp} {}
+
+  _LIBCPP_HIDE_FROM_ABI zoned_time(_TimeZonePtr __zone, const 
local_time<_Duration>& __tp, choose __c)
+requires(is_convertible_v<
+decltype(std::declval<_TimeZonePtr&>() -> 
to_sys(local_time<_Duration>{}, choose::earliest)),
+sys_time>)
+  : __zone_{std::move(__zone)}, __tp_{__zone_->to_sys(__tp, __c)} {}
+
+  _LIBCPP_HIDE_FROM_ABI zoned_time(string_view __name, const 
local_time<_Duration>& __tp, choose __c)
+requires(requires {

ldionne wrote:

Constraint.

https://github.com/llvm/llvm-project/pull/95010
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Finishes zoned_time constructors. (PR #95010)

2024-06-18 Thread Louis Dionne via llvm-branch-commits


@@ -76,12 +81,71 @@ class zoned_time {
 
   _LIBCPP_HIDE_FROM_ABI explicit zoned_time(string_view __name)
 requires(requires { __traits::locate_zone(string_view{}); } &&
- // constructible_from
- // would create a dependency on itself. Instead depend on the fact
- // a constructor taking a _TimeZonePtr exists.
  constructible_from<_TimeZonePtr, 
decltype(__traits::locate_zone(string_view{}))>)
   : __zone_{__traits::locate_zone(__name)}, __tp_{} {}
 
+  template 
+  _LIBCPP_HIDE_FROM_ABI zoned_time(const zoned_time<_Duration2, _TimeZonePtr>& 
__zt)
+requires is_convertible_v, sys_time<_Duration>>
+  : __zone_{__zt.get_time_zone()}, __tp_{__zt.get_sys_time()} {}
+
+  _LIBCPP_HIDE_FROM_ABI zoned_time(_TimeZonePtr __zone, const 
sys_time<_Duration>& __tp)
+  : __zone_{std::move(__zone)}, __tp_{__tp} {}
+
+  _LIBCPP_HIDE_FROM_ABI zoned_time(string_view __name, const 
sys_time<_Duration>& __tp)
+requires requires { _TimeZonePtr{__traits::locate_zone(string_view{})}; }
+  : zoned_time{__traits::locate_zone(__name), __tp} {}
+
+  _LIBCPP_HIDE_FROM_ABI zoned_time(_TimeZonePtr __zone, const 
local_time<_Duration>& __tp)
+requires(is_convertible_v() -> 
to_sys(local_time<_Duration>{})),
+  sys_time>)
+  : __zone_{std::move(__zone)}, __tp_{__zone_->to_sys(__tp)} {}
+
+  _LIBCPP_HIDE_FROM_ABI zoned_time(string_view __name, const 
local_time<_Duration>& __tp)
+requires(requires {
+  _TimeZonePtr{__traits::locate_zone(string_view{})};
+} && is_convertible_v() -> 
to_sys(local_time<_Duration>{})),
+  sys_time>)
+  : zoned_time{__traits::locate_zone(__name), __tp} {}
+
+  _LIBCPP_HIDE_FROM_ABI zoned_time(_TimeZonePtr __zone, const 
local_time<_Duration>& __tp, choose __c)
+requires(is_convertible_v<
+decltype(std::declval<_TimeZonePtr&>() -> 
to_sys(local_time<_Duration>{}, choose::earliest)),
+sys_time>)
+  : __zone_{std::move(__zone)}, __tp_{__zone_->to_sys(__tp, __c)} {}
+
+  _LIBCPP_HIDE_FROM_ABI zoned_time(string_view __name, const 
local_time<_Duration>& __tp, choose __c)
+requires(requires {
+  _TimeZonePtr{__traits::locate_zone(string_view{})};
+} && is_convertible_v() -> 
to_sys(local_time<_Duration>{}, choose::earliest)),
+  sys_time>)
+  : zoned_time{__traits::locate_zone(__name), __tp, __c} {}
+
+  template 
+  _LIBCPP_HIDE_FROM_ABI zoned_time(_TimeZonePtr __zone, const 
zoned_time<_Duration2, _TimeZonePtr2>& __zt)
+requires is_convertible_v, sys_time<_Duration>>
+  : __zone_{std::move(__zone)}, __tp_{__zt.get_sys_time()} {}
+
+  // per wording choose has no effect
+  template 
+  _LIBCPP_HIDE_FROM_ABI zoned_time(_TimeZonePtr __zone, const 
zoned_time<_Duration2, _TimeZonePtr2>& __zt, choose)
+requires is_convertible_v, sys_time<_Duration>>
+  : __zone_{std::move(__zone)}, __tp_{__zt.get_sys_time()} {}
+
+  template 
+  _LIBCPP_HIDE_FROM_ABI zoned_time(string_view __name, const 
zoned_time<_Duration2, _TimeZonePtr2>& __zt)
+requires(requires {

ldionne wrote:

Constraint.

https://github.com/llvm/llvm-project/pull/95010
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Finishes zoned_time constructors. (PR #95010)

2024-06-18 Thread Louis Dionne via llvm-branch-commits


@@ -76,12 +81,71 @@ class zoned_time {
 
   _LIBCPP_HIDE_FROM_ABI explicit zoned_time(string_view __name)
 requires(requires { __traits::locate_zone(string_view{}); } &&
- // constructible_from
- // would create a dependency on itself. Instead depend on the fact
- // a constructor taking a _TimeZonePtr exists.
  constructible_from<_TimeZonePtr, 
decltype(__traits::locate_zone(string_view{}))>)
   : __zone_{__traits::locate_zone(__name)}, __tp_{} {}
 
+  template 
+  _LIBCPP_HIDE_FROM_ABI zoned_time(const zoned_time<_Duration2, _TimeZonePtr>& 
__zt)
+requires is_convertible_v, sys_time<_Duration>>
+  : __zone_{__zt.get_time_zone()}, __tp_{__zt.get_sys_time()} {}
+
+  _LIBCPP_HIDE_FROM_ABI zoned_time(_TimeZonePtr __zone, const 
sys_time<_Duration>& __tp)
+  : __zone_{std::move(__zone)}, __tp_{__tp} {}
+
+  _LIBCPP_HIDE_FROM_ABI zoned_time(string_view __name, const 
sys_time<_Duration>& __tp)
+requires requires { _TimeZonePtr{__traits::locate_zone(string_view{})}; }
+  : zoned_time{__traits::locate_zone(__name), __tp} {}
+
+  _LIBCPP_HIDE_FROM_ABI zoned_time(_TimeZonePtr __zone, const 
local_time<_Duration>& __tp)
+requires(is_convertible_v() -> 
to_sys(local_time<_Duration>{})),
+  sys_time>)
+  : __zone_{std::move(__zone)}, __tp_{__zone_->to_sys(__tp)} {}
+
+  _LIBCPP_HIDE_FROM_ABI zoned_time(string_view __name, const 
local_time<_Duration>& __tp)
+requires(requires {
+  _TimeZonePtr{__traits::locate_zone(string_view{})};
+} && is_convertible_v() -> 
to_sys(local_time<_Duration>{})),
+  sys_time>)
+  : zoned_time{__traits::locate_zone(__name), __tp} {}
+
+  _LIBCPP_HIDE_FROM_ABI zoned_time(_TimeZonePtr __zone, const 
local_time<_Duration>& __tp, choose __c)
+requires(is_convertible_v<
+decltype(std::declval<_TimeZonePtr&>() -> 
to_sys(local_time<_Duration>{}, choose::earliest)),
+sys_time>)
+  : __zone_{std::move(__zone)}, __tp_{__zone_->to_sys(__tp, __c)} {}
+
+  _LIBCPP_HIDE_FROM_ABI zoned_time(string_view __name, const 
local_time<_Duration>& __tp, choose __c)
+requires(requires {
+  _TimeZonePtr{__traits::locate_zone(string_view{})};
+} && is_convertible_v() -> 
to_sys(local_time<_Duration>{}, choose::earliest)),
+  sys_time>)
+  : zoned_time{__traits::locate_zone(__name), __tp, __c} {}
+
+  template 
+  _LIBCPP_HIDE_FROM_ABI zoned_time(_TimeZonePtr __zone, const 
zoned_time<_Duration2, _TimeZonePtr2>& __zt)
+requires is_convertible_v, sys_time<_Duration>>
+  : __zone_{std::move(__zone)}, __tp_{__zt.get_sys_time()} {}
+
+  // per wording choose has no effect
+  template 
+  _LIBCPP_HIDE_FROM_ABI zoned_time(_TimeZonePtr __zone, const 
zoned_time<_Duration2, _TimeZonePtr2>& __zt, choose)
+requires is_convertible_v, sys_time<_Duration>>
+  : __zone_{std::move(__zone)}, __tp_{__zt.get_sys_time()} {}
+
+  template 
+  _LIBCPP_HIDE_FROM_ABI zoned_time(string_view __name, const 
zoned_time<_Duration2, _TimeZonePtr2>& __zt)
+requires(requires {
+  _TimeZonePtr{__traits::locate_zone(string_view{})};
+} && is_convertible_v, sys_time<_Duration>>)
+  : zoned_time{__traits::locate_zone(__name), __zt} {}
+
+  template 
+  _LIBCPP_HIDE_FROM_ABI zoned_time(string_view __name, const 
zoned_time<_Duration2, _TimeZonePtr2>& __zt, choose __c)
+requires(requires {

ldionne wrote:

Constraint.

https://github.com/llvm/llvm-project/pull/95010
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Finishes zoned_time constructors. (PR #95010)

2024-06-18 Thread Louis Dionne via llvm-branch-commits


@@ -0,0 +1,105 @@
+//===--===/
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+// UNSUPPORTED: no-filesystem, no-localization, no-tzdb
+
+// XFAIL: libcpp-has-no-experimental-tzdb
+// XFAIL: availability-tzdb-missing
+
+// 
+
+// template
+// class zoned_time;
+//
+// zoned_time(string_view name, const local_time& st, choose c);
+
+#include 
+#include 
+#include 
+
+#include "test_offset_time_zone.h"
+
+int main(int, char**) {
+  // Tests unique conversions. To make sure the test is does not depend on 
changes
+  // in the database it uses a time zone with a fixed offset.
+  {
+std::chrono::zoned_time zt{
+"Etc/GMT+1", std::chrono::local_seconds{std::chrono::seconds{0}}, 
std::chrono::choose::earliest};
+
+assert(zt.get_time_zone() == std::chrono::locate_zone("Etc/GMT+1"));
+assert(zt.get_sys_time() == 
std::chrono::sys_seconds{std::chrono::hours{1}});
+  }
+
+  // Tests ambiguous conversions.
+  {
+// Z Europe/Berlin 0:53:28 - LMT 1893 Ap
+// ...
+// 1 DE CE%sT 1980
+// 1 E CE%sT
+//
+// ...
+// R E 1981 ma - Mar lastSu 1u 1 S
+// R E 1996 ma - O lastSu 1u 0 -
+
+using namespace std::literals::chrono_literals;
+{
+  std::chrono::zoned_time zt{

ldionne wrote:

You could consider using `namespace cr = std::chrono` to shorten these tests.

https://github.com/llvm/llvm-project/pull/95010
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [clang] [libcxx] [clang] Finish implementation of P0522 (PR #96023)

2024-06-19 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne approved this pull request.

libcxx/ nit LGTM.

https://github.com/llvm/llvm-project/pull/96023
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] Add release note for #95264 (PR #96116)

2024-06-19 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne created 
https://github.com/llvm/llvm-project/pull/96116

None

>From 4044e7c930381e5e070c7131c5b14a3dfd373259 Mon Sep 17 00:00:00 2001
From: Louis Dionne 
Date: Wed, 19 Jun 2024 16:50:07 -0400
Subject: [PATCH] [libc++] Add release note for #95264

---
 libcxx/docs/ReleaseNotes/18.rst | 9 +
 1 file changed, 9 insertions(+)

diff --git a/libcxx/docs/ReleaseNotes/18.rst b/libcxx/docs/ReleaseNotes/18.rst
index 7ea13e6943dd4..3e19e7c33f6af 100644
--- a/libcxx/docs/ReleaseNotes/18.rst
+++ b/libcxx/docs/ReleaseNotes/18.rst
@@ -328,6 +328,15 @@ ABI Affecting Changes
   done to fix `#70494 `_ 
and the vendor communication is handled
   in `#70820 `_.
 
+- LLVM 18.1.8 Fixed an issue that caused ``std::string`` to pass an incorrect 
size to ``allocator_traits::deallocate``
+  when deallocating memory. The impact is different depending on a few factors:
+  - Users who don't use a custom allocator in ``std::string`` and don't enable 
sized deallocation (which is
+off by default in Clang 18) will not be affected. This is expected to be 
the vast majority of users.
+  - Users who don't use a custom allocator in ``std::string`` but are enabling 
sized deallocation (e.g. with
+``-fsized-deallocation``) will notice that ``operator delete(void*, 
size_t)`` is now being passed the correct
+size. This likely has no impact if they were not customizing ``operator 
delete``.
+  - Users who use a custom allocator in ``std::string`` will notice that they 
now get passed the correct allocation
+size upon deallocation.
 
 Build System Changes
 

___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] Add release note for #95264 (PR #96116)

2024-06-19 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne milestoned 
https://github.com/llvm/llvm-project/pull/96116
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++] Implement std::move_only_function (P0288R9) (PR #94670)

2024-06-20 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne requested changes to this pull request.

I am not certain how to best make a decision about the bit-stealing mechanism 
yet, but I do have a few comments.

https://github.com/llvm/llvm-project/pull/94670
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++] Implement std::move_only_function (P0288R9) (PR #94670)

2024-06-20 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne edited 
https://github.com/llvm/llvm-project/pull/94670
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++] Implement std::move_only_function (P0288R9) (PR #94670)

2024-06-20 Thread Louis Dionne via llvm-branch-commits


@@ -0,0 +1,93 @@
+//===--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef _LIBCPP___FUNCTIONAL_MOVE_ONLY_FUNCTION_H
+#define _LIBCPP___FUNCTIONAL_MOVE_ONLY_FUNCTION_H
+
+#include <__config>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#  pragma GCC system_header
+#endif
+
+#if _LIBCPP_STD_VER >= 23
+
+// move_only_function design:
+//
+// move_only_function has a small buffer with a size of `3 * sizeof(void*)` 
bytes. This buffer can only be used when the
+// object that should be stored is trivially relocatable (currently only when 
it is trivially move constructible and
+// trivially destructible). There is also a bool in the lower bits of the vptr 
stored which is set when the contained
+// object is not trivially destructible.
+//
+// trivially relocatable: It would also be possible to store 
nothrow_move_constructible types, but that would mean
+// that move_only_function itself would not be trivially relocatable anymore. 
The decision to keep move_only_function
+// trivially relocatable was made because we expect move_only_function to be 
mostly used to store a functor. To only
+// forward functors there is std::function_ref (not voted in yet, expected in 
C++26).
+//
+// buffer size: We did a survey of six implementations from various vendors. 
Three of them had a buffer size of 24 bytes
+// on 64 bit systems. This also allows storing a std::string or std::vector 
inside the small buffer (once the compiler
+// has full support of trivially_relocatable annotations).
+//
+// trivially-destructible bit: This allows us to keep the overall binary size 
smaller because we don't have to store
+// a pointer to a noop function inside the vtable. It also avoids loading the 
vtable during destruction, potentially
+// resulting in fewer cache misses. The downside is that calling the function 
now also requires setting the lower bits
+// of the pointer to zero, but this is a very fast operation on modern CPUs.

ldionne wrote:

I would like us to document the design constraints around avoiding 
double-wrapping when constructing a `std::move_ony_function` from a 
`std::copyable_function` that we discussed just now. Gist of it:

This requires having compatible vtables in both implementations, where 
compatible means that the layout is the same but also that the implementation 
of the vtable functions can be swapped for one another. Basically, this means 
we'll need to use small buffer sizes where `sizeof(move_only_function) >= 
sizeof(copyable_function)`, and have the same criteria for when we put it in 
the small buffer (trivially-relocatable?).

IMO these requirements make it important to reuse the same machinery for 
implementing both classes. I am not certain in what form we want to do that 
yet, but I am thinking that there should be a single place where we encode 
these decisions (the small buffer size, the vtable layout, the condition for 
being in the SBO, etc). I don't think those belong in the `__small_buffer` 
class itself since that can be reused for stuff that isn't `move_only_function` 
or `copyable_function`, but it should be somewhere. Do you have thoughts on 
this?

https://github.com/llvm/llvm-project/pull/94670
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++] Implement std::move_only_function (P0288R9) (PR #94670)

2024-06-20 Thread Louis Dionne via llvm-branch-commits


@@ -0,0 +1,93 @@
+//===--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef _LIBCPP___FUNCTIONAL_MOVE_ONLY_FUNCTION_H
+#define _LIBCPP___FUNCTIONAL_MOVE_ONLY_FUNCTION_H
+
+#include <__config>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#  pragma GCC system_header
+#endif
+
+#if _LIBCPP_STD_VER >= 23
+
+// move_only_function design:

ldionne wrote:

We should keep this type experimental until we have an implementation of 
`copyable_function` since those will need to interact in non-trivial ways.

https://github.com/llvm/llvm-project/pull/94670
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++] Implement std::move_only_function (P0288R9) (PR #94670)

2024-06-20 Thread Louis Dionne via llvm-branch-commits


@@ -0,0 +1,93 @@
+//===--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef _LIBCPP___FUNCTIONAL_MOVE_ONLY_FUNCTION_H
+#define _LIBCPP___FUNCTIONAL_MOVE_ONLY_FUNCTION_H
+
+#include <__config>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#  pragma GCC system_header
+#endif
+
+#if _LIBCPP_STD_VER >= 23
+
+// move_only_function design:
+//
+// move_only_function has a small buffer with a size of `3 * sizeof(void*)` 
bytes. This buffer can only be used when the
+// object that should be stored is trivially relocatable (currently only when 
it is trivially move constructible and
+// trivially destructible). There is also a bool in the lower bits of the vptr 
stored which is set when the contained
+// object is not trivially destructible.
+//
+// trivially relocatable: It would also be possible to store 
nothrow_move_constructible types, but that would mean
+// that move_only_function itself would not be trivially relocatable anymore. 
The decision to keep move_only_function
+// trivially relocatable was made because we expect move_only_function to be 
mostly used to store a functor. To only
+// forward functors there is std::function_ref (not voted in yet, expected in 
C++26).

ldionne wrote:

```suggestion
// forward functors there is C++26's std::function_ref.
```

https://github.com/llvm/llvm-project/pull/94670
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] Add release note for #95264 (PR #96116)

2024-06-20 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne updated 
https://github.com/llvm/llvm-project/pull/96116

>From 4044e7c930381e5e070c7131c5b14a3dfd373259 Mon Sep 17 00:00:00 2001
From: Louis Dionne 
Date: Wed, 19 Jun 2024 16:50:07 -0400
Subject: [PATCH 1/2] [libc++] Add release note for #95264

---
 libcxx/docs/ReleaseNotes/18.rst | 9 +
 1 file changed, 9 insertions(+)

diff --git a/libcxx/docs/ReleaseNotes/18.rst b/libcxx/docs/ReleaseNotes/18.rst
index 7ea13e6943dd4..3e19e7c33f6af 100644
--- a/libcxx/docs/ReleaseNotes/18.rst
+++ b/libcxx/docs/ReleaseNotes/18.rst
@@ -328,6 +328,15 @@ ABI Affecting Changes
   done to fix `#70494 `_ 
and the vendor communication is handled
   in `#70820 `_.
 
+- LLVM 18.1.8 Fixed an issue that caused ``std::string`` to pass an incorrect 
size to ``allocator_traits::deallocate``
+  when deallocating memory. The impact is different depending on a few factors:
+  - Users who don't use a custom allocator in ``std::string`` and don't enable 
sized deallocation (which is
+off by default in Clang 18) will not be affected. This is expected to be 
the vast majority of users.
+  - Users who don't use a custom allocator in ``std::string`` but are enabling 
sized deallocation (e.g. with
+``-fsized-deallocation``) will notice that ``operator delete(void*, 
size_t)`` is now being passed the correct
+size. This likely has no impact if they were not customizing ``operator 
delete``.
+  - Users who use a custom allocator in ``std::string`` will notice that they 
now get passed the correct allocation
+size upon deallocation.
 
 Build System Changes
 

>From 99a23b574283752ffdc69934c32f9f825ddf10ed Mon Sep 17 00:00:00 2001
From: Louis Dionne 
Date: Thu, 20 Jun 2024 13:46:04 -0400
Subject: [PATCH 2/2] Try to fix RST syntax error

---
 libcxx/docs/ReleaseNotes/18.rst | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/libcxx/docs/ReleaseNotes/18.rst b/libcxx/docs/ReleaseNotes/18.rst
index 3e19e7c33f6af..f7c269acda8b2 100644
--- a/libcxx/docs/ReleaseNotes/18.rst
+++ b/libcxx/docs/ReleaseNotes/18.rst
@@ -330,13 +330,14 @@ ABI Affecting Changes
 
 - LLVM 18.1.8 Fixed an issue that caused ``std::string`` to pass an incorrect 
size to ``allocator_traits::deallocate``
   when deallocating memory. The impact is different depending on a few factors:
-  - Users who don't use a custom allocator in ``std::string`` and don't enable 
sized deallocation (which is
-off by default in Clang 18) will not be affected. This is expected to be 
the vast majority of users.
-  - Users who don't use a custom allocator in ``std::string`` but are enabling 
sized deallocation (e.g. with
-``-fsized-deallocation``) will notice that ``operator delete(void*, 
size_t)`` is now being passed the correct
-size. This likely has no impact if they were not customizing ``operator 
delete``.
-  - Users who use a custom allocator in ``std::string`` will notice that they 
now get passed the correct allocation
-size upon deallocation.
+
+- Users who don't use a custom allocator in ``std::string`` and don't 
enable sized deallocation (which is
+  off by default in Clang 18) will not be affected. This is expected to be 
the vast majority of users.
+- Users who don't use a custom allocator in ``std::string`` but are 
enabling sized deallocation (e.g. with
+  ``-fsized-deallocation``) will notice that ``operator delete(void*, 
size_t)`` is now being passed the correct
+  size. This likely has no impact if they were not customizing ``operator 
delete``.
+- Users who use a custom allocator in ``std::string`` will notice that 
they now get passed the correct allocation
+  size upon deallocation.
 
 Build System Changes
 

___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] Add release note for #95264 (PR #96116)

2024-06-24 Thread Louis Dionne via llvm-branch-commits

ldionne wrote:

Gentle ping @tstellar, do we want to merge this?

The CI issues are unrelated.

https://github.com/llvm/llvm-project/pull/96116
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] Add release note for #95264 (PR #96116)

2024-06-24 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne closed 
https://github.com/llvm/llvm-project/pull/96116
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] Add release note for #95264 (PR #96116)

2024-06-24 Thread Louis Dionne via llvm-branch-commits

ldionne wrote:

> We don't have a great way to add release notes after the final release. I 
> added this to the release announcement do you think that is enough?

Yes I think that is fine.

https://github.com/llvm/llvm-project/pull/96116
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [clang] [libcxx] [clang] Finish implementation of P0522 (PR #96023)

2024-06-25 Thread Louis Dionne via llvm-branch-commits


@@ -18,5 +18,9 @@
 #include 
 #include 
 
-// expected-error@+1 {{template template argument has different template 
parameters than its corresponding template template parameter}}
-static_assert(!std::__is_specialization_v, 
std::array>);
+#if defined(__clang__) && __clang_major__ >= 19
+// expected-error@array:* {{could not match _Size against 
'type-parameter-0-0'}}
+#else
+// expected-error@#SA {{template template argument has different template 
parameters than its corresponding template template parameter}}
+#endif

ldionne wrote:

Yeah, I really don't think the exact error message is important to test.

https://github.com/llvm/llvm-project/pull/96023
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [runtimes] Allow building against an installed LLVM tree (PR #86209)

2024-06-25 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne approved this pull request.

LGTM but I think @petrhosek should also have a look.

https://github.com/llvm/llvm-project/pull/86209
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Finishes zoned_time constructors. (PR #95010)

2024-07-09 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne milestoned 
https://github.com/llvm/llvm-project/pull/95010
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Finishes zoned_time member functions. (PR #95026)

2024-07-09 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne edited 
https://github.com/llvm/llvm-project/pull/95026
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Finishes zoned_time member functions. (PR #95026)

2024-07-09 Thread Louis Dionne via llvm-branch-commits


@@ -0,0 +1,136 @@
+//===--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+// UNSUPPORTED: no-filesystem, no-localization, no-tzdb
+
+// XFAIL: libcpp-has-no-experimental-tzdb
+// XFAIL: availability-tzdb-missing
+
+// 
+
+// template
+// class zoned_time;
+//
+// zoned_time& operator=(const sys_time& st);
+
+#include 
+#include 
+#include 
+#include 
+
+namespace cr = std::chrono;
+
+int main(int, char**) {
+  {
+using duration   = cr::nanoseconds;
+using time_point = cr::sys_time;
+using zoned_time = cr::zoned_time;
+zoned_time zt{time_point{duration{42}}};
+
+assert(zt.get_time_zone() == cr::locate_zone("UTC"));
+assert(zt.get_sys_time() == time_point{duration{42}});
+
+[[maybe_unused]] std::same_as decltype(auto) _ = zt = 
time_point{duration{99}};

ldionne wrote:

Same comment here!

https://github.com/llvm/llvm-project/pull/95026
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Finishes zoned_time member functions. (PR #95026)

2024-07-09 Thread Louis Dionne via llvm-branch-commits


@@ -0,0 +1,135 @@
+//===--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+// UNSUPPORTED: no-filesystem, no-localization, no-tzdb
+
+// XFAIL: libcpp-has-no-experimental-tzdb
+// XFAIL: availability-tzdb-missing
+
+// 
+
+// template
+// class zoned_time;
+//
+// explicit operator local_time() const;
+
+#include 
+#include 
+
+#include "../test_offset_time_zone.h"
+
+namespace cr = std::chrono;
+
+static void test_const_member() {
+  {
+using duration = cr::nanoseconds;
+using time_point   = cr::sys_time;
+using local_time_point = cr::local_time;
+static_assert(!std::is_convertible_v>);
+cr::zoned_time zt{"Etc/GMT+1", time_point{duration{42}}};

ldionne wrote:

Same comment.

https://github.com/llvm/llvm-project/pull/95026
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Finishes zoned_time member functions. (PR #95026)

2024-07-09 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne approved this pull request.

LGTM with a few suggestions!

https://github.com/llvm/llvm-project/pull/95026
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Finishes zoned_time member functions. (PR #95026)

2024-07-09 Thread Louis Dionne via llvm-branch-commits


@@ -0,0 +1,243 @@
+//===--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+// UNSUPPORTED: no-filesystem, no-localization, no-tzdb
+
+// XFAIL: libcpp-has-no-experimental-tzdb
+// XFAIL: availability-tzdb-missing
+
+// 
+
+// template
+// class zoned_time;
+//
+// zoned_time& operator=(const local_time& st);
+
+// TODO TZDB Investigate the issues in this test, this seems like
+// a design issue of the class.
+//
+// [time.zone.zonedtime.members]/3
+//   Effects: After assignment, get_local_time() == lt.
+//   This assignment has no effect on the return value of get_time_zone().
+//
+// The test cases describe the issues.
+
+#include 
+#include 
+#include 
+#include 
+
+#include "test_macros.h"
+
+namespace cr = std::chrono;
+
+// Tests unique conversions. To make sure the test is does not depend on 
changes
+// in the database it uses a time zone with a fixed offset.
+static void test_unique() {
+  // common_type_t -> duration
+  {
+using duration = cr::nanoseconds;
+using sys_time_point   = cr::sys_time;
+using local_time_point = cr::local_time;
+using zoned_time   = cr::zoned_time;
+zoned_time zt{"Etc/GMT+1", sys_time_point{duration{42}}};
+
+assert(zt.get_time_zone() == cr::locate_zone("Etc/GMT+1"));
+assert(zt.get_sys_time() == sys_time_point{duration{42}});
+assert(zt.get_local_time() == local_time_point{duration{42} - 
cr::hours{1}});
+
+[[maybe_unused]] std::same_as decltype(auto) _ = zt = 
local_time_point{duration{99}};

ldionne wrote:

```suggestion
std::same_as decltype(auto) result = zt = 
local_time_point{duration{99}};
assert(&result == &zt);
```

Kinda pedantic but necessary!

https://github.com/llvm/llvm-project/pull/95026
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Finishes zoned_time member functions. (PR #95026)

2024-07-09 Thread Louis Dionne via llvm-branch-commits


@@ -0,0 +1,133 @@
+//===--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+// UNSUPPORTED: no-filesystem, no-localization, no-tzdb
+
+// XFAIL: libcpp-has-no-experimental-tzdb
+// XFAIL: availability-tzdb-missing
+
+// 
+
+// template
+// class zoned_time;
+//
+// local_time get_local_time() const;
+
+#include 
+#include 
+
+#include "../test_offset_time_zone.h"
+
+namespace cr = std::chrono;
+
+static void test_const_member() {
+  {
+using duration = cr::nanoseconds;
+using time_point   = cr::sys_time;
+using local_time_point = cr::local_time;
+cr::zoned_time zt{"Etc/GMT+1", time_point{duration{42}}};

ldionne wrote:

This test seems out of place, since the function name suggests that we're only 
testing const-qualification in it.

https://github.com/llvm/llvm-project/pull/95026
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Finishes zoned_time member functions. (PR #95026)

2024-07-09 Thread Louis Dionne via llvm-branch-commits


@@ -0,0 +1,243 @@
+//===--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+// UNSUPPORTED: no-filesystem, no-localization, no-tzdb
+
+// XFAIL: libcpp-has-no-experimental-tzdb
+// XFAIL: availability-tzdb-missing
+
+// 
+
+// template
+// class zoned_time;
+//
+// zoned_time& operator=(const local_time& st);
+
+// TODO TZDB Investigate the issues in this test, this seems like
+// a design issue of the class.
+//
+// [time.zone.zonedtime.members]/3
+//   Effects: After assignment, get_local_time() == lt.
+//   This assignment has no effect on the return value of get_time_zone().
+//
+// The test cases describe the issues.
+
+#include 
+#include 
+#include 
+#include 
+
+#include "test_macros.h"
+
+namespace cr = std::chrono;
+
+// Tests unique conversions. To make sure the test is does not depend on 
changes
+// in the database it uses a time zone with a fixed offset.
+static void test_unique() {
+  // common_type_t -> duration
+  {
+using duration = cr::nanoseconds;
+using sys_time_point   = cr::sys_time;
+using local_time_point = cr::local_time;
+using zoned_time   = cr::zoned_time;
+zoned_time zt{"Etc/GMT+1", sys_time_point{duration{42}}};
+
+assert(zt.get_time_zone() == cr::locate_zone("Etc/GMT+1"));
+assert(zt.get_sys_time() == sys_time_point{duration{42}});
+assert(zt.get_local_time() == local_time_point{duration{42} - 
cr::hours{1}});
+
+[[maybe_unused]] std::same_as decltype(auto) _ = zt = 
local_time_point{duration{99}};
+assert(zt.get_time_zone() == cr::locate_zone("Etc/GMT+1"));
+assert(zt.get_sys_time() == sys_time_point{duration{99} + cr::hours{1}});
+assert(zt.get_local_time() == local_time_point{duration{99}});
+  }
+  {
+using duration = cr::microseconds;
+using sys_time_point   = cr::sys_time;
+using local_time_point = cr::local_time;
+using zoned_time   = cr::zoned_time;
+zoned_time zt{"Etc/GMT+1", sys_time_point{duration{42}}};
+
+assert(zt.get_time_zone() == cr::locate_zone("Etc/GMT+1"));
+assert(zt.get_sys_time() == sys_time_point{duration{42}});
+assert(zt.get_local_time() == local_time_point{duration{42} - 
cr::hours{1}});
+
+[[maybe_unused]] std::same_as decltype(auto) _ = zt = 
local_time_point{duration{99}};
+assert(zt.get_time_zone() == cr::locate_zone("Etc/GMT+1"));
+assert(zt.get_sys_time() == sys_time_point{duration{99} + cr::hours{1}});
+assert(zt.get_local_time() == local_time_point{duration{99}});
+  }
+  {
+using duration = cr::milliseconds;
+using sys_time_point   = cr::sys_time;
+using local_time_point = cr::local_time;
+using zoned_time   = cr::zoned_time;
+zoned_time zt{"Etc/GMT+1", sys_time_point{duration{42}}};
+
+assert(zt.get_time_zone() == cr::locate_zone("Etc/GMT+1"));
+assert(zt.get_sys_time() == sys_time_point{duration{42}});
+assert(zt.get_local_time() == local_time_point{duration{42} - 
cr::hours{1}});
+
+[[maybe_unused]] std::same_as decltype(auto) _ = zt = 
local_time_point{duration{99}};
+assert(zt.get_time_zone() == cr::locate_zone("Etc/GMT+1"));
+assert(zt.get_sys_time() == sys_time_point{duration{99} + cr::hours{1}});
+assert(zt.get_local_time() == local_time_point{duration{99}});
+  }
+  // common_type_t -> seconds
+  {
+using duration = cr::seconds;
+using sys_time_point   = cr::sys_time;
+using local_time_point = cr::local_time;
+using zoned_time   = cr::zoned_time;
+zoned_time zt{"Etc/GMT+1", sys_time_point{duration{42}}};
+
+assert(zt.get_time_zone() == cr::locate_zone("Etc/GMT+1"));
+assert(zt.get_sys_time() == sys_time_point{duration{42}});
+assert(zt.get_local_time() == local_time_point{duration{42} - 
cr::hours{1}});
+
+[[maybe_unused]] std::same_as decltype(auto) _ = zt = 
local_time_point{duration{99}};
+assert(zt.get_time_zone() == cr::locate_zone("Etc/GMT+1"));
+assert(zt.get_sys_time() == sys_time_point{duration{99} + cr::hours{1}});
+assert(zt.get_local_time() == local_time_point{duration{99}});
+  }
+  // common_type_t -> seconds
+  {
+using duration = cr::days;
+using sys_time_point   = cr::sys_time;
+using local_time_point = cr::local_time;
+using zoned_time   = cr::zoned_time;
+zoned_time zt{"Etc/GMT+1", sys_time_point{duration{42}}};
+
+assert(zt.get_time_zone() == cr::locate_zone("Etc/GMT+1"));
+assert(zt.get_sys_time() == cr::sys_seconds{duration{42}});
+assert(zt.get_local_time() == cr::local_seconds{duration{42} - 
cr::hours{1}});
+
+[[maybe_unused]] std::same_as decltyp

[llvm-branch-commits] [libcxx] [libc++][TZDB] Finishes zoned_time member functions. (PR #95026)

2024-07-09 Thread Louis Dionne via llvm-branch-commits


@@ -0,0 +1,135 @@
+//===--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+// UNSUPPORTED: no-filesystem, no-localization, no-tzdb
+
+// XFAIL: libcpp-has-no-experimental-tzdb
+// XFAIL: availability-tzdb-missing
+
+// 
+
+// template
+// class zoned_time;
+//
+// explicit operator local_time() const;
+
+#include 
+#include 
+
+#include "../test_offset_time_zone.h"
+
+namespace cr = std::chrono;
+
+static void test_const_member() {
+  {
+using duration = cr::nanoseconds;
+using time_point   = cr::sys_time;
+using local_time_point = cr::local_time;
+static_assert(!std::is_convertible_v>);
+cr::zoned_time zt{"Etc/GMT+1", time_point{duration{42}}};
+
+std::same_as decltype(auto) time = 
static_cast(zt);
+assert(time == local_time_point{duration{42} - cr::hours{1}});
+  }
+  {
+using duration = cr::nanoseconds;
+using time_point   = cr::sys_time;
+using local_time_point = cr::local_time;
+static_assert(!std::is_convertible_v>);
+const cr::zoned_time zt{"Etc/GMT+1", time_point{duration{42}}};
+
+std::same_as decltype(auto) time = 
static_cast(zt);
+assert(time == local_time_point{duration{42} - cr::hours{1}});
+  }
+}
+
+static void test_duration_conversion() {
+  // common_type_t -> duration
+  {
+using duration = cr::nanoseconds;
+using time_point   = cr::sys_time;
+using local_time_point = cr::local_time;
+cr::zoned_time zt{"Etc/GMT+1", time_point{duration{42}}};
+
+std::same_as decltype(auto) time = 
static_cast(zt);
+assert(time == local_time_point{duration{42} - cr::hours{1}});
+  }
+  {
+using duration = cr::microseconds;
+using time_point   = cr::sys_time;
+using local_time_point = cr::local_time;
+cr::zoned_time zt{"Etc/GMT+1", time_point{duration{42}}};
+
+std::same_as decltype(auto) time = 
static_cast(zt);
+assert(time == local_time_point{duration{42} - cr::hours{1}});
+  }
+  {
+using duration = cr::milliseconds;
+using time_point   = cr::sys_time;
+using local_time_point = cr::local_time;
+cr::zoned_time zt{"Etc/GMT+1", time_point{duration{42}}};
+
+std::same_as decltype(auto) time = 
static_cast(zt);
+assert(time == local_time_point{duration{42} - cr::hours{1}});
+  }
+  // common_type_t -> seconds
+  {
+using duration = cr::seconds;
+using time_point   = cr::sys_time;
+using local_time_point = cr::local_time;
+cr::zoned_time zt{"Etc/GMT+1", time_point{duration{42}}};
+
+std::same_as decltype(auto) time = 
static_cast(zt);
+assert(time == local_time_point{duration{42} - cr::hours{1}});
+  }
+  // common_type_t -> seconds
+  {
+using duration = cr::days;
+using time_point   = cr::sys_time;
+using local_time_point = cr::local_time;
+cr::zoned_time zt{"Etc/GMT+1", time_point{duration{42}}};
+
+std::same_as decltype(auto) time = 
static_cast(zt);
+assert(time == local_time_point{duration{42} - cr::hours{1}});
+  }
+  {
+using duration = cr::weeks;
+using time_point   = cr::sys_time;
+using local_time_point = cr::local_time;
+cr::zoned_time zt{"Etc/GMT+1", time_point{duration{42}}};
+
+std::same_as decltype(auto) time = 
static_cast(zt);
+assert(time == local_time_point{duration{42} - cr::hours{1}});
+  }
+  {
+using duration = cr::months;
+using time_point   = cr::sys_time;
+using local_time_point = cr::local_time;
+cr::zoned_time zt{"Etc/GMT+1", time_point{duration{42}}};
+
+std::same_as decltype(auto) time = 
static_cast(zt);
+assert(time == local_time_point{duration{42} - cr::hours{1}});
+  }
+  {
+using duration = cr::years;
+using time_point   = cr::sys_time;
+using local_time_point = cr::local_time;
+cr::zoned_time zt{"Etc/GMT+1", time_point{duration{42}}};

ldionne wrote:

Note that you could make all of those `zt`'s `const` since the method is 
`const`.

https://github.com/llvm/llvm-project/pull/95026
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Finishes zoned_time member functions. (PR #95026)

2024-07-09 Thread Louis Dionne via llvm-branch-commits


@@ -147,8 +148,29 @@ class zoned_time {
 } && is_convertible_v, sys_time<_Duration>>)
   : zoned_time{__traits::locate_zone(__name), __zt, __c} {}
 
+  _LIBCPP_HIDE_FROM_ABI zoned_time& operator=(const sys_time<_Duration>& __tp) 
{
+__tp_ = __tp;
+return *this;
+  }
+
+  _LIBCPP_HIDE_FROM_ABI zoned_time& operator=(const local_time<_Duration>& 
__tp) {
+// TODO TZDB This seems wrong.
+// Assigning a non-existant or abiguous time will throw and not satisfy

ldionne wrote:

```suggestion
// Assigning a non-existent or ambiguous time will throw and not satisfy
```

Can you file a LWG issue about this?

https://github.com/llvm/llvm-project/pull/95026
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Adds zoned_time deduction guides. (PR #95139)

2024-07-09 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne milestoned 
https://github.com/llvm/llvm-project/pull/95139
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Adds zoned_time deduction guides. (PR #95139)

2024-07-09 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne approved this pull request.


https://github.com/llvm/llvm-project/pull/95139
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Implements zoned_time's operator==. (PR #95140)

2024-07-09 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne milestoned 
https://github.com/llvm/llvm-project/pull/95140
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Implements zoned_time's operator==. (PR #95140)

2024-07-09 Thread Louis Dionne via llvm-branch-commits


@@ -0,0 +1,71 @@
+//===--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+
+// REQUIRES: has-unix-headers
+// REQUIRES: libcpp-hardening-mode={{extensive|debug}}
+// XFAIL: libcpp-hardening-mode=debug && availability-verbose_abort-missing

ldionne wrote:

This looks like a leftover?

https://github.com/llvm/llvm-project/pull/95140
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Implements zoned_time's operator==. (PR #95140)

2024-07-09 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne edited 
https://github.com/llvm/llvm-project/pull/95140
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Implements zoned_time's operator==. (PR #95140)

2024-07-09 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne approved this pull request.


https://github.com/llvm/llvm-project/pull/95140
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Implements zoned_seconds typedef. (PR #95141)

2024-07-09 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne milestoned 
https://github.com/llvm/llvm-project/pull/95141
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][TZDB] Implements zoned_seconds typedef. (PR #95141)

2024-07-09 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne approved this pull request.


https://github.com/llvm/llvm-project/pull/95141
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][spaceship] Marks P1614 as complete. (PR #99375)

2024-07-18 Thread Louis Dionne via llvm-branch-commits


@@ -1,5 +1,5 @@
 "Number","Name","Status","First released version"

ldionne wrote:

Could we actually get rid of this file? I don't think it serves much of a 
purpose anymore. We should make sure that all the information in that file is 
tracked in our other `Cxx20Papers.rst` & friends and then get rid of it.

https://github.com/llvm/llvm-project/pull/99375
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][spaceship] Marks P1614 as complete. (PR #99375)

2024-07-23 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne edited 
https://github.com/llvm/llvm-project/pull/99375
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][spaceship] Marks P1614 as complete. (PR #99375)

2024-07-23 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne approved this pull request.

LGTM. Let's cherry-pick.

https://github.com/llvm/llvm-project/pull/99375
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][doc] Update the release notes for LLVM 19. (PR #100167)

2024-07-23 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne approved this pull request.


https://github.com/llvm/llvm-project/pull/100167
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][spaceship] Implements X::iterator container requirements. (#… (PR #100440)

2024-07-24 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne edited 
https://github.com/llvm/llvm-project/pull/100440
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][spaceship] Implements X::iterator container requirements. (#99343) (PR #100440)

2024-07-24 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne edited 
https://github.com/llvm/llvm-project/pull/100440
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][spaceship] Implements X::iterator container requirements. (#99343) (PR #100440)

2024-07-24 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne approved this pull request.


https://github.com/llvm/llvm-project/pull/100440
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] release/19.x: [libc++] Increase atomic_ref's required alignment for small types (#99654) (PR #101492)

2024-08-01 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne approved this pull request.


https://github.com/llvm/llvm-project/pull/101492
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libcxxabi] [libunwind] release/19x: [NFC][libc++][libc++abi][libunwind][test] Fix/unify AIX triples used in LIT tests (#101196) (PR #101527)

2024-08-01 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne approved this pull request.


https://github.com/llvm/llvm-project/pull/101527
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][bit] Improves rotate functions. (#98032) (PR #101892)

2024-08-05 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne approved this pull request.


https://github.com/llvm/llvm-project/pull/101892
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] Cherry-pick fixes to std::hypot for PowerPC (PR #102052)

2024-08-05 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne milestoned 
https://github.com/llvm/llvm-project/pull/102052
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] Cherry-pick fixes to std::hypot for PowerPC (PR #102052)

2024-08-12 Thread Louis Dionne via llvm-branch-commits

ldionne wrote:

The CI failures are not related to this patch and are being fixed separately. 
This looks good to me for merging.

https://github.com/llvm/llvm-project/pull/102052
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][format][2/7] Optimizes c-string arguments. (PR #101805)

2024-08-13 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne edited 
https://github.com/llvm/llvm-project/pull/101805
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][format][2/7] Optimizes c-string arguments. (PR #101805)

2024-08-13 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne approved this pull request.


https://github.com/llvm/llvm-project/pull/101805
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][format][2/7] Optimizes c-string arguments. (PR #101805)

2024-08-13 Thread Louis Dionne via llvm-branch-commits


@@ -64,32 +64,14 @@ struct _LIBCPP_TEMPLATE_VIS formatter : public __formatte
   template 
   _LIBCPP_HIDE_FROM_ABI typename _FormatContext::iterator format(const _CharT* 
__str, _FormatContext& __ctx) const {
 _LIBCPP_ASSERT_INTERNAL(__str, "The basic_format_arg constructor should 
have prevented an invalid pointer.");
-
-__format_spec::__parsed_specifications<_CharT> __specs = 
_Base::__parser_.__get_parsed_std_specifications(__ctx);
-#  if _LIBCPP_STD_VER >= 23
-if (_Base::__parser_.__type_ == __format_spec::__type::__debug)
-  return 
__formatter::__format_escaped_string(basic_string_view<_CharT>{__str}, 
__ctx.out(), __specs);
-#  endif
-
-// When using a center or right alignment and the width option the length
-// of __str must be known to add the padding upfront. This case is handled
-// by the base class by converting the argument to a basic_string_view.
+// Converting the input to a basic_string_view means the data is looped 
over twice;
+// - once to determine the lenght, and

ldionne wrote:

```suggestion
// - once to determine the length, and
```

https://github.com/llvm/llvm-project/pull/101805
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][format][2/7] Optimizes c-string arguments. (PR #101805)

2024-08-13 Thread Louis Dionne via llvm-branch-commits


@@ -64,32 +64,14 @@ struct _LIBCPP_TEMPLATE_VIS formatter : public __formatte
   template 
   _LIBCPP_HIDE_FROM_ABI typename _FormatContext::iterator format(const _CharT* 
__str, _FormatContext& __ctx) const {
 _LIBCPP_ASSERT_INTERNAL(__str, "The basic_format_arg constructor should 
have prevented an invalid pointer.");
-
-__format_spec::__parsed_specifications<_CharT> __specs = 
_Base::__parser_.__get_parsed_std_specifications(__ctx);
-#  if _LIBCPP_STD_VER >= 23
-if (_Base::__parser_.__type_ == __format_spec::__type::__debug)
-  return 
__formatter::__format_escaped_string(basic_string_view<_CharT>{__str}, 
__ctx.out(), __specs);
-#  endif
-
-// When using a center or right alignment and the width option the length
-// of __str must be known to add the padding upfront. This case is handled
-// by the base class by converting the argument to a basic_string_view.
+// Converting the input to a basic_string_view means the data is looped 
over twice;
+// - once to determine the lenght, and
+// - once to process the data.
 //
-// When using left alignment and the width option the padding is added
-// after outputting __str so the length can be determined while outputting
-// __str. The same holds true for the precision, during outputting __str it
-// can be validated whether the precision threshold has been reached. For
-// now these optimizations aren't implemented. Instead the base class
-// handles these options.
-// TODO FMT Implement these improvements.
-if (__specs.__has_width() || __specs.__has_precision())
-  return __formatter::__write_string(basic_string_view<_CharT>{__str}, 
__ctx.out(), __specs);
-
-// No formatting required, copy the string to the output.
-auto __out_it = __ctx.out();
-while (*__str)
-  *__out_it++ = *__str++;
-return __out_it;
+// This sounds slower than writing the output directly. However internally
+// the output algorithms have optimizations for "bulk" operations. This
+// means processing the data twice is faster than processing it once.

ldionne wrote:

```suggestion
// This sounds slower than writing the output directly. However internally
// the output algorithms have optimizations for "bulk" operations, which 
make
// this faster than a single-pass character-by-character output.
```

https://github.com/llvm/llvm-project/pull/101805
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][format][3/7] Improves std::format performance. (PR #101817)

2024-08-13 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne edited 
https://github.com/llvm/llvm-project/pull/101817
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][format][3/7] Improves std::format performance. (PR #101817)

2024-08-13 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne edited 
https://github.com/llvm/llvm-project/pull/101817
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][format][3/7] Improves std::format performance. (PR #101817)

2024-08-13 Thread Louis Dionne via llvm-branch-commits


@@ -58,23 +59,156 @@ namespace __format {
 /// This helper is used together with the @ref back_insert_iterator to offer
 /// type-erasure for the formatting functions. This reduces the number to
 /// template instantiations.
+///
+/// The design of the class is being changed to improve performance and do some
+/// code cleanups.
+/// The original design (as shipped up to LLVM-19) uses the following design:
+/// - There is an external object that connects the buffer to the output.
+/// - The class constructor stores a function pointer to a grow function and a
+///   type-erased pointer to the object that does the grow.
+/// - When writing data to the buffer would exceed the external buffer's
+///   capacity it requests the external buffer to flush its contents.
+///
+/// The new design tries to solve some issues with the current design:
+/// - The buffer used is a fixed-size buffer, benchmarking shows that using a
+///   dynamic allocated buffer has performance benefits.
+/// - Implementing P3107R5 "Permit an efficient implementation of std::print"
+///   is not trivial with the current buffers. Using the code from this series
+///   makes it trivial.
+///
+/// This class is ABI-tagged, still the new design does not change the size of
+/// objects of this class.
+///
+/// The new design contains information regarding format_to_n changes, these
+/// will be implemented in follow-up patch.
+///
+/// The new design is the following.
+/// - There is an external object that connects the buffer to the output.
+/// - This buffer object:
+///   - inherits publicly from this class.
+///   - has a static or dynamic buffer.
+///   - has a static member function to make space in its buffer write
+/// operations. This can be done by increasing the size of the internal
+/// buffer or by writing the contents of the buffer to the output iterator.
+///
+/// This member function is a constructor argument, so its name is not
+/// fixed. The code uses the name __prepare_write.
+/// - The number of output code units can be limited by a __max_output_size
+///   object. This is used in format_to_n This object:
+///   - Contains the maximum number of code units to be written.
+///   - Contains the number of code units that are requested to be written.
+/// This number is returned to the user of format_to_n.
+///   - The write functions call objects __request_write member function.
+/// This function:
+/// - Updates the number of code units that are requested to be written.
+/// - Returns the number of code units that can be written without
+///   exceeding the maximum number of code units to be written.
+///
+/// Documentation for the buffer usage members:
+/// - __ptr_ the start of the buffer.
+/// - __capacity_ the number of code units that can be written.
+///   This means [__ptr_, __ptr_ + __capacity_) is a valid range to write to.
+/// - __size_ the number of code units written in the buffer. The next code
+///   unit will be written at __ptr_ + __size_. This __size_ may NOT contain
+///   the total number of code units written by the __output_buffer. Whether or
+///   not it does depends on the sub-class used. Typically the total number of
+///   code units written is not interesting. It is interesting for format_to_n
+///   which has its own way to track this number.

ldionne wrote:

```suggestion
/// - __ptr_
///The start of the buffer.
/// - __capacity_
///The number of code units that can be written.
///   This means [__ptr_, __ptr_ + __capacity_) is a valid range to write to.
/// - __size_
///The number of code units written in the buffer. The next code
///   unit will be written at __ptr_ + __size_. This __size_ may NOT contain
///   the total number of code units written by the __output_buffer. Whether or
///   not it does depends on the sub-class used. Typically the total number of
///   code units written is not interesting. It is interesting for format_to_n
///   which has its own way to track this number.
```

Breaking the line makes it easier to see that the first word is the name of the 
member.

https://github.com/llvm/llvm-project/pull/101817
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][format][3/7] Improves std::format performance. (PR #101817)

2024-08-13 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne requested changes to this pull request.

As discussed, I think we should behave more like `std::vector` and not assume 
that elements between `[size(), capacity())` are already initialized. IMO this 
is a simpler mental model that is consistent with `std::vector` and I don't 
think we leave anything on the table by doing that.

https://github.com/llvm/llvm-project/pull/101817
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][format][3/7] Improves std::format performance. (PR #101817)

2024-08-13 Thread Louis Dionne via llvm-branch-commits


@@ -58,23 +59,156 @@ namespace __format {
 /// This helper is used together with the @ref back_insert_iterator to offer
 /// type-erasure for the formatting functions. This reduces the number to
 /// template instantiations.
+///
+/// The design of the class is being changed to improve performance and do some
+/// code cleanups.
+/// The original design (as shipped up to LLVM-19) uses the following design:
+/// - There is an external object that connects the buffer to the output.
+/// - The class constructor stores a function pointer to a grow function and a
+///   type-erased pointer to the object that does the grow.
+/// - When writing data to the buffer would exceed the external buffer's
+///   capacity it requests the external buffer to flush its contents.
+///
+/// The new design tries to solve some issues with the current design:
+/// - The buffer used is a fixed-size buffer, benchmarking shows that using a
+///   dynamic allocated buffer has performance benefits.
+/// - Implementing P3107R5 "Permit an efficient implementation of std::print"
+///   is not trivial with the current buffers. Using the code from this series
+///   makes it trivial.
+///
+/// This class is ABI-tagged, still the new design does not change the size of
+/// objects of this class.
+///
+/// The new design contains information regarding format_to_n changes, these
+/// will be implemented in follow-up patch.
+///
+/// The new design is the following.
+/// - There is an external object that connects the buffer to the output.
+/// - This buffer object:
+///   - inherits publicly from this class.
+///   - has a static or dynamic buffer.
+///   - has a static member function to make space in its buffer write
+/// operations. This can be done by increasing the size of the internal
+/// buffer or by writing the contents of the buffer to the output iterator.
+///
+/// This member function is a constructor argument, so its name is not
+/// fixed. The code uses the name __prepare_write.
+/// - The number of output code units can be limited by a __max_output_size
+///   object. This is used in format_to_n This object:
+///   - Contains the maximum number of code units to be written.
+///   - Contains the number of code units that are requested to be written.
+/// This number is returned to the user of format_to_n.
+///   - The write functions call objects __request_write member function.
+/// This function:
+/// - Updates the number of code units that are requested to be written.
+/// - Returns the number of code units that can be written without
+///   exceeding the maximum number of code units to be written.
+///
+/// Documentation for the buffer usage members:
+/// - __ptr_ the start of the buffer.
+/// - __capacity_ the number of code units that can be written.
+///   This means [__ptr_, __ptr_ + __capacity_) is a valid range to write to.
+/// - __size_ the number of code units written in the buffer. The next code
+///   unit will be written at __ptr_ + __size_. This __size_ may NOT contain
+///   the total number of code units written by the __output_buffer. Whether or
+///   not it does depends on the sub-class used. Typically the total number of
+///   code units written is not interesting. It is interesting for format_to_n
+///   which has its own way to track this number.
+///
+/// Documentation for the buffer changes function:
+/// The subclasses have a function with the following signature:
+///
+///   static void __prepare_write(
+/// __output_buffer<_CharT>& __buffer, size_t __code_units);
+///
+/// This function is called when a write function writes more code units than
+/// the buffer' available space. When an __max_output_size object is provided
+/// the number of code units is the number of code units returned from
+/// __max_output_size::__request_write function.
+///
+/// - The __buffer contains *this. Since the class containing this function
+///   inherits from __output_buffer it's save to cast it to the subclass being
+///   used.
+/// - The __code_units is the number of code units the caller will write + 1.
+///   - This value does not take the avaiable space of the buffer into account.
+///   - The push_back function is more efficient when writing before resizing,
+/// this means the buffer should always have room for one code unit. Hence
+/// the + 1 is the size.
+/// - When the function returns there is room for at least one code unit. There
+///   is no requirement there is room for __code_units code units:
+///   - The class has some "bulk" operations. For example, __copy which copies
+/// the contents of a basic_string_view to the output. If the sub-class has
+/// a fixed size buffer the size of the basic_string_view may be larger
+/// than the buffer. In that case it's impossible to honor the requested
+/// size.
+///   - The at least one code unit makes sure the entire output can be written.
+/// (Obviously making room on

[llvm-branch-commits] [libcxx] [libc++][format][3/7] Improves std::format performance. (PR #101817)

2024-08-13 Thread Louis Dionne via llvm-branch-commits


@@ -58,23 +59,156 @@ namespace __format {
 /// This helper is used together with the @ref back_insert_iterator to offer
 /// type-erasure for the formatting functions. This reduces the number to
 /// template instantiations.
+///
+/// The design of the class is being changed to improve performance and do some
+/// code cleanups.
+/// The original design (as shipped up to LLVM-19) uses the following design:
+/// - There is an external object that connects the buffer to the output.
+/// - The class constructor stores a function pointer to a grow function and a
+///   type-erased pointer to the object that does the grow.

ldionne wrote:

```suggestion
/// - The class constructor stores a function pointer to a "grow" function and a
///   type-erased pointer to the object that should be grown.
```

https://github.com/llvm/llvm-project/pull/101817
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][format][3/7] Improves std::format performance. (PR #101817)

2024-08-13 Thread Louis Dionne via llvm-branch-commits


@@ -499,11 +652,78 @@ struct _LIBCPP_TEMPLATE_VIS __format_to_n_buffer final
   _LIBCPP_HIDE_FROM_ABI auto __make_output_iterator() { return 
this->__output_.__make_output_iterator(); }
 
   _LIBCPP_HIDE_FROM_ABI format_to_n_result<_OutIt> __result() && {
-this->__output_.__flush();
+this->__output_.__flush(0);
 return {std::move(this->__writer_).__out_it(), this->__size_};
   }
 };
 
+// * * * LLVM-20 classes * * *
+
+// A dynamically growing buffer.
+template <__fmt_char_type _CharT>
+class _LIBCPP_TEMPLATE_VIS __allocating_buffer : public 
__output_buffer<_CharT> {
+public:
+  __allocating_buffer(const __allocating_buffer&)= delete;
+  __allocating_buffer& operator=(const __allocating_buffer&) = delete;
+
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI __allocating_buffer()
+  : __output_buffer<_CharT>{__buffer_, __buffer_size_, __prepare_write} {}
+
+  _LIBCPP_HIDE_FROM_ABI ~__allocating_buffer() {
+if (__ptr_ != __buffer_) {
+  ranges::destroy_n(__ptr_, this->__size());
+  allocator_traits<_Alloc>::deallocate(__alloc_, __ptr_, 
this->__capacity());
+}
+  }
+
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI basic_string_view<_CharT> __view() { 
return {__ptr_, this->__size()}; }
+
+private:
+  // At the moment the allocator is hard-code. There might be reasons to have
+  // an allocator trait in the future. This ensures forward compatibility.
+  using _Alloc = allocator<_CharT>;
+  _LIBCPP_NO_UNIQUE_ADDRESS _Alloc __alloc_;
+
+  // Since allocating is expensive the class has a small internal buffer. When
+  // its capacity is exceeded a dynamic buffer will be allocated.
+  static constexpr size_t __buffer_size_ = 256;
+  _CharT __buffer_[__buffer_size_];
+  _CharT* __ptr_{__buffer_};
+
+  _LIBCPP_HIDE_FROM_ABI void __grow_buffer(size_t __capacity) {
+if (__capacity < __buffer_size_)
+  return;
+
+_LIBCPP_ASSERT_INTERNAL(__capacity > this->__capacity(), "the buffer must 
grow");
+auto __result = std::__allocate_at_least(__alloc_, __capacity);
+auto __guard  = std::__make_exception_guard([&] {
+  allocator_traits<_Alloc>::deallocate(__alloc_, __result.ptr, 
__result.count);
+});
+// This shouldn't throw, but just to be safe. Note that at -O1 this
+// guard is optimized away so there is no runtime overhead.
+new (__result.ptr) _CharT[__result.count];
+std::copy_n(__ptr_, this->__size(), __result.ptr);
+__guard.__complete();
+if (__ptr_ != __buffer_) {
+  ranges::destroy_n(__ptr_, this->__capacity());
+  allocator_traits<_Alloc>::deallocate(__alloc_, __ptr_, 
this->__capacity());
+}

ldionne wrote:

```suggestion
auto __guard  = std::__make_exception_guard([&] {
  allocator_traits<_Alloc>::deallocate(__alloc_, __result.ptr, 
__result.count);
});
// This shouldn't throw, but just to be safe. Note that at -O1 this
// guard is optimized away so there is no runtime overhead.
std::__uninitialized_allocator_relocate(__alloc_, __ptr_, __ptr_ + 
this->__size(), __result.ptr);
__guard.__complete();
if (__ptr_ != __buffer_) {
  allocator_traits<_Alloc>::deallocate(__alloc_, __ptr_, 
this->__capacity());
}
```

https://github.com/llvm/llvm-project/pull/101817
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][format][3/7] Improves std::format performance. (PR #101817)

2024-08-13 Thread Louis Dionne via llvm-branch-commits


@@ -58,23 +59,156 @@ namespace __format {
 /// This helper is used together with the @ref back_insert_iterator to offer
 /// type-erasure for the formatting functions. This reduces the number to
 /// template instantiations.
+///
+/// The design of the class is being changed to improve performance and do some
+/// code cleanups.
+/// The original design (as shipped up to LLVM-19) uses the following design:
+/// - There is an external object that connects the buffer to the output.
+/// - The class constructor stores a function pointer to a grow function and a
+///   type-erased pointer to the object that does the grow.
+/// - When writing data to the buffer would exceed the external buffer's
+///   capacity it requests the external buffer to flush its contents.
+///
+/// The new design tries to solve some issues with the current design:
+/// - The buffer used is a fixed-size buffer, benchmarking shows that using a
+///   dynamic allocated buffer has performance benefits.
+/// - Implementing P3107R5 "Permit an efficient implementation of std::print"
+///   is not trivial with the current buffers. Using the code from this series
+///   makes it trivial.
+///
+/// This class is ABI-tagged, still the new design does not change the size of
+/// objects of this class.
+///
+/// The new design contains information regarding format_to_n changes, these
+/// will be implemented in follow-up patch.
+///
+/// The new design is the following.
+/// - There is an external object that connects the buffer to the output.
+/// - This buffer object:
+///   - inherits publicly from this class.
+///   - has a static or dynamic buffer.
+///   - has a static member function to make space in its buffer write
+/// operations. This can be done by increasing the size of the internal
+/// buffer or by writing the contents of the buffer to the output iterator.
+///
+/// This member function is a constructor argument, so its name is not
+/// fixed. The code uses the name __prepare_write.
+/// - The number of output code units can be limited by a __max_output_size
+///   object. This is used in format_to_n This object:
+///   - Contains the maximum number of code units to be written.
+///   - Contains the number of code units that are requested to be written.
+/// This number is returned to the user of format_to_n.

ldionne wrote:

```suggestion
///   - Contains the number of code units that were actually written.
/// This number is returned to the user of format_to_n.
```

https://github.com/llvm/llvm-project/pull/101817
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][format][3/7] Improves std::format performance. (PR #101817)

2024-08-13 Thread Louis Dionne via llvm-branch-commits


@@ -58,23 +59,156 @@ namespace __format {
 /// This helper is used together with the @ref back_insert_iterator to offer
 /// type-erasure for the formatting functions. This reduces the number to
 /// template instantiations.
+///
+/// The design of the class is being changed to improve performance and do some
+/// code cleanups.
+/// The original design (as shipped up to LLVM-19) uses the following design:
+/// - There is an external object that connects the buffer to the output.
+/// - The class constructor stores a function pointer to a grow function and a
+///   type-erased pointer to the object that does the grow.
+/// - When writing data to the buffer would exceed the external buffer's
+///   capacity it requests the external buffer to flush its contents.
+///
+/// The new design tries to solve some issues with the current design:
+/// - The buffer used is a fixed-size buffer, benchmarking shows that using a
+///   dynamic allocated buffer has performance benefits.
+/// - Implementing P3107R5 "Permit an efficient implementation of std::print"
+///   is not trivial with the current buffers. Using the code from this series
+///   makes it trivial.
+///
+/// This class is ABI-tagged, still the new design does not change the size of
+/// objects of this class.
+///
+/// The new design contains information regarding format_to_n changes, these
+/// will be implemented in follow-up patch.
+///
+/// The new design is the following.
+/// - There is an external object that connects the buffer to the output.
+/// - This buffer object:
+///   - inherits publicly from this class.
+///   - has a static or dynamic buffer.
+///   - has a static member function to make space in its buffer write
+/// operations. This can be done by increasing the size of the internal
+/// buffer or by writing the contents of the buffer to the output iterator.
+///
+/// This member function is a constructor argument, so its name is not
+/// fixed. The code uses the name __prepare_write.
+/// - The number of output code units can be limited by a __max_output_size
+///   object. This is used in format_to_n This object:
+///   - Contains the maximum number of code units to be written.
+///   - Contains the number of code units that are requested to be written.
+/// This number is returned to the user of format_to_n.
+///   - The write functions call objects __request_write member function.
+/// This function:
+/// - Updates the number of code units that are requested to be written.
+/// - Returns the number of code units that can be written without
+///   exceeding the maximum number of code units to be written.
+///
+/// Documentation for the buffer usage members:
+/// - __ptr_ the start of the buffer.
+/// - __capacity_ the number of code units that can be written.
+///   This means [__ptr_, __ptr_ + __capacity_) is a valid range to write to.
+/// - __size_ the number of code units written in the buffer. The next code
+///   unit will be written at __ptr_ + __size_. This __size_ may NOT contain
+///   the total number of code units written by the __output_buffer. Whether or
+///   not it does depends on the sub-class used. Typically the total number of
+///   code units written is not interesting. It is interesting for format_to_n
+///   which has its own way to track this number.
+///
+/// Documentation for the buffer changes function:
+/// The subclasses have a function with the following signature:
+///
+///   static void __prepare_write(
+/// __output_buffer<_CharT>& __buffer, size_t __code_units);
+///
+/// This function is called when a write function writes more code units than
+/// the buffer' available space. When an __max_output_size object is provided

ldionne wrote:

```suggestion
/// the buffer's available space. When an __max_output_size object is provided
```

https://github.com/llvm/llvm-project/pull/101817
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][format][3/7] Improves std::format performance. (PR #101817)

2024-08-13 Thread Louis Dionne via llvm-branch-commits


@@ -58,23 +59,156 @@ namespace __format {
 /// This helper is used together with the @ref back_insert_iterator to offer
 /// type-erasure for the formatting functions. This reduces the number to
 /// template instantiations.
+///
+/// The design of the class is being changed to improve performance and do some
+/// code cleanups.
+/// The original design (as shipped up to LLVM-19) uses the following design:
+/// - There is an external object that connects the buffer to the output.
+/// - The class constructor stores a function pointer to a grow function and a
+///   type-erased pointer to the object that does the grow.
+/// - When writing data to the buffer would exceed the external buffer's
+///   capacity it requests the external buffer to flush its contents.
+///
+/// The new design tries to solve some issues with the current design:
+/// - The buffer used is a fixed-size buffer, benchmarking shows that using a
+///   dynamic allocated buffer has performance benefits.
+/// - Implementing P3107R5 "Permit an efficient implementation of std::print"
+///   is not trivial with the current buffers. Using the code from this series
+///   makes it trivial.
+///
+/// This class is ABI-tagged, still the new design does not change the size of
+/// objects of this class.
+///
+/// The new design contains information regarding format_to_n changes, these
+/// will be implemented in follow-up patch.
+///
+/// The new design is the following.
+/// - There is an external object that connects the buffer to the output.
+/// - This buffer object:
+///   - inherits publicly from this class.
+///   - has a static or dynamic buffer.
+///   - has a static member function to make space in its buffer write
+/// operations. This can be done by increasing the size of the internal
+/// buffer or by writing the contents of the buffer to the output iterator.
+///
+/// This member function is a constructor argument, so its name is not
+/// fixed. The code uses the name __prepare_write.
+/// - The number of output code units can be limited by a __max_output_size
+///   object. This is used in format_to_n This object:
+///   - Contains the maximum number of code units to be written.
+///   - Contains the number of code units that are requested to be written.
+/// This number is returned to the user of format_to_n.
+///   - The write functions call objects __request_write member function.
+/// This function:
+/// - Updates the number of code units that are requested to be written.
+/// - Returns the number of code units that can be written without
+///   exceeding the maximum number of code units to be written.
+///
+/// Documentation for the buffer usage members:
+/// - __ptr_ the start of the buffer.
+/// - __capacity_ the number of code units that can be written.
+///   This means [__ptr_, __ptr_ + __capacity_) is a valid range to write to.
+/// - __size_ the number of code units written in the buffer. The next code
+///   unit will be written at __ptr_ + __size_. This __size_ may NOT contain
+///   the total number of code units written by the __output_buffer. Whether or
+///   not it does depends on the sub-class used. Typically the total number of
+///   code units written is not interesting. It is interesting for format_to_n
+///   which has its own way to track this number.
+///
+/// Documentation for the buffer changes function:
+/// The subclasses have a function with the following signature:
+///
+///   static void __prepare_write(
+/// __output_buffer<_CharT>& __buffer, size_t __code_units);
+///
+/// This function is called when a write function writes more code units than
+/// the buffer' available space. When an __max_output_size object is provided
+/// the number of code units is the number of code units returned from
+/// __max_output_size::__request_write function.
+///
+/// - The __buffer contains *this. Since the class containing this function
+///   inherits from __output_buffer it's save to cast it to the subclass being
+///   used.
+/// - The __code_units is the number of code units the caller will write + 1.
+///   - This value does not take the avaiable space of the buffer into account.
+///   - The push_back function is more efficient when writing before resizing,
+/// this means the buffer should always have room for one code unit. Hence
+/// the + 1 is the size.
+/// - When the function returns there is room for at least one code unit. There
+///   is no requirement there is room for __code_units code units:
+///   - The class has some "bulk" operations. For example, __copy which copies
+/// the contents of a basic_string_view to the output. If the sub-class has
+/// a fixed size buffer the size of the basic_string_view may be larger
+/// than the buffer. In that case it's impossible to honor the requested
+/// size.
+///   - The at least one code unit makes sure the entire output can be written.
+/// (Obviously making room on

[llvm-branch-commits] [libcxx] [libc++][format][3/7] Improves std::format performance. (PR #101817)

2024-08-13 Thread Louis Dionne via llvm-branch-commits


@@ -58,23 +59,156 @@ namespace __format {
 /// This helper is used together with the @ref back_insert_iterator to offer
 /// type-erasure for the formatting functions. This reduces the number to
 /// template instantiations.
+///
+/// The design of the class is being changed to improve performance and do some
+/// code cleanups.
+/// The original design (as shipped up to LLVM-19) uses the following design:
+/// - There is an external object that connects the buffer to the output.
+/// - The class constructor stores a function pointer to a grow function and a
+///   type-erased pointer to the object that does the grow.
+/// - When writing data to the buffer would exceed the external buffer's
+///   capacity it requests the external buffer to flush its contents.
+///
+/// The new design tries to solve some issues with the current design:
+/// - The buffer used is a fixed-size buffer, benchmarking shows that using a
+///   dynamic allocated buffer has performance benefits.
+/// - Implementing P3107R5 "Permit an efficient implementation of std::print"
+///   is not trivial with the current buffers. Using the code from this series
+///   makes it trivial.
+///
+/// This class is ABI-tagged, still the new design does not change the size of
+/// objects of this class.
+///
+/// The new design contains information regarding format_to_n changes, these
+/// will be implemented in follow-up patch.
+///
+/// The new design is the following.
+/// - There is an external object that connects the buffer to the output.
+/// - This buffer object:
+///   - inherits publicly from this class.
+///   - has a static or dynamic buffer.
+///   - has a static member function to make space in its buffer write
+/// operations. This can be done by increasing the size of the internal
+/// buffer or by writing the contents of the buffer to the output iterator.
+///
+/// This member function is a constructor argument, so its name is not
+/// fixed. The code uses the name __prepare_write.
+/// - The number of output code units can be limited by a __max_output_size
+///   object. This is used in format_to_n This object:
+///   - Contains the maximum number of code units to be written.
+///   - Contains the number of code units that are requested to be written.
+/// This number is returned to the user of format_to_n.
+///   - The write functions call objects __request_write member function.
+/// This function:
+/// - Updates the number of code units that are requested to be written.
+/// - Returns the number of code units that can be written without
+///   exceeding the maximum number of code units to be written.
+///
+/// Documentation for the buffer usage members:
+/// - __ptr_ the start of the buffer.
+/// - __capacity_ the number of code units that can be written.
+///   This means [__ptr_, __ptr_ + __capacity_) is a valid range to write to.
+/// - __size_ the number of code units written in the buffer. The next code
+///   unit will be written at __ptr_ + __size_. This __size_ may NOT contain
+///   the total number of code units written by the __output_buffer. Whether or
+///   not it does depends on the sub-class used. Typically the total number of
+///   code units written is not interesting. It is interesting for format_to_n
+///   which has its own way to track this number.
+///
+/// Documentation for the buffer changes function:
+/// The subclasses have a function with the following signature:
+///
+///   static void __prepare_write(
+/// __output_buffer<_CharT>& __buffer, size_t __code_units);
+///
+/// This function is called when a write function writes more code units than
+/// the buffer' available space. When an __max_output_size object is provided
+/// the number of code units is the number of code units returned from
+/// __max_output_size::__request_write function.
+///
+/// - The __buffer contains *this. Since the class containing this function
+///   inherits from __output_buffer it's save to cast it to the subclass being

ldionne wrote:

```suggestion
///   inherits from __output_buffer it's safe to cast it to the subclass being
```

https://github.com/llvm/llvm-project/pull/101817
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][format][3/7] Improves std::format performance. (PR #101817)

2024-08-13 Thread Louis Dionne via llvm-branch-commits


@@ -58,23 +59,156 @@ namespace __format {
 /// This helper is used together with the @ref back_insert_iterator to offer
 /// type-erasure for the formatting functions. This reduces the number to
 /// template instantiations.
+///
+/// The design of the class is being changed to improve performance and do some
+/// code cleanups.
+/// The original design (as shipped up to LLVM-19) uses the following design:
+/// - There is an external object that connects the buffer to the output.
+/// - The class constructor stores a function pointer to a grow function and a
+///   type-erased pointer to the object that does the grow.
+/// - When writing data to the buffer would exceed the external buffer's
+///   capacity it requests the external buffer to flush its contents.
+///
+/// The new design tries to solve some issues with the current design:
+/// - The buffer used is a fixed-size buffer, benchmarking shows that using a
+///   dynamic allocated buffer has performance benefits.
+/// - Implementing P3107R5 "Permit an efficient implementation of std::print"
+///   is not trivial with the current buffers. Using the code from this series
+///   makes it trivial.
+///
+/// This class is ABI-tagged, still the new design does not change the size of
+/// objects of this class.
+///
+/// The new design contains information regarding format_to_n changes, these
+/// will be implemented in follow-up patch.
+///
+/// The new design is the following.
+/// - There is an external object that connects the buffer to the output.
+/// - This buffer object:
+///   - inherits publicly from this class.
+///   - has a static or dynamic buffer.
+///   - has a static member function to make space in its buffer write
+/// operations. This can be done by increasing the size of the internal
+/// buffer or by writing the contents of the buffer to the output iterator.
+///
+/// This member function is a constructor argument, so its name is not
+/// fixed. The code uses the name __prepare_write.
+/// - The number of output code units can be limited by a __max_output_size
+///   object. This is used in format_to_n This object:
+///   - Contains the maximum number of code units to be written.
+///   - Contains the number of code units that are requested to be written.
+/// This number is returned to the user of format_to_n.
+///   - The write functions call objects __request_write member function.
+/// This function:

ldionne wrote:

```suggestion
///   - The write functions call the object's __request_write member function.
/// This function:
```

https://github.com/llvm/llvm-project/pull/101817
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][format][3/7] Improves std::format performance. (PR #101817)

2024-08-13 Thread Louis Dionne via llvm-branch-commits


@@ -499,11 +652,78 @@ struct _LIBCPP_TEMPLATE_VIS __format_to_n_buffer final
   _LIBCPP_HIDE_FROM_ABI auto __make_output_iterator() { return 
this->__output_.__make_output_iterator(); }
 
   _LIBCPP_HIDE_FROM_ABI format_to_n_result<_OutIt> __result() && {
-this->__output_.__flush();
+this->__output_.__flush(0);
 return {std::move(this->__writer_).__out_it(), this->__size_};
   }
 };
 
+// * * * LLVM-20 classes * * *
+
+// A dynamically growing buffer.
+template <__fmt_char_type _CharT>
+class _LIBCPP_TEMPLATE_VIS __allocating_buffer : public 
__output_buffer<_CharT> {
+public:
+  __allocating_buffer(const __allocating_buffer&)= delete;
+  __allocating_buffer& operator=(const __allocating_buffer&) = delete;
+
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI __allocating_buffer()
+  : __output_buffer<_CharT>{__buffer_, __buffer_size_, __prepare_write} {}
+
+  _LIBCPP_HIDE_FROM_ABI ~__allocating_buffer() {
+if (__ptr_ != __buffer_) {
+  ranges::destroy_n(__ptr_, this->__size());
+  allocator_traits<_Alloc>::deallocate(__alloc_, __ptr_, 
this->__capacity());
+}
+  }
+
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI basic_string_view<_CharT> __view() { 
return {__ptr_, this->__size()}; }
+
+private:
+  // At the moment the allocator is hard-code. There might be reasons to have
+  // an allocator trait in the future. This ensures forward compatibility.
+  using _Alloc = allocator<_CharT>;
+  _LIBCPP_NO_UNIQUE_ADDRESS _Alloc __alloc_;

ldionne wrote:

If we hardcode the allocator, I would simply not use one. This would simplify 
business with `construct_at` & friends too.

https://github.com/llvm/llvm-project/pull/101817
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][format][3/7] Improves std::format performance. (PR #101817)

2024-08-13 Thread Louis Dionne via llvm-branch-commits


@@ -452,9 +452,9 @@ format_to(_OutIt __out_it, wformat_string<_Args...> __fmt, 
_Args&&... __args) {
 // fires too eagerly, see http://llvm.org/PR61563.
 template 
 [[nodiscard]] _LIBCPP_ALWAYS_INLINE inline _LIBCPP_HIDE_FROM_ABI string 
vformat(string_view __fmt, format_args __args) {
-  string __res;
-  std::vformat_to(std::back_inserter(__res), __fmt, __args);
-  return __res;
+  __format::__allocating_buffer __buffer;
+  std::vformat_to(__buffer.__make_output_iterator(), __fmt, __args);

ldionne wrote:

Can we remove an include since you don't mention `back_inserter` directly 
anymore?

https://github.com/llvm/llvm-project/pull/101817
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][format][3/7] Improves std::format performance. (PR #101817)

2024-08-13 Thread Louis Dionne via llvm-branch-commits


@@ -499,11 +652,78 @@ struct _LIBCPP_TEMPLATE_VIS __format_to_n_buffer final
   _LIBCPP_HIDE_FROM_ABI auto __make_output_iterator() { return 
this->__output_.__make_output_iterator(); }
 
   _LIBCPP_HIDE_FROM_ABI format_to_n_result<_OutIt> __result() && {
-this->__output_.__flush();
+this->__output_.__flush(0);
 return {std::move(this->__writer_).__out_it(), this->__size_};
   }
 };
 
+// * * * LLVM-20 classes * * *
+
+// A dynamically growing buffer.
+template <__fmt_char_type _CharT>
+class _LIBCPP_TEMPLATE_VIS __allocating_buffer : public 
__output_buffer<_CharT> {
+public:
+  __allocating_buffer(const __allocating_buffer&)= delete;
+  __allocating_buffer& operator=(const __allocating_buffer&) = delete;
+
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI __allocating_buffer()
+  : __output_buffer<_CharT>{__buffer_, __buffer_size_, __prepare_write} {}
+
+  _LIBCPP_HIDE_FROM_ABI ~__allocating_buffer() {
+if (__ptr_ != __buffer_) {
+  ranges::destroy_n(__ptr_, this->__size());
+  allocator_traits<_Alloc>::deallocate(__alloc_, __ptr_, 
this->__capacity());
+}
+  }
+
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI basic_string_view<_CharT> __view() { 
return {__ptr_, this->__size()}; }
+
+private:
+  // At the moment the allocator is hard-code. There might be reasons to have
+  // an allocator trait in the future. This ensures forward compatibility.
+  using _Alloc = allocator<_CharT>;
+  _LIBCPP_NO_UNIQUE_ADDRESS _Alloc __alloc_;
+
+  // Since allocating is expensive the class has a small internal buffer. When
+  // its capacity is exceeded a dynamic buffer will be allocated.
+  static constexpr size_t __buffer_size_ = 256;
+  _CharT __buffer_[__buffer_size_];

ldionne wrote:

```suggestion
  std::byte __buffer_[__buffer_size_ * sizeof(_CharT)];
```

https://github.com/llvm/llvm-project/pull/101817
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] release/19.x: [libc++] Use a different smart ptr type alias (#102089) (PR #103003)

2024-08-13 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne approved this pull request.


https://github.com/llvm/llvm-project/pull/103003
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] release/19.x: [libc++] Fix rejects-valid in std::span copy construction (#104500) (PR #104603)

2024-08-16 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne approved this pull request.


https://github.com/llvm/llvm-project/pull/104603
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][format][4/7] Improves std::format_to performance. (PR #101823)

2024-08-20 Thread Louis Dionne via llvm-branch-commits


@@ -722,6 +724,95 @@ class _LIBCPP_TEMPLATE_VIS __allocating_buffer : public 
__output_buffer<_CharT>
   }
 };
 
+// A buffer that directly writes to the underlying buffer.
+template 
+class _LIBCPP_TEMPLATE_VIS __direct_iterator_buffer : public 
__output_buffer<_CharT> {
+public:
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI explicit __direct_iterator_buffer(_OutIt 
__out_it)
+  : __output_buffer<_CharT>{std::__unwrap_iter(__out_it), __buffer_size, 
__prepare_write}, __out_it_(__out_it) {}
+
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI _OutIt __out_it() && { return __out_it_ 
+ this->__size(); }
+
+private:
+  // The function format_to expects a buffer large enough for the output. The
+  // function format_to_n has its own helper class that restricts the number of
+  // write options. So this function class can pretend to have an infinite
+  // buffer.
+  static constexpr size_t __buffer_size = -1;
+
+  _OutIt __out_it_;
+
+  _LIBCPP_HIDE_FROM_ABI static void
+  __prepare_write([[maybe_unused]] __output_buffer<_CharT>& __buffer, 
[[maybe_unused]] size_t __size_hint) {
+std::__throw_length_error("__direct_iterator_buffer");
+  }
+};
+
+// A buffer that writes its output to the end of a container.
+template 
+class _LIBCPP_TEMPLATE_VIS __container_inserter_buffer : public 
__output_buffer<_CharT> {
+public:
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI explicit 
__container_inserter_buffer(_OutIt __out_it)
+  : __output_buffer<_CharT>{__buffer_, __buffer_size, __prepare_write}, 
__container_{__out_it.__get_container()} {}
+
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI auto __out_it() && {
+__container_->insert(__container_->end(), __buffer_, __buffer_ + 
this->__size());
+return std::back_inserter(*__container_);
+  }
+
+private:
+  typename __back_insert_iterator_container<_OutIt>::type* __container_;
+
+  // This class uses a fixed size buffer and appends the elements in
+  // __buffer_size chunks. An alternative would be to use an allocating buffer
+  // and append the output in one write operation. Benchmarking showed no
+  // performance difference.
+  static constexpr size_t __buffer_size = 256;
+  _CharT __buffer_[__buffer_size];
+
+  _LIBCPP_HIDE_FROM_ABI void __prepare_write() {
+__container_->insert(__container_->end(), __buffer_, __buffer_ + 
this->__size());
+this->__buffer_flused();
+  }
+
+  _LIBCPP_HIDE_FROM_ABI static void
+  __prepare_write(__output_buffer<_CharT>& __buffer, [[maybe_unused]] size_t 
__size_hint) {
+static_cast<__container_inserter_buffer<_OutIt, 
_CharT>&>(__buffer).__prepare_write();
+  }
+};
+
+// A buffer that writes to an iterator.
+//
+// Unlinke the __container_inserter_buffer this class' perfomance does benefit

ldionne wrote:

```suggestion
// Unlike the __container_inserter_buffer this class' performance does benefit
```

https://github.com/llvm/llvm-project/pull/101823
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][format][3/7] Improves std::format performance. (PR #101817)

2024-08-20 Thread Louis Dionne via llvm-branch-commits


@@ -452,9 +452,9 @@ format_to(_OutIt __out_it, wformat_string<_Args...> __fmt, 
_Args&&... __args) {
 // fires too eagerly, see http://llvm.org/PR61563.
 template 
 [[nodiscard]] _LIBCPP_ALWAYS_INLINE inline _LIBCPP_HIDE_FROM_ABI string 
vformat(string_view __fmt, format_args __args) {
-  string __res;
-  std::vformat_to(std::back_inserter(__res), __fmt, __args);
-  return __res;
+  __format::__allocating_buffer __buffer;

ldionne wrote:

It would actually be really nice if `vformat_to(std::back_inserter(string), 
...)` were just as performant as using the allocating buffer. It seems to me 
that if using the `__allocating_buffer` directly is faster, there may be a 
problem with next patch's `buffer_selector` which would basically be picking a 
less efficient approach for the case of `std::back_inserter`.

https://github.com/llvm/llvm-project/pull/101817
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][format][3/7] Improves std::format performance. (PR #101817)

2024-08-20 Thread Louis Dionne via llvm-branch-commits


@@ -58,23 +59,156 @@ namespace __format {
 /// This helper is used together with the @ref back_insert_iterator to offer
 /// type-erasure for the formatting functions. This reduces the number to
 /// template instantiations.
+///
+/// The design of the class is being changed to improve performance and do some
+/// code cleanups.
+/// The original design (as shipped up to LLVM-19) uses the following design:
+/// - There is an external object that connects the buffer to the output.
+/// - The class constructor stores a function pointer to a grow function and a
+///   type-erased pointer to the object that does the grow.
+/// - When writing data to the buffer would exceed the external buffer's
+///   capacity it requests the external buffer to flush its contents.
+///
+/// The new design tries to solve some issues with the current design:
+/// - The buffer used is a fixed-size buffer, benchmarking shows that using a
+///   dynamic allocated buffer has performance benefits.
+/// - Implementing P3107R5 "Permit an efficient implementation of std::print"
+///   is not trivial with the current buffers. Using the code from this series
+///   makes it trivial.
+///
+/// This class is ABI-tagged, still the new design does not change the size of
+/// objects of this class.
+///
+/// The new design contains information regarding format_to_n changes, these
+/// will be implemented in follow-up patch.
+///
+/// The new design is the following.
+/// - There is an external object that connects the buffer to the output.
+/// - This buffer object:
+///   - inherits publicly from this class.
+///   - has a static or dynamic buffer.
+///   - has a static member function to make space in its buffer write
+/// operations. This can be done by increasing the size of the internal
+/// buffer or by writing the contents of the buffer to the output iterator.
+///
+/// This member function is a constructor argument, so its name is not
+/// fixed. The code uses the name __prepare_write.
+/// - The number of output code units can be limited by a __max_output_size
+///   object. This is used in format_to_n This object:
+///   - Contains the maximum number of code units to be written.
+///   - Contains the number of code units that are requested to be written.
+/// This number is returned to the user of format_to_n.
+///   - The write functions call objects __request_write member function.
+/// This function:
+/// - Updates the number of code units that are requested to be written.
+/// - Returns the number of code units that can be written without
+///   exceeding the maximum number of code units to be written.
+///
+/// Documentation for the buffer usage members:
+/// - __ptr_ the start of the buffer.
+/// - __capacity_ the number of code units that can be written.
+///   This means [__ptr_, __ptr_ + __capacity_) is a valid range to write to.
+/// - __size_ the number of code units written in the buffer. The next code
+///   unit will be written at __ptr_ + __size_. This __size_ may NOT contain
+///   the total number of code units written by the __output_buffer. Whether or
+///   not it does depends on the sub-class used. Typically the total number of
+///   code units written is not interesting. It is interesting for format_to_n
+///   which has its own way to track this number.
+///
+/// Documentation for the buffer changes function:
+/// The subclasses have a function with the following signature:
+///
+///   static void __prepare_write(
+/// __output_buffer<_CharT>& __buffer, size_t __code_units);
+///
+/// This function is called when a write function writes more code units than
+/// the buffer' available space. When an __max_output_size object is provided
+/// the number of code units is the number of code units returned from
+/// __max_output_size::__request_write function.
+///
+/// - The __buffer contains *this. Since the class containing this function
+///   inherits from __output_buffer it's save to cast it to the subclass being
+///   used.
+/// - The __code_units is the number of code units the caller will write + 1.
+///   - This value does not take the avaiable space of the buffer into account.
+///   - The push_back function is more efficient when writing before resizing,
+/// this means the buffer should always have room for one code unit. Hence
+/// the + 1 is the size.
+/// - When the function returns there is room for at least one code unit. There
+///   is no requirement there is room for __code_units code units:
+///   - The class has some "bulk" operations. For example, __copy which copies
+/// the contents of a basic_string_view to the output. If the sub-class has
+/// a fixed size buffer the size of the basic_string_view may be larger
+/// than the buffer. In that case it's impossible to honor the requested
+/// size.
+///   - The at least one code unit makes sure the entire output can be written.
+/// (Obviously making room on

[llvm-branch-commits] [libcxx] [libc++][format][3/7] Improves std::format performance. (PR #101817)

2024-08-20 Thread Louis Dionne via llvm-branch-commits


@@ -58,23 +59,156 @@ namespace __format {
 /// This helper is used together with the @ref back_insert_iterator to offer
 /// type-erasure for the formatting functions. This reduces the number to
 /// template instantiations.
+///
+/// The design of the class is being changed to improve performance and do some
+/// code cleanups.
+/// The original design (as shipped up to LLVM-19) uses the following design:
+/// - There is an external object that connects the buffer to the output.
+/// - The class constructor stores a function pointer to a grow function and a
+///   type-erased pointer to the object that does the grow.
+/// - When writing data to the buffer would exceed the external buffer's
+///   capacity it requests the external buffer to flush its contents.
+///
+/// The new design tries to solve some issues with the current design:
+/// - The buffer used is a fixed-size buffer, benchmarking shows that using a
+///   dynamic allocated buffer has performance benefits.
+/// - Implementing P3107R5 "Permit an efficient implementation of std::print"
+///   is not trivial with the current buffers. Using the code from this series
+///   makes it trivial.
+///
+/// This class is ABI-tagged, still the new design does not change the size of
+/// objects of this class.
+///
+/// The new design contains information regarding format_to_n changes, these
+/// will be implemented in follow-up patch.
+///
+/// The new design is the following.
+/// - There is an external object that connects the buffer to the output.
+/// - This buffer object:
+///   - inherits publicly from this class.
+///   - has a static or dynamic buffer.
+///   - has a static member function to make space in its buffer write
+/// operations. This can be done by increasing the size of the internal
+/// buffer or by writing the contents of the buffer to the output iterator.
+///
+/// This member function is a constructor argument, so its name is not
+/// fixed. The code uses the name __prepare_write.
+/// - The number of output code units can be limited by a __max_output_size
+///   object. This is used in format_to_n This object:
+///   - Contains the maximum number of code units to be written.
+///   - Contains the number of code units that are requested to be written.
+/// This number is returned to the user of format_to_n.
+///   - The write functions call objects __request_write member function.
+/// This function:
+/// - Updates the number of code units that are requested to be written.
+/// - Returns the number of code units that can be written without
+///   exceeding the maximum number of code units to be written.
+///
+/// Documentation for the buffer usage members:
+/// - __ptr_ the start of the buffer.
+/// - __capacity_ the number of code units that can be written.
+///   This means [__ptr_, __ptr_ + __capacity_) is a valid range to write to.
+/// - __size_ the number of code units written in the buffer. The next code
+///   unit will be written at __ptr_ + __size_. This __size_ may NOT contain
+///   the total number of code units written by the __output_buffer. Whether or
+///   not it does depends on the sub-class used. Typically the total number of
+///   code units written is not interesting. It is interesting for format_to_n
+///   which has its own way to track this number.
+///
+/// Documentation for the buffer changes function:
+/// The subclasses have a function with the following signature:
+///
+///   static void __prepare_write(
+/// __output_buffer<_CharT>& __buffer, size_t __code_units);
+///
+/// This function is called when a write function writes more code units than
+/// the buffer' available space. When an __max_output_size object is provided
+/// the number of code units is the number of code units returned from
+/// __max_output_size::__request_write function.
+///
+/// - The __buffer contains *this. Since the class containing this function
+///   inherits from __output_buffer it's save to cast it to the subclass being
+///   used.
+/// - The __code_units is the number of code units the caller will write + 1.
+///   - This value does not take the avaiable space of the buffer into account.
+///   - The push_back function is more efficient when writing before resizing,
+/// this means the buffer should always have room for one code unit. Hence
+/// the + 1 is the size.
+/// - When the function returns there is room for at least one code unit. There
+///   is no requirement there is room for __code_units code units:
+///   - The class has some "bulk" operations. For example, __copy which copies
+/// the contents of a basic_string_view to the output. If the sub-class has
+/// a fixed size buffer the size of the basic_string_view may be larger
+/// than the buffer. In that case it's impossible to honor the requested
+/// size.
+///   - The at least one code unit makes sure the entire output can be written.
+/// (Obviously making room on

[llvm-branch-commits] [libcxx] [libc++][format][4/7] Improves std::format_to performance. (PR #101823)

2024-08-20 Thread Louis Dionne via llvm-branch-commits

https://github.com/ldionne requested changes to this pull request.


https://github.com/llvm/llvm-project/pull/101823
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [libcxx] [libc++][format][4/7] Improves std::format_to performance. (PR #101823)

2024-08-20 Thread Louis Dionne via llvm-branch-commits


@@ -722,6 +724,95 @@ class _LIBCPP_TEMPLATE_VIS __allocating_buffer : public 
__output_buffer<_CharT>
   }
 };
 
+// A buffer that directly writes to the underlying buffer.
+template 
+class _LIBCPP_TEMPLATE_VIS __direct_iterator_buffer : public 
__output_buffer<_CharT> {
+public:
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI explicit __direct_iterator_buffer(_OutIt 
__out_it)
+  : __output_buffer<_CharT>{std::__unwrap_iter(__out_it), __buffer_size, 
__prepare_write}, __out_it_(__out_it) {}
+
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI _OutIt __out_it() && { return __out_it_ 
+ this->__size(); }
+
+private:
+  // The function format_to expects a buffer large enough for the output. The
+  // function format_to_n has its own helper class that restricts the number of
+  // write options. So this function class can pretend to have an infinite
+  // buffer.
+  static constexpr size_t __buffer_size = -1;
+
+  _OutIt __out_it_;
+
+  _LIBCPP_HIDE_FROM_ABI static void
+  __prepare_write([[maybe_unused]] __output_buffer<_CharT>& __buffer, 
[[maybe_unused]] size_t __size_hint) {
+std::__throw_length_error("__direct_iterator_buffer");
+  }
+};
+
+// A buffer that writes its output to the end of a container.
+template 
+class _LIBCPP_TEMPLATE_VIS __container_inserter_buffer : public 
__output_buffer<_CharT> {
+public:
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI explicit 
__container_inserter_buffer(_OutIt __out_it)
+  : __output_buffer<_CharT>{__buffer_, __buffer_size, __prepare_write}, 
__container_{__out_it.__get_container()} {}
+
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI auto __out_it() && {
+__container_->insert(__container_->end(), __buffer_, __buffer_ + 
this->__size());
+return std::back_inserter(*__container_);
+  }
+
+private:
+  typename __back_insert_iterator_container<_OutIt>::type* __container_;
+
+  // This class uses a fixed size buffer and appends the elements in
+  // __buffer_size chunks. An alternative would be to use an allocating buffer
+  // and append the output in one write operation. Benchmarking showed no
+  // performance difference.
+  static constexpr size_t __buffer_size = 256;
+  _CharT __buffer_[__buffer_size];
+
+  _LIBCPP_HIDE_FROM_ABI void __prepare_write() {
+__container_->insert(__container_->end(), __buffer_, __buffer_ + 
this->__size());
+this->__buffer_flused();

ldionne wrote:

```suggestion
this->__buffer_flushed();
```

https://github.com/llvm/llvm-project/pull/101823
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


  1   2   3   4   5   6   >