[libcxx-commits] [libcxx] 8893022 - [libc++][format][3/6] Adds a __container_buffer.

Mark de Wever via libcxx-commits libcxx-commits at lists.llvm.org
Sat Apr 9 00:35:53 PDT 2022


Author: Mark de Wever
Date: 2022-04-09T09:35:48+02:00
New Revision: 889302292bf6607a6606a771bb2bf0ce919e80bc

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

LOG: [libc++][format][3/6] Adds a __container_buffer.

Instead of writing every character directly into the container by using
a `back_insert_iterator` the data is buffered in an `array`. This buffer
is then inserted to the container by calling its `insert` member function.

Since there's no guarantee every container's `insert` behaves properly
containers need to opt-in to this behaviour. The appropriate standard
containers opt-in to this behaviour.

This change improves the performance of the format functions that use a
`back_insert_iterator`.

Depends on D110495

Reviewed By: ldionne, vitaut, #libc

Differential Revision: https://reviews.llvm.org/D110497

Added: 
    libcxx/include/__format/enable_insertable.h
    libcxx/test/libcxx/utilities/format/enable_insertable.compile.pass.cpp

Modified: 
    libcxx/include/CMakeLists.txt
    libcxx/include/__format/buffer.h
    libcxx/include/deque
    libcxx/include/format
    libcxx/include/list
    libcxx/include/module.modulemap
    libcxx/include/string
    libcxx/include/vector
    libcxx/test/libcxx/private_headers.verify.cpp

Removed: 
    


################################################################################
diff  --git a/libcxx/include/CMakeLists.txt b/libcxx/include/CMakeLists.txt
index 5069d06d894b4..ebe95c09eed29 100644
--- a/libcxx/include/CMakeLists.txt
+++ b/libcxx/include/CMakeLists.txt
@@ -200,6 +200,7 @@ set(files
   __filesystem/space_info.h
   __filesystem/u8path.h
   __format/buffer.h
+  __format/enable_insertable.h
   __format/format_arg.h
   __format/format_args.h
   __format/format_context.h

diff  --git a/libcxx/include/__format/buffer.h b/libcxx/include/__format/buffer.h
index 32513d0350a63..584345c98fdbd 100644
--- a/libcxx/include/__format/buffer.h
+++ b/libcxx/include/__format/buffer.h
@@ -13,6 +13,7 @@
 #include <__algorithm/copy_n.h>
 #include <__algorithm/unwrap_iter.h>
 #include <__config>
+#include <__format/enable_insertable.h>
 #include <__format/formatter.h> // for __char_type TODO FMT Move the concept?
 #include <__iterator/back_insert_iterator.h>
 #include <__iterator/concepts.h>
@@ -152,13 +153,58 @@ class _LIBCPP_TEMPLATE_VIS __writer_iterator {
   _OutIt __out_it_;
 };
 
+/// Concept to see whether a \a _Container is insertable.
+///
+/// The concept is used to validate whether multiple calls to a
+/// \ref back_insert_iterator can be replace by a call to \c _Container::insert.
+///
+/// \note a \a _Container needs to opt-in to the concept by specializing
+/// \ref __enable_insertable.
+template <class _Container>
+concept __insertable =
+    __enable_insertable<_Container> && __formatter::__char_type<typename _Container::value_type> &&
+    requires(_Container& __t, add_pointer_t<typename _Container::value_type> __first,
+             add_pointer_t<typename _Container::value_type> __last) { __t.insert(__t.end(), __first, __last); };
+
+/// Extract the container type of a \ref back_insert_iterator.
+template <class _It>
+struct _LIBCPP_TEMPLATE_VIS __back_insert_iterator_container {
+  using type = void;
+};
+
+template <__insertable _Container>
+struct _LIBCPP_TEMPLATE_VIS __back_insert_iterator_container<back_insert_iterator<_Container>> {
+  using type = _Container;
+};
+
+/// Write policy for inserting the buffer in a container.
+template <class _Container>
+class _LIBCPP_TEMPLATE_VIS __writer_container {
+public:
+  using _CharT = typename _Container::value_type;
+
+  _LIBCPP_HIDE_FROM_ABI explicit __writer_container(back_insert_iterator<_Container> __out_it)
+      : __container_{__out_it.__get_container()} {}
+
+  _LIBCPP_HIDE_FROM_ABI auto out() { return back_inserter(*__container_); }
+
+  _LIBCPP_HIDE_FROM_ABI void flush(_CharT* __ptr, size_t __size) {
+    __container_->insert(__container_->end(), __ptr, __ptr + __size);
+  }
+
+private:
+  _Container* __container_;
+};
+
 /// Selects the type of the writer used for the output iterator.
 template <class _OutIt, class _CharT>
 class _LIBCPP_TEMPLATE_VIS __writer_selector {
+  using _Container = typename __back_insert_iterator_container<_OutIt>::type;
+
 public:
-  using type = conditional_t<__enable_direct_output<_OutIt, _CharT>,
-                             __writer_direct<_OutIt, _CharT>,
-                             __writer_iterator<_OutIt, _CharT>>;
+  using type = conditional_t<!same_as<_Container, void>, __writer_container<_Container>,
+                             conditional_t<__enable_direct_output<_OutIt, _CharT>, __writer_direct<_OutIt, _CharT>,
+                                           __writer_iterator<_OutIt, _CharT>>>;
 };
 
 /// The generic formatting buffer.

diff  --git a/libcxx/include/__format/enable_insertable.h b/libcxx/include/__format/enable_insertable.h
new file mode 100644
index 0000000000000..71b4252930ded
--- /dev/null
+++ b/libcxx/include/__format/enable_insertable.h
@@ -0,0 +1,35 @@
+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+// 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___FORMAT_ENABLE_INSERTABLE_H
+#define _LIBCPP___FORMAT_ENABLE_INSERTABLE_H
+
+#include <__config>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#  pragma GCC system_header
+#endif
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+#if _LIBCPP_STD_VER > 17
+
+namespace __format {
+
+/// Opt-in to enable \ref __insertable for a \a _Container.
+template <class _Container>
+inline constexpr bool __enable_insertable = false;
+
+} // namespace __format
+
+#endif //_LIBCPP_STD_VER > 17
+
+_LIBCPP_END_NAMESPACE_STD
+
+#endif // _LIBCPP___FORMAT_ENABLE_INSERTABLE_H

diff  --git a/libcxx/include/deque b/libcxx/include/deque
index 4b78d77e6c7d5..d782963c18fec 100644
--- a/libcxx/include/deque
+++ b/libcxx/include/deque
@@ -171,6 +171,7 @@ template <class T, class Allocator, class Predicate>
 #include <__algorithm/unwrap_iter.h>
 #include <__assert> // all public C++ headers provide the assertion handler
 #include <__config>
+#include <__format/enable_insertable.h>
 #include <__iterator/iterator_traits.h>
 #include <__split_buffer>
 #include <__utility/forward.h>
@@ -3029,8 +3030,15 @@ erase_if(deque<_Tp, _Allocator>& __c, _Predicate __pred) {
   __c.erase(_VSTD::remove_if(__c.begin(), __c.end(), __pred), __c.end());
   return __old_size - __c.size();
 }
+
+template <>
+inline constexpr bool __format::__enable_insertable<std::deque<char>> = true;
+#ifndef _LIBCPP_HAS_NO_WIDE_CHARACTERS
+template <>
+inline constexpr bool __format::__enable_insertable<std::deque<wchar_t>> = true;
 #endif
 
+#endif // _LIBCPP_STD_VER > 17
 
 _LIBCPP_END_NAMESPACE_STD
 

diff  --git a/libcxx/include/format b/libcxx/include/format
index 6cefab4948d22..4ae98ce86c7fe 100644
--- a/libcxx/include/format
+++ b/libcxx/include/format
@@ -127,6 +127,7 @@ namespace std {
 #include <__config>
 #include <__debug>
 #include <__format/buffer.h>
+#include <__format/enable_insertable.h>
 #include <__format/format_arg.h>
 #include <__format/format_args.h>
 #include <__format/format_context.h>

diff  --git a/libcxx/include/list b/libcxx/include/list
index c3337d4588350..aa8b2c0805bfb 100644
--- a/libcxx/include/list
+++ b/libcxx/include/list
@@ -187,6 +187,7 @@ template <class T, class Allocator, class Predicate>
 #include <__assert> // all public C++ headers provide the assertion handler
 #include <__config>
 #include <__debug>
+#include <__format/enable_insertable.h>
 #include <__utility/forward.h>
 #include <__utility/move.h>
 #include <__utility/swap.h>
@@ -2420,8 +2421,16 @@ inline _LIBCPP_INLINE_VISIBILITY typename list<_Tp, _Allocator>::size_type
 erase(list<_Tp, _Allocator>& __c, const _Up& __v) {
   return _VSTD::erase_if(__c, [&](auto& __elem) { return __elem == __v; });
 }
+
+template <>
+inline constexpr bool __format::__enable_insertable<std::list<char>> = true;
+#ifndef _LIBCPP_HAS_NO_WIDE_CHARACTERS
+template <>
+inline constexpr bool __format::__enable_insertable<std::list<wchar_t>> = true;
 #endif
 
+#endif // _LIBCPP_STD_VER > 17
+
 _LIBCPP_END_NAMESPACE_STD
 
 _LIBCPP_POP_MACROS

diff  --git a/libcxx/include/module.modulemap b/libcxx/include/module.modulemap
index 0f07fd1577ea2..ece960fdf2e67 100644
--- a/libcxx/include/module.modulemap
+++ b/libcxx/include/module.modulemap
@@ -528,6 +528,7 @@ module std [system] {
 
     module __format {
       module buffer                   { private header "__format/buffer.h" }
+      module enable_insertable        { private header "__format/enable_insertable.h" }
       module format_arg               { private header "__format/format_arg.h" }
       module format_args              { private header "__format/format_args.h" }
       module format_context {

diff  --git a/libcxx/include/string b/libcxx/include/string
index a0056745f5490..88b18be41cf66 100644
--- a/libcxx/include/string
+++ b/libcxx/include/string
@@ -525,6 +525,7 @@ basic_string<char32_t> operator "" s( const char32_t *str, size_t len ); // C++1
 #include <__assert> // all public C++ headers provide the assertion handler
 #include <__config>
 #include <__debug>
+#include <__format/enable_insertable.h>
 #include <__ios/fpos.h>
 #include <__iterator/wrap_iter.h>
 #include <__utility/auto_cast.h>
@@ -4471,6 +4472,16 @@ inline namespace literals
     }
   } // namespace string_literals
 } // namespace literals
+
+#if _LIBCPP_STD_VER > 17
+template <>
+inline constexpr bool __format::__enable_insertable<std::basic_string<char>> = true;
+#ifndef _LIBCPP_HAS_NO_WIDE_CHARACTERS
+template <>
+inline constexpr bool __format::__enable_insertable<std::basic_string<wchar_t>> = true;
+#endif
+#endif
+
 #endif
 
 _LIBCPP_END_NAMESPACE_STD

diff  --git a/libcxx/include/vector b/libcxx/include/vector
index 064a99d2f5de0..3ad43ee2a9de6 100644
--- a/libcxx/include/vector
+++ b/libcxx/include/vector
@@ -283,6 +283,7 @@ erase_if(vector<T, Allocator>& c, Predicate pred);    // C++20
 #include <__bit_reference>
 #include <__config>
 #include <__debug>
+#include <__format/enable_insertable.h>
 #include <__functional/hash.h>
 #include <__iterator/iterator_traits.h>
 #include <__iterator/wrap_iter.h>
@@ -3304,8 +3305,16 @@ erase_if(vector<_Tp, _Allocator>& __c, _Predicate __pred) {
   __c.erase(_VSTD::remove_if(__c.begin(), __c.end(), __pred), __c.end());
   return __old_size - __c.size();
 }
+
+template <>
+inline constexpr bool __format::__enable_insertable<std::vector<char>> = true;
+#ifndef _LIBCPP_HAS_NO_WIDE_CHARACTERS
+template <>
+inline constexpr bool __format::__enable_insertable<std::vector<wchar_t>> = true;
 #endif
 
+#endif // _LIBCPP_STD_VER > 17
+
 _LIBCPP_END_NAMESPACE_STD
 
 _LIBCPP_POP_MACROS

diff  --git a/libcxx/test/libcxx/private_headers.verify.cpp b/libcxx/test/libcxx/private_headers.verify.cpp
index f1173ff2ca51a..06c5988ba0464 100644
--- a/libcxx/test/libcxx/private_headers.verify.cpp
+++ b/libcxx/test/libcxx/private_headers.verify.cpp
@@ -232,6 +232,7 @@ END-SCRIPT
 #include <__filesystem/space_info.h> // expected-error@*:* {{use of private header from outside its module: '__filesystem/space_info.h'}}
 #include <__filesystem/u8path.h> // expected-error@*:* {{use of private header from outside its module: '__filesystem/u8path.h'}}
 #include <__format/buffer.h> // expected-error@*:* {{use of private header from outside its module: '__format/buffer.h'}}
+#include <__format/enable_insertable.h> // expected-error@*:* {{use of private header from outside its module: '__format/enable_insertable.h'}}
 #include <__format/format_arg.h> // expected-error@*:* {{use of private header from outside its module: '__format/format_arg.h'}}
 #include <__format/format_args.h> // expected-error@*:* {{use of private header from outside its module: '__format/format_args.h'}}
 #include <__format/format_context.h> // expected-error@*:* {{use of private header from outside its module: '__format/format_context.h'}}

diff  --git a/libcxx/test/libcxx/utilities/format/enable_insertable.compile.pass.cpp b/libcxx/test/libcxx/utilities/format/enable_insertable.compile.pass.cpp
new file mode 100644
index 0000000000000..62564034d1c91
--- /dev/null
+++ b/libcxx/test/libcxx/utilities/format/enable_insertable.compile.pass.cpp
@@ -0,0 +1,155 @@
+//===----------------------------------------------------------------------===//
+// 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: libcpp-has-no-incomplete-format
+
+// <format>
+
+#include <format>
+#include <array>
+#include <deque>
+#include <forward_list>
+#include <list>
+#include <map>
+#include <queue>
+#include <set>
+#include <span>
+#include <stack>
+#include <string>
+#include <string_view>
+
+#include "test_macros.h"
+
+template <class CharT>
+struct no_value_type {
+  using iterator = CharT*;
+  iterator end();
+  void insert(iterator, CharT*, CharT*);
+};
+
+template <class CharT>
+struct no_end {
+  using value_type = CharT;
+  using iterator = CharT*;
+  void insert(iterator, CharT*, CharT*);
+};
+
+template <class CharT>
+struct no_insert {
+  using value_type = CharT;
+  using iterator = CharT*;
+  iterator end();
+};
+
+template <class CharT>
+struct no_specialization {
+  using value_type = CharT;
+  using iterator = CharT*;
+  iterator end();
+  void insert(iterator, CharT*, CharT*);
+};
+
+template <class CharT>
+struct valid {
+  using value_type = CharT;
+  using iterator = CharT*;
+  iterator end();
+  void insert(iterator, CharT*, CharT*);
+};
+
+namespace std::__format {
+template <>
+inline constexpr bool __enable_insertable<no_value_type<char>> = true;
+template <>
+inline constexpr bool __enable_insertable<no_end<char>> = true;
+template <>
+inline constexpr bool __enable_insertable<no_insert<char>> = true;
+template <>
+inline constexpr bool __enable_insertable<valid<char>> = true;
+#ifndef TEST_HAS_NO_WIDE_CHARACTERS
+template <>
+inline constexpr bool __enable_insertable<no_value_type<wchar_t>> = true;
+template <>
+inline constexpr bool __enable_insertable<no_end<wchar_t>> = true;
+template <>
+inline constexpr bool __enable_insertable<no_insert<wchar_t>> = true;
+template <>
+inline constexpr bool __enable_insertable<valid<wchar_t>> = true;
+#endif
+} // namespace std::__format
+
+static_assert(!std::__format::__insertable<no_value_type<char>>);
+static_assert(!std::__format::__insertable<no_end<char>>);
+static_assert(!std::__format::__insertable<no_insert<char>>);
+static_assert(!std::__format::__insertable<no_specialization<char>>);
+static_assert(std::__format::__insertable<valid<char>>);
+#ifndef TEST_HAS_NO_WIDE_CHARACTERS
+static_assert(!std::__format::__insertable<no_value_type<wchar_t>>);
+static_assert(!std::__format::__insertable<no_end<wchar_t>>);
+static_assert(!std::__format::__insertable<no_insert<wchar_t>>);
+static_assert(!std::__format::__insertable<no_specialization<wchar_t>>);
+static_assert(std::__format::__insertable<valid<wchar_t>>);
+#endif
+
+namespace std::__format {
+template <>
+inline constexpr bool __enable_insertable<valid<signed char>> = true;
+template <>
+inline constexpr bool __enable_insertable<valid<unsigned char>> = true;
+} // namespace std::__format
+
+static_assert(!std::__format::__insertable<valid<signed char>>);
+static_assert(!std::__format::__insertable<valid<unsigned char>>);
+
+static_assert(std::__format::__insertable<std::string>);
+static_assert(!std::__format::__insertable<std::string_view>);
+#ifndef TEST_HAS_NO_WIDE_CHARACTERS
+static_assert(std::__format::__insertable<std::wstring>);
+static_assert(!std::__format::__insertable<std::wstring_view>);
+#endif
+
+static_assert(!std::__format::__insertable<std::array<char, 1>>);
+#ifndef TEST_HAS_NO_WIDE_CHARACTERS
+static_assert(!std::__format::__insertable<std::array<wchar_t, 1>>);
+#endif
+
+static_assert(std::__format::__insertable<std::vector<char>>);
+static_assert(std::__format::__insertable<std::deque<char>>);
+static_assert(!std::__format::__insertable<std::forward_list<char>>);
+static_assert(std::__format::__insertable<std::list<char>>);
+#ifndef TEST_HAS_NO_WIDE_CHARACTERS
+static_assert(std::__format::__insertable<std::vector<wchar_t>>);
+static_assert(std::__format::__insertable<std::deque<wchar_t>>);
+static_assert(!std::__format::__insertable<std::forward_list<wchar_t>>);
+static_assert(std::__format::__insertable<std::list<wchar_t>>);
+#endif
+
+static_assert(!std::__format::__insertable<std::set<char>>);
+static_assert(!std::__format::__insertable<std::map<char, char>>);
+static_assert(!std::__format::__insertable<std::multiset<char>>);
+static_assert(!std::__format::__insertable<std::multimap<char, char>>);
+#ifndef TEST_HAS_NO_WIDE_CHARACTERS
+static_assert(!std::__format::__insertable<std::set<wchar_t>>);
+static_assert(!std::__format::__insertable<std::map<wchar_t, wchar_t>>);
+static_assert(!std::__format::__insertable<std::multiset<wchar_t>>);
+static_assert(!std::__format::__insertable<std::multimap<wchar_t, wchar_t>>);
+#endif
+
+static_assert(!std::__format::__insertable<std::stack<char>>);
+static_assert(!std::__format::__insertable<std::queue<char>>);
+static_assert(!std::__format::__insertable<std::priority_queue<char>>);
+#ifndef TEST_HAS_NO_WIDE_CHARACTERS
+static_assert(!std::__format::__insertable<std::stack<wchar_t>>);
+static_assert(!std::__format::__insertable<std::queue<wchar_t>>);
+static_assert(!std::__format::__insertable<std::priority_queue<wchar_t>>);
+#endif
+
+static_assert(!std::__format::__insertable<std::span<char>>);
+#ifndef TEST_HAS_NO_WIDE_CHARACTERS
+static_assert(!std::__format::__insertable<std::span<wchar_t>>);
+#endif


        


More information about the libcxx-commits mailing list