[libcxx-commits] [libcxx] [libc++][format] P2637R3: Member `visit` (PR #76449)

via libcxx-commits libcxx-commits at lists.llvm.org
Wed Dec 27 08:30:34 PST 2023


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libcxx

Author: Hristo Hristov (H-G-Hristov)

<details>
<summary>Changes</summary>

Implements parts of: https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2023/p2637r3.html (https://eel.is/c++draft/variant.visit)

Adds member `visit` tests and (NFC) refactors + reformats the non-member `visit` tests to accomodate the member `visit` additions for consistency.

---

Patch is 33.84 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/76449.diff


8 Files Affected:

- (modified) libcxx/docs/Status/Cxx2cPapers.csv (+1-1) 
- (modified) libcxx/include/__config (+6) 
- (modified) libcxx/include/__format/format_arg.h (+66-2) 
- (modified) libcxx/include/format (+1-1) 
- (renamed) libcxx/test/std/utilities/format/format.arguments/format.arg/visit.pass.cpp (+68-60) 
- (added) libcxx/test/std/utilities/format/format.arguments/format.arg/visit.return_type.pass.cpp (+326) 
- (added) libcxx/test/std/utilities/format/format.arguments/format.arg/visit_format_arg.deprecated.verify.cpp (+64) 
- (modified) libcxx/utils/generate_feature_test_macro_components.py (+1) 


``````````diff
diff --git a/libcxx/docs/Status/Cxx2cPapers.csv b/libcxx/docs/Status/Cxx2cPapers.csv
index ff83648aa76830..b33543aad8b945 100644
--- a/libcxx/docs/Status/Cxx2cPapers.csv
+++ b/libcxx/docs/Status/Cxx2cPapers.csv
@@ -17,7 +17,7 @@
 "`P0792R14 <https://wg21.link/P0792R14>`__","LWG","``function_ref``: a type-erased callable reference","Varna June 2023","","",""
 "`P2874R2 <https://wg21.link/P2874R2>`__","LWG","Mandating Annex D Require No More","Varna June 2023","","",""
 "`P2757R3 <https://wg21.link/P2757R3>`__","LWG","Type-checking format args","Varna June 2023","","","|format|"
-"`P2637R3 <https://wg21.link/P2637R3>`__","LWG","Member ``visit``","Varna June 2023","","","|format|"
+"`P2637R3 <https://wg21.link/P2637R3>`__","LWG","Member ``visit``","Varna June 2023","|Complete|","18.0",""
 "`P2641R4 <https://wg21.link/P2641R4>`__","CWG, LWG","Checking if a ``union`` alternative is active","Varna June 2023","","",""
 "`P1759R6 <https://wg21.link/P1759R6>`__","LWG","Native handles and file streams","Varna June 2023","","",""
 "`P2697R1 <https://wg21.link/P2697R1>`__","LWG","Interfacing ``bitset`` with ``string_view``","Varna June 2023","|Complete|","18.0",""
diff --git a/libcxx/include/__config b/libcxx/include/__config
index adff13e714cb64..ee85b73c2e4cfe 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -956,6 +956,12 @@ typedef __char32_t char32_t;
 #    define _LIBCPP_DEPRECATED_IN_CXX23
 #  endif
 
+#  if _LIBCPP_STD_VER >= 26
+#    define _LIBCPP_DEPRECATED_IN_CXX26 _LIBCPP_DEPRECATED
+#  else
+#    define _LIBCPP_DEPRECATED_IN_CXX26
+#  endif
+
 #  if !defined(_LIBCPP_HAS_NO_CHAR8_T)
 #    define _LIBCPP_DEPRECATED_WITH_CHAR8_T _LIBCPP_DEPRECATED
 #  else
diff --git a/libcxx/include/__format/format_arg.h b/libcxx/include/__format/format_arg.h
index 280c9108241754..bc9e13eb9029a5 100644
--- a/libcxx/include/__format/format_arg.h
+++ b/libcxx/include/__format/format_arg.h
@@ -93,7 +93,7 @@ _LIBCPP_HIDE_FROM_ABI constexpr __arg_t __get_packed_type(uint64_t __types, size
 
 } // namespace __format
 
-// This function is not user obervable, so it can directly use the non-standard
+// This function is not user observable, so it can directly use the non-standard
 // types of the "variant". See __arg_t for more details.
 template <class _Visitor, class _Context>
 _LIBCPP_HIDE_FROM_ABI decltype(auto) __visit_format_arg(_Visitor&& __vis, basic_format_arg<_Context> __arg) {
@@ -144,6 +144,57 @@ _LIBCPP_HIDE_FROM_ABI decltype(auto) __visit_format_arg(_Visitor&& __vis, basic_
   __libcpp_unreachable();
 }
 
+#  if _LIBCPP_STD_VER >= 26
+template <class _Rp, class _Visitor, class _Context>
+_LIBCPP_HIDE_FROM_ABI _Rp __visit_format_arg(_Visitor&& __vis, basic_format_arg<_Context> __arg) {
+  switch (__arg.__type_) {
+  case __format::__arg_t::__none:
+    return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__monostate_);
+  case __format::__arg_t::__boolean:
+    return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__boolean_);
+  case __format::__arg_t::__char_type:
+    return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__char_type_);
+  case __format::__arg_t::__int:
+    return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__int_);
+  case __format::__arg_t::__long_long:
+    return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__long_long_);
+  case __format::__arg_t::__i128:
+#    ifndef _LIBCPP_HAS_NO_INT128
+    return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__i128_);
+#    else
+    __libcpp_unreachable();
+#    endif
+  case __format::__arg_t::__unsigned:
+    return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__unsigned_);
+  case __format::__arg_t::__unsigned_long_long:
+    return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__unsigned_long_long_);
+  case __format::__arg_t::__u128:
+#    ifndef _LIBCPP_HAS_NO_INT128
+    return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__u128_);
+#    else
+    __libcpp_unreachable();
+#    endif
+  case __format::__arg_t::__float:
+    return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__float_);
+  case __format::__arg_t::__double:
+    return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__double_);
+  case __format::__arg_t::__long_double:
+    return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__long_double_);
+  case __format::__arg_t::__const_char_type_ptr:
+    return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__const_char_type_ptr_);
+  case __format::__arg_t::__string_view:
+    return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__string_view_);
+  case __format::__arg_t::__ptr:
+    return std::invoke_r<_Rp>(std::forward<_Visitor>(__vis), __arg.__value_.__ptr_);
+  case __format::__arg_t::__handle:
+    return std::invoke_r<_Rp>(
+        std::forward<_Visitor>(__vis), typename basic_format_arg<_Context>::handle{__arg.__value_.__handle_});
+  }
+
+  __libcpp_unreachable();
+}
+#  endif
+
 /// Contains the values used in basic_format_arg.
 ///
 /// This is a separate type so it's possible to store the values and types in
@@ -227,6 +278,18 @@ class _LIBCPP_TEMPLATE_VIS basic_format_arg {
 
   _LIBCPP_HIDE_FROM_ABI explicit operator bool() const noexcept { return __type_ != __format::__arg_t::__none; }
 
+#  if _LIBCPP_STD_VER >= 26
+  template <class _Visitor>
+  _LIBCPP_HIDE_FROM_ABI decltype(auto) visit(this basic_format_arg __arg, _Visitor&& __vis) {
+    return std::__visit_format_arg(std::forward<_Visitor>(__vis), __arg);
+  }
+
+  template <class _Rp, class _Visitor>
+  _LIBCPP_HIDE_FROM_ABI _Rp visit(this basic_format_arg __arg, _Visitor&& __vis) {
+    return std::__visit_format_arg<_Rp>(std::forward<_Visitor>(__vis), __arg);
+  }
+#  endif
+
 private:
   using char_type = typename _Context::char_type;
 
@@ -267,7 +330,8 @@ class _LIBCPP_TEMPLATE_VIS basic_format_arg<_Context>::handle {
 // This function is user facing, so it must wrap the non-standard types of
 // the "variant" in a handle to stay conforming. See __arg_t for more details.
 template <class _Visitor, class _Context>
-_LIBCPP_HIDE_FROM_ABI decltype(auto) visit_format_arg(_Visitor&& __vis, basic_format_arg<_Context> __arg) {
+_LIBCPP_DEPRECATED_IN_CXX26 _LIBCPP_HIDE_FROM_ABI decltype(auto)
+visit_format_arg(_Visitor&& __vis, basic_format_arg<_Context> __arg) {
   switch (__arg.__type_) {
 #  ifndef _LIBCPP_HAS_NO_INT128
   case __format::__arg_t::__i128: {
diff --git a/libcxx/include/format b/libcxx/include/format
index ab9b336d0cdabe..e1566fb4636a7b 100644
--- a/libcxx/include/format
+++ b/libcxx/include/format
@@ -170,7 +170,7 @@ namespace std {
   template<class Context> class basic_format_arg;
 
   template<class Visitor, class Context>
-    see below visit_format_arg(Visitor&& vis, basic_format_arg<Context> arg);
+    see below visit_format_arg(Visitor&& vis, basic_format_arg<Context> arg); // deprecated in C++26
 
   // [format.arg.store], class template format-arg-store
   template<class Context, class... Args> struct format-arg-store;      // exposition only
diff --git a/libcxx/test/std/utilities/format/format.arguments/format.arg/visit_format_arg.pass.cpp b/libcxx/test/std/utilities/format/format.arguments/format.arg/visit.pass.cpp
similarity index 77%
rename from libcxx/test/std/utilities/format/format.arguments/format.arg/visit_format_arg.pass.cpp
rename to libcxx/test/std/utilities/format/format.arguments/format.arg/visit.pass.cpp
index 3ddf2d0ff732a7..c3b1609c159c8d 100644
--- a/libcxx/test/std/utilities/format/format.arguments/format.arg/visit_format_arg.pass.cpp
+++ b/libcxx/test/std/utilities/format/format.arguments/format.arg/visit.pass.cpp
@@ -10,18 +10,26 @@
 
 // <format>
 
+// class basic_format_arg;
+
+// template<class Visitor>
+//   decltype(auto) visit(this basic_format_arg arg, Visitor&& vis);  // since C++26
+
 // template<class Visitor, class Context>
-//   see below visit_format_arg(Visitor&& vis, basic_format_arg<Context> arg);
+//   see below visit_format_arg(Visitor&& vis, basic_format_arg<Context> arg); // deprecated in C++26
 
 #include <algorithm>
-#include <format>
 #include <cassert>
+#include <format>
 #include <type_traits>
 
 #include "constexpr_char_traits.h"
-#include "test_macros.h"
 #include "make_string.h"
 #include "min_allocator.h"
+#include "test_macros.h"
+
+// Deprecated `std::visit_format_arg` should be tested in C++26 or newer.
+TEST_CLANG_DIAGNOSTIC_IGNORED("-Wdeprecated-declarations")
 
 template <class Context, class To, class From>
 void test(From value) {
@@ -31,20 +39,41 @@ void test(From value) {
   LIBCPP_ASSERT(format_args.__size() == 1);
   assert(format_args.get(0));
 
-  auto result = std::visit_format_arg(
-      [v = To(value)](auto a) -> To {
-        if constexpr (std::is_same_v<To, decltype(a)>) {
-          assert(v == a);
-          return a;
-        } else {
-          assert(false);
-          return {};
-        }
-      },
-      format_args.get(0));
+#if _LIBCPP_STD_VER >= 26
+  // member
+  {
+    auto result = format_args.get(0).visit([v = To(value)](auto a) -> To {
+      if constexpr (std::is_same_v<To, decltype(a)>) {
+        assert(v == a);
+        return a;
+      } else {
+        assert(false);
+        return {};
+      }
+    });
+
+    using ct = std::common_type_t<From, To>;
+    assert(static_cast<ct>(result) == static_cast<ct>(value));
+  }
+#endif
 
-  using ct = std::common_type_t<From, To>;
-  assert(static_cast<ct>(result) == static_cast<ct>(value));
+  // non-member
+  {
+    auto result = std::visit_format_arg(
+        [v = To(value)](auto a) -> To {
+          if constexpr (std::is_same_v<To, decltype(a)>) {
+            assert(v == a);
+            return a;
+          } else {
+            assert(false);
+            return {};
+          }
+        },
+        format_args.get(0));
+
+    using ct = std::common_type_t<From, To>;
+    assert(static_cast<ct>(result) == static_cast<ct>(value));
+  }
 }
 
 // Some types, as an extension, are stored in the variant. The Standard
@@ -75,8 +104,8 @@ void test_string_view(From value) {
   assert(format_args.get(0));
 
   using CharT = typename Context::char_type;
-  using To = std::basic_string_view<CharT>;
-  using V = std::basic_string<CharT>;
+  using To    = std::basic_string_view<CharT>;
+  using V     = std::basic_string<CharT>;
   auto result = std::visit_format_arg(
       [v = V(value.begin(), value.end())](auto a) -> To {
         if constexpr (std::is_same_v<To, decltype(a)>) {
@@ -170,8 +199,7 @@ void test() {
   test<Context, int, int>(std::numeric_limits<short>::max());
   test<Context, int, int>(std::numeric_limits<int>::max());
 
-  using LongToType =
-      std::conditional_t<sizeof(long) == sizeof(int), int, long long>;
+  using LongToType = std::conditional_t<sizeof(long) == sizeof(int), int, long long>;
 
   test<Context, LongToType, long>(std::numeric_limits<long>::min());
   test<Context, LongToType, long>(std::numeric_limits<int>::min());
@@ -202,14 +230,11 @@ void test() {
   // Test unsigned integer types.
 
   test<Context, unsigned, unsigned char>(0);
-  test<Context, unsigned, unsigned char>(
-      std::numeric_limits<unsigned char>::max());
+  test<Context, unsigned, unsigned char>(std::numeric_limits<unsigned char>::max());
 
   test<Context, unsigned, unsigned short>(0);
-  test<Context, unsigned, unsigned short>(
-      std::numeric_limits<unsigned char>::max());
-  test<Context, unsigned, unsigned short>(
-      std::numeric_limits<unsigned short>::max());
+  test<Context, unsigned, unsigned short>(std::numeric_limits<unsigned char>::max());
+  test<Context, unsigned, unsigned short>(std::numeric_limits<unsigned short>::max());
 
   test<Context, unsigned, unsigned>(0);
   test<Context, unsigned, unsigned>(std::numeric_limits<unsigned char>::max());
@@ -217,30 +242,20 @@ void test() {
   test<Context, unsigned, unsigned>(std::numeric_limits<unsigned>::max());
 
   using UnsignedLongToType =
-      std::conditional_t<sizeof(unsigned long) == sizeof(unsigned), unsigned,
-                         unsigned long long>;
+      std::conditional_t<sizeof(unsigned long) == sizeof(unsigned), unsigned, unsigned long long>;
 
   test<Context, UnsignedLongToType, unsigned long>(0);
-  test<Context, UnsignedLongToType, unsigned long>(
-      std::numeric_limits<unsigned char>::max());
-  test<Context, UnsignedLongToType, unsigned long>(
-      std::numeric_limits<unsigned short>::max());
-  test<Context, UnsignedLongToType, unsigned long>(
-      std::numeric_limits<unsigned>::max());
-  test<Context, UnsignedLongToType, unsigned long>(
-      std::numeric_limits<unsigned long>::max());
+  test<Context, UnsignedLongToType, unsigned long>(std::numeric_limits<unsigned char>::max());
+  test<Context, UnsignedLongToType, unsigned long>(std::numeric_limits<unsigned short>::max());
+  test<Context, UnsignedLongToType, unsigned long>(std::numeric_limits<unsigned>::max());
+  test<Context, UnsignedLongToType, unsigned long>(std::numeric_limits<unsigned long>::max());
 
   test<Context, unsigned long long, unsigned long long>(0);
-  test<Context, unsigned long long, unsigned long long>(
-      std::numeric_limits<unsigned char>::max());
-  test<Context, unsigned long long, unsigned long long>(
-      std::numeric_limits<unsigned short>::max());
-  test<Context, unsigned long long, unsigned long long>(
-      std::numeric_limits<unsigned>::max());
-  test<Context, unsigned long long, unsigned long long>(
-      std::numeric_limits<unsigned long>::max());
-  test<Context, unsigned long long, unsigned long long>(
-      std::numeric_limits<unsigned long long>::max());
+  test<Context, unsigned long long, unsigned long long>(std::numeric_limits<unsigned char>::max());
+  test<Context, unsigned long long, unsigned long long>(std::numeric_limits<unsigned short>::max());
+  test<Context, unsigned long long, unsigned long long>(std::numeric_limits<unsigned>::max());
+  test<Context, unsigned long long, unsigned long long>(std::numeric_limits<unsigned long>::max());
+  test<Context, unsigned long long, unsigned long long>(std::numeric_limits<unsigned long long>::max());
 
 #ifndef TEST_HAS_NO_INT128
   test_handle<Context, __uint128_t>(0);
@@ -262,16 +277,12 @@ void test() {
   test<Context, double, double>(std::numeric_limits<double>::min());
   test<Context, double, double>(std::numeric_limits<double>::max());
 
-  test<Context, long double, long double>(
-      -std::numeric_limits<long double>::max());
-  test<Context, long double, long double>(
-      -std::numeric_limits<long double>::min());
+  test<Context, long double, long double>(-std::numeric_limits<long double>::max());
+  test<Context, long double, long double>(-std::numeric_limits<long double>::min());
   test<Context, long double, long double>(-0.0);
   test<Context, long double, long double>(0.0);
-  test<Context, long double, long double>(
-      std::numeric_limits<long double>::min());
-  test<Context, long double, long double>(
-      std::numeric_limits<long double>::max());
+  test<Context, long double, long double>(std::numeric_limits<long double>::min());
+  test<Context, long double, long double>(std::numeric_limits<long double>::max());
 
   // Test const CharT pointer types.
 
@@ -307,8 +318,7 @@ void test() {
   }
 
   {
-    using From = std::basic_string<CharT, constexpr_char_traits<CharT>,
-                                   std::allocator<CharT>>;
+    using From = std::basic_string<CharT, constexpr_char_traits<CharT>, std::allocator<CharT>>;
 
     test_string_view<Context>(From());
     test_string_view<Context>(From(empty.c_str()));
@@ -316,8 +326,7 @@ void test() {
   }
 
   {
-    using From =
-        std::basic_string<CharT, std::char_traits<CharT>, min_allocator<CharT>>;
+    using From = std::basic_string<CharT, std::char_traits<CharT>, min_allocator<CharT>>;
 
     test_string_view<Context>(From());
     test_string_view<Context>(From(empty.c_str()));
@@ -325,8 +334,7 @@ void test() {
   }
 
   {
-    using From = std::basic_string<CharT, constexpr_char_traits<CharT>,
-                                   min_allocator<CharT>>;
+    using From = std::basic_string<CharT, constexpr_char_traits<CharT>, min_allocator<CharT>>;
 
     test_string_view<Context>(From());
     test_string_view<Context>(From(empty.c_str()));
diff --git a/libcxx/test/std/utilities/format/format.arguments/format.arg/visit.return_type.pass.cpp b/libcxx/test/std/utilities/format/format.arguments/format.arg/visit.return_type.pass.cpp
new file mode 100644
index 00000000000000..470df35ef97196
--- /dev/null
+++ b/libcxx/test/std/utilities/format/format.arguments/format.arg/visit.return_type.pass.cpp
@@ -0,0 +1,326 @@
+//===----------------------------------------------------------------------===//
+// 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, c++20, c++23
+// UNSUPPORTED: GCC-ALWAYS_INLINE-FIXME
+
+// <format>
+
+// class basic_format_arg;
+
+// template<class R, class Visitor>
+//   R visit(this basic_format_arg arg, Visitor&& vis);
+
+#include <algorithm>
+#include <cassert>
+#include <format>
+#include <type_traits>
+
+#include "constexpr_char_traits.h"
+#include "make_string.h"
+#include "min_allocator.h"
+#include "test_macros.h"
+
+// The expected result type shouldn't matter,therefore use a hardcoded value for simplicity.
+using ExpectedResultType = bool;
+constexpr ExpectedResultType visited{true};
+
+template <class ExpectedR>
+ExpectedR make_expected_result() {
+  if constexpr (std::is_same_v<ExpectedR, bool>) {
+    return true;
+  } else if constexpr (std::is_same_v<ExpectedR, long>) {
+    return 192812079084L;
+  } else {
+    return "visited";
+  }
+}
+
+template <class Context, class To, class ExpectedR, class From>
+void test(From value, const ExpectedR& expectedValue) {
+  auto store = std::make_format_args<Context>(value);
+  std::basic_format_args<Context> format_args{store};
+
+  LIBCPP_ASSERT(format_args.__size() == 1);
+  assert(format_args.get(0));
+
+  // member
+  {
+    std::same_as<ExpectedR> decltype(auto) result =
+        format_args.get(0).template visit<ExpectedR>([v = To(value)](auto a) -> ExpectedR {
+          if constexpr (std::is_same_v<To, decltype(a)>) {
+            assert(v == a);
+            return make_expected_result<ExpectedR>();
+          } else {
+            assert(false);
+            return {};
+          }
+        });
+
+    assert(result == expectedValue);
+  }
+}
+
+// Some types, as an extension, are stored in the variant. The Standard
+// requires them to be observed as a handle.
+template <class Context, class T, class ExpectedR>
+void test_handle(T value, ExpectedR expectedValue) {
+  auto store = std::make_format_args<Context>(value);
+  std::basic_format_args<Context> format_args{store};
+
+  LIBCPP_ASSERT(format_args.__size() == 1);
+  assert(format_args.get(0));
+
+  std::same_as<ExpectedR> decltype(auto) result = format_args.get(0).template visit<ExpectedR>([](auto a) -> ExpectedR {
+    // TODO: This check fails
+    (void)a;
+    // assert((std::is_same_v<decltype(a), typename std::basic_format_arg<Context>::handle>));
+
+    return make_expected_result<ExpectedR>();
+  });
+
+  assert(result == expectedValue);
+}
+
+// Test specific for string and string_view.
+//
+// Since both result in a string_view there's no need to pass this as a
+// template argument.
+template <class Context, class ExpectedR, class From>
+void test_string_view(From value, ExpectedR expectedValue) {
+  auto store = std::make_format_args<Context>(value);
+  std::basic_format_args<Context> format_args{store};
+
+  LIBCPP_ASSERT(format_args.__size() == 1);
+  assert(format_args.get(0));
+
+  using CharT = typename Context::char_type;
+  using To    = std::basic_string_view<CharT>;
+  using V     = std::basic_string<CharT>;
+
+  std::same_as<ExpectedR> decltype(auto) result =
+      format_args.get(0).template visit<ExpectedR>([v = V(value.begin(), value.end())](auto a) -> ExpectedR {
+        if constexpr (std::is_same_v<To, decltype(a)>) {...
[truncated]

``````````

</details>


https://github.com/llvm/llvm-project/pull/76449


More information about the libcxx-commits mailing list