Skip to content

[libc++][variant] P2637R3: Member visit (std::variant) #76447

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 43 commits into from
Jan 21, 2024

Conversation

H-G-Hristov
Copy link
Contributor

@H-G-Hristov H-G-Hristov commented Dec 27, 2023

Implements parts of: P2637R3 https://wg21.link/P2637R3 (https://eel.is/c++draft/variant.visit)

Implements:
variant.visit()
variant.visit<R>()

The tests are as close as possible to the non-member function.

To land after: #76268

Copy link

github-actions bot commented Dec 27, 2023

✅ With the latest revision this PR passed the Python code formatter.

@H-G-Hristov H-G-Hristov changed the title [libc++][variant] P2637R3 - Member visit [libc++][variant] P2637R3: Member visit Dec 27, 2023
@Zingam Zingam added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Dec 27, 2023
@llvmbot
Copy link
Member

llvmbot commented Dec 27, 2023

@llvm/pr-subscribers-libcxx

Author: Hristo Hristov (H-G-Hristov)

Changes

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 70.62 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/76447.diff

7 Files Affected:

  • (modified) libcxx/docs/Status/Cxx2c.rst (+2)
  • (modified) libcxx/docs/Status/Cxx2cPapers.csv (+1-1)
  • (modified) libcxx/include/variant (+34)
  • (modified) libcxx/test/std/utilities/variant/variant.visit/robust_against_adl.pass.cpp (+37-16)
  • (modified) libcxx/test/std/utilities/variant/variant.visit/visit.pass.cpp (+428-178)
  • (modified) libcxx/test/std/utilities/variant/variant.visit/visit_return_type.pass.cpp (+545-223)
  • (modified) libcxx/utils/generate_feature_test_macro_components.py (+5-1)
diff --git a/libcxx/docs/Status/Cxx2c.rst b/libcxx/docs/Status/Cxx2c.rst
index a7ebc4662f517c..5c700d2cb0d6d3 100644
--- a/libcxx/docs/Status/Cxx2c.rst
+++ b/libcxx/docs/Status/Cxx2c.rst
@@ -40,6 +40,8 @@ Paper Status
 .. note::
 
    .. [#note-P2510R3] This paper is applied as DR against C++20. (MSVC STL and libstdc++ will do the same.)
+   .. [#note-P2637R3] P2637R3: Implemented `variant` member `visit`
+
 
 .. _issues-status-cxx2c:
 
diff --git a/libcxx/docs/Status/Cxx2cPapers.csv b/libcxx/docs/Status/Cxx2cPapers.csv
index ff83648aa76830..a3214ab2bfe75c 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","|Partial| [#note-P2637R3]","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/variant b/libcxx/include/variant
index 6179b2a1a0ab61..4c9b04942f2779 100644
--- a/libcxx/include/variant
+++ b/libcxx/include/variant
@@ -69,6 +69,12 @@ namespace std {
 
     // 20.7.2.6, swap
     void swap(variant&) noexcept(see below);
+
+    // [variant.visit], visitation
+    template<class Self, class Visitor>
+      constexpr decltype(auto) visit(this Self&&, Visitor&&);
+    template<class R, class Self, class Visitor>
+      constexpr R visit(this Self&&, Visitor&&);
   };
 
   // 20.7.3, variant helper classes
@@ -235,6 +241,7 @@ namespace std {
 #include <__type_traits/void_t.h>
 #include <__utility/declval.h>
 #include <__utility/forward.h>
+#include <__utility/forward_like.h>
 #include <__utility/in_place.h>
 #include <__utility/move.h>
 #include <__utility/swap.h>
@@ -1273,6 +1280,15 @@ public:
     __impl_.__swap(__that.__impl_);
   }
 
+#  if _LIBCPP_STD_VER >= 26
+  // [variant.visit], visitation
+  template <int=0, class _Self, class _Visitor>
+  constexpr decltype(auto) visit(this _Self&& __self, _Visitor&& __visitor);
+
+  template <class _R, class _Self, class _Visitor>
+  constexpr _R visit(this _Self&& __self, _Visitor&& __visitor);
+#  endif
+
 private:
   __variant_detail::__impl<_Types...> __impl_;
 
@@ -1532,6 +1548,24 @@ visit(_Visitor&& __visitor, _Vs&&... __vs) {
 }
 #  endif
 
+#  if _LIBCPP_STD_VER >= 26
+// [variant.visit], visitation
+
+template <class... _Types>
+template <int, class _Self, class _Visitor>
+constexpr decltype(auto) variant<_Types...>::visit(this _Self&& __self, _Visitor&& __visitor) {
+  using _V = _OverrideRef<_Self&&, _CopyConst<remove_reference_t<_Self>, variant>>;
+  return std::visit(std::forward<_Visitor>(__visitor), (_V)__self);
+}
+
+template <class... _Types>
+template <class _Rp, class _Self, class _Visitor>
+constexpr _Rp variant<_Types...>::visit(this _Self&& __self, _Visitor&& __visitor) {
+  using _V = _OverrideRef<_Self&&, _CopyConst<remove_reference_t<_Self>, variant>>;
+  return std::visit<_Rp>(std::forward<_Visitor>(__visitor), (_V)__self);
+}
+#  endif
+
 template <class... _Types>
 _LIBCPP_HIDE_FROM_ABI auto
 swap(variant<_Types...>& __lhs, variant<_Types...>& __rhs) noexcept(noexcept(__lhs.swap(__rhs)))
diff --git a/libcxx/test/std/utilities/variant/variant.visit/robust_against_adl.pass.cpp b/libcxx/test/std/utilities/variant/variant.visit/robust_against_adl.pass.cpp
index 6f17fa32648d41..3bd305a7c62c17 100644
--- a/libcxx/test/std/utilities/variant/variant.visit/robust_against_adl.pass.cpp
+++ b/libcxx/test/std/utilities/variant/variant.visit/robust_against_adl.pass.cpp
@@ -9,6 +9,13 @@
 // UNSUPPORTED: c++03, c++11, c++14
 
 // <variant>
+
+// class variant;
+// template<class Self, class Visitor>
+//   constexpr decltype(auto) visit(this Self&&, Visitor&&); // since C++26
+// template<class R, class Self, class Visitor>
+//   constexpr R visit(this Self&&, Visitor&&);              // since C++26
+
 // template <class Visitor, class... Variants>
 // constexpr see below visit(Visitor&& vis, Variants&&... vars);
 
@@ -17,27 +24,41 @@
 #include "test_macros.h"
 
 struct Incomplete;
-template<class T> struct Holder { T t; };
-
-constexpr bool test(bool do_it)
-{
-    if (do_it) {
-        std::variant<Holder<Incomplete>*, int> v = nullptr;
-        std::visit([](auto){}, v);
-        std::visit([](auto) -> Holder<Incomplete>* { return nullptr; }, v);
+template <class T>
+struct Holder {
+  T t;
+};
+
+constexpr bool test(bool do_it) {
+  if (do_it) {
+    std::variant<Holder<Incomplete>*, int> v = nullptr;
+
+#if _LIBCPP_STD_VER >= 26
+    // member
+    {
+      v.visit([](auto) {});
+      v.visit([](auto) -> Holder<Incomplete>* { return nullptr; });
+      v.visit<void>([](auto) {});
+      v.visit<void*>([](auto) -> Holder<Incomplete>* { return nullptr; });
+    }
+#endif
+    // non-member
+    {
+      std::visit([](auto) {}, v);
+      std::visit([](auto) -> Holder<Incomplete>* { return nullptr; }, v);
 #if TEST_STD_VER > 17
-        std::visit<void>([](auto){}, v);
-        std::visit<void*>([](auto) -> Holder<Incomplete>* { return nullptr; }, v);
+      std::visit<void>([](auto) {}, v);
+      std::visit<void*>([](auto) -> Holder<Incomplete>* { return nullptr; }, v);
 #endif
     }
-    return true;
+  }
+  return true;
 }
 
-int main(int, char**)
-{
-    test(true);
+int main(int, char**) {
+  test(true);
 #if TEST_STD_VER > 17
-    static_assert(test(true));
+  static_assert(test(true));
 #endif
-    return 0;
+  return 0;
 }
diff --git a/libcxx/test/std/utilities/variant/variant.visit/visit.pass.cpp b/libcxx/test/std/utilities/variant/variant.visit/visit.pass.cpp
index 097b784f2bf2ce..8781174ff7d669 100644
--- a/libcxx/test/std/utilities/variant/variant.visit/visit.pass.cpp
+++ b/libcxx/test/std/utilities/variant/variant.visit/visit.pass.cpp
@@ -9,6 +9,11 @@
 // UNSUPPORTED: c++03, c++11, c++14
 
 // <variant>
+
+// class variant;
+// template<class Self, class Visitor>
+//   constexpr decltype(auto) visit(this Self&&, Visitor&&); // since C++26
+
 // template <class Visitor, class... Variants>
 // constexpr see below visit(Visitor&& vis, Variants&&... vars);
 
@@ -25,299 +30,503 @@
 void test_call_operator_forwarding() {
   using Fn = ForwardingCallObject;
   Fn obj{};
-  const Fn &cobj = obj;
+  const Fn& cobj = obj;
+
   { // test call operator forwarding - no variant
-    std::visit(obj);
-    assert(Fn::check_call<>(CT_NonConst | CT_LValue));
-    std::visit(cobj);
-    assert(Fn::check_call<>(CT_Const | CT_LValue));
-    std::visit(std::move(obj));
-    assert(Fn::check_call<>(CT_NonConst | CT_RValue));
-    std::visit(std::move(cobj));
-    assert(Fn::check_call<>(CT_Const | CT_RValue));
+    // non-member
+    {
+      std::visit(obj);
+      assert(Fn::check_call<>(CT_NonConst | CT_LValue));
+      std::visit(cobj);
+      assert(Fn::check_call<>(CT_Const | CT_LValue));
+      std::visit(std::move(obj));
+      assert(Fn::check_call<>(CT_NonConst | CT_RValue));
+      std::visit(std::move(cobj));
+      assert(Fn::check_call<>(CT_Const | CT_RValue));
+    }
   }
   { // test call operator forwarding - single variant, single arg
     using V = std::variant<int>;
     V v(42);
-    std::visit(obj, v);
-    assert(Fn::check_call<int &>(CT_NonConst | CT_LValue));
-    std::visit(cobj, v);
-    assert(Fn::check_call<int &>(CT_Const | CT_LValue));
-    std::visit(std::move(obj), v);
-    assert(Fn::check_call<int &>(CT_NonConst | CT_RValue));
-    std::visit(std::move(cobj), v);
-    assert(Fn::check_call<int &>(CT_Const | CT_RValue));
+
+#if _LIBCPP_STD_VER >= 26
+    // member
+    {
+      v.visit(obj);
+      assert(Fn::check_call<int&>(CT_NonConst | CT_LValue));
+      v.visit(cobj);
+      assert(Fn::check_call<int&>(CT_Const | CT_LValue));
+      v.visit(std::move(obj));
+      assert(Fn::check_call<int&>(CT_NonConst | CT_RValue));
+      v.visit(std::move(cobj));
+      assert(Fn::check_call<int&>(CT_Const | CT_RValue));
+    }
+#endif
+    // non-member
+    {
+      std::visit(obj, v);
+      assert(Fn::check_call<int&>(CT_NonConst | CT_LValue));
+      std::visit(cobj, v);
+      assert(Fn::check_call<int&>(CT_Const | CT_LValue));
+      std::visit(std::move(obj), v);
+      assert(Fn::check_call<int&>(CT_NonConst | CT_RValue));
+      std::visit(std::move(cobj), v);
+      assert(Fn::check_call<int&>(CT_Const | CT_RValue));
+    }
   }
   { // test call operator forwarding - single variant, multi arg
     using V = std::variant<int, long, double>;
-    V v(42l);
-    std::visit(obj, v);
-    assert(Fn::check_call<long &>(CT_NonConst | CT_LValue));
-    std::visit(cobj, v);
-    assert(Fn::check_call<long &>(CT_Const | CT_LValue));
-    std::visit(std::move(obj), v);
-    assert(Fn::check_call<long &>(CT_NonConst | CT_RValue));
-    std::visit(std::move(cobj), v);
-    assert(Fn::check_call<long &>(CT_Const | CT_RValue));
+    V v(42L);
+
+#if _LIBCPP_STD_VER >= 26
+    // member
+    {
+      v.visit(obj);
+      assert(Fn::check_call<long&>(CT_NonConst | CT_LValue));
+      v.visit(cobj);
+      assert(Fn::check_call<long&>(CT_Const | CT_LValue));
+      v.visit(std::move(obj));
+      assert(Fn::check_call<long&>(CT_NonConst | CT_RValue));
+      v.visit(std::move(cobj));
+      assert(Fn::check_call<long&>(CT_Const | CT_RValue));
+    }
+#endif
+    // non-member
+    {
+      std::visit(obj, v);
+      assert(Fn::check_call<long&>(CT_NonConst | CT_LValue));
+      std::visit(cobj, v);
+      assert(Fn::check_call<long&>(CT_Const | CT_LValue));
+      std::visit(std::move(obj), v);
+      assert(Fn::check_call<long&>(CT_NonConst | CT_RValue));
+      std::visit(std::move(cobj), v);
+      assert(Fn::check_call<long&>(CT_Const | CT_RValue));
+    }
   }
   { // test call operator forwarding - multi variant, multi arg
-    using V = std::variant<int, long, double>;
-    using V2 = std::variant<int *, std::string>;
-    V v(42l);
+    using V  = std::variant<int, long, double>;
+    using V2 = std::variant<int*, std::string>;
+    V v(42L);
     V2 v2("hello");
-    std::visit(obj, v, v2);
-    assert((Fn::check_call<long &, std::string &>(CT_NonConst | CT_LValue)));
-    std::visit(cobj, v, v2);
-    assert((Fn::check_call<long &, std::string &>(CT_Const | CT_LValue)));
-    std::visit(std::move(obj), v, v2);
-    assert((Fn::check_call<long &, std::string &>(CT_NonConst | CT_RValue)));
-    std::visit(std::move(cobj), v, v2);
-    assert((Fn::check_call<long &, std::string &>(CT_Const | CT_RValue)));
+
+    // non-member
+    {
+      std::visit(obj, v, v2);
+      assert((Fn::check_call<long&, std::string&>(CT_NonConst | CT_LValue)));
+      std::visit(cobj, v, v2);
+      assert((Fn::check_call<long&, std::string&>(CT_Const | CT_LValue)));
+      std::visit(std::move(obj), v, v2);
+      assert((Fn::check_call<long&, std::string&>(CT_NonConst | CT_RValue)));
+      std::visit(std::move(cobj), v, v2);
+      assert((Fn::check_call<long&, std::string&>(CT_Const | CT_RValue)));
+    }
   }
   {
     using V = std::variant<int, long, double, std::string>;
-    V v1(42l), v2("hello"), v3(101), v4(1.1);
-    std::visit(obj, v1, v2, v3, v4);
-    assert((Fn::check_call<long &, std::string &, int &, double &>(CT_NonConst | CT_LValue)));
-    std::visit(cobj, v1, v2, v3, v4);
-    assert((Fn::check_call<long &, std::string &, int &, double &>(CT_Const | CT_LValue)));
-    std::visit(std::move(obj), v1, v2, v3, v4);
-    assert((Fn::check_call<long &, std::string &, int &, double &>(CT_NonConst | CT_RValue)));
-    std::visit(std::move(cobj), v1, v2, v3, v4);
-    assert((Fn::check_call<long &, std::string &, int &, double &>(CT_Const | CT_RValue)));
+    V v1(42L), v2("hello"), v3(101), v4(1.1);
+
+    // non-member
+    {
+      std::visit(obj, v1, v2, v3, v4);
+      assert((Fn::check_call<long&, std::string&, int&, double&>(CT_NonConst | CT_LValue)));
+      std::visit(cobj, v1, v2, v3, v4);
+      assert((Fn::check_call<long&, std::string&, int&, double&>(CT_Const | CT_LValue)));
+      std::visit(std::move(obj), v1, v2, v3, v4);
+      assert((Fn::check_call<long&, std::string&, int&, double&>(CT_NonConst | CT_RValue)));
+      std::visit(std::move(cobj), v1, v2, v3, v4);
+      assert((Fn::check_call<long&, std::string&, int&, double&>(CT_Const | CT_RValue)));
+    }
   }
   {
     using V = std::variant<int, long, double, int*, std::string>;
-    V v1(42l), v2("hello"), v3(nullptr), v4(1.1);
-    std::visit(obj, v1, v2, v3, v4);
-    assert((Fn::check_call<long &, std::string &, int *&, double &>(CT_NonConst | CT_LValue)));
-    std::visit(cobj, v1, v2, v3, v4);
-    assert((Fn::check_call<long &, std::string &, int *&, double &>(CT_Const | CT_LValue)));
-    std::visit(std::move(obj), v1, v2, v3, v4);
-    assert((Fn::check_call<long &, std::string &, int *&, double &>(CT_NonConst | CT_RValue)));
-    std::visit(std::move(cobj), v1, v2, v3, v4);
-    assert((Fn::check_call<long &, std::string &, int *&, double &>(CT_Const | CT_RValue)));
+    V v1(42L), v2("hello"), v3(nullptr), v4(1.1);
+
+    // non-member
+    {
+      std::visit(obj, v1, v2, v3, v4);
+      assert((Fn::check_call<long&, std::string&, int*&, double&>(CT_NonConst | CT_LValue)));
+      std::visit(cobj, v1, v2, v3, v4);
+      assert((Fn::check_call<long&, std::string&, int*&, double&>(CT_Const | CT_LValue)));
+      std::visit(std::move(obj), v1, v2, v3, v4);
+      assert((Fn::check_call<long&, std::string&, int*&, double&>(CT_NonConst | CT_RValue)));
+      std::visit(std::move(cobj), v1, v2, v3, v4);
+      assert((Fn::check_call<long&, std::string&, int*&, double&>(CT_Const | CT_RValue)));
+    }
   }
 }
 
+// Applies to non-member `std::visit` only.
 void test_argument_forwarding() {
   using Fn = ForwardingCallObject;
   Fn obj{};
-  const auto Val = CT_LValue | CT_NonConst;
+  const auto val = CT_LValue | CT_NonConst;
+
   { // single argument - value type
     using V = std::variant<int>;
     V v(42);
-    const V &cv = v;
-    std::visit(obj, v);
-    assert(Fn::check_call<int &>(Val));
-    std::visit(obj, cv);
-    assert(Fn::check_call<const int &>(Val));
-    std::visit(obj, std::move(v));
-    assert(Fn::check_call<int &&>(Val));
-    std::visit(obj, std::move(cv));
-    assert(Fn::check_call<const int &&>(Val));
+    const V& cv = v;
+
+#if _LIBCPP_STD_VER >= 26
+    // member
+    {
+      v.visit(obj);
+      assert(Fn::check_call<int&>(val));
+      cv.visit(obj);
+      assert(Fn::check_call<const int&>(val));
+      std::move(v).visit(obj);
+      assert(Fn::check_call<int&&>(val));
+      std::move(cv).visit(obj);
+      assert(Fn::check_call<const int&&>(val));
+    }
+#endif
+    // non-member
+    {
+      std::visit(obj, v);
+      assert(Fn::check_call<int&>(val));
+      std::visit(obj, cv);
+      assert(Fn::check_call<const int&>(val));
+      std::visit(obj, std::move(v));
+      assert(Fn::check_call<int&&>(val));
+      std::visit(obj, std::move(cv));
+      assert(Fn::check_call<const int&&>(val));
+    }
   }
 #if !defined(TEST_VARIANT_HAS_NO_REFERENCES)
   { // single argument - lvalue reference
-    using V = std::variant<int &>;
-    int x = 42;
+    using V = std::variant<int&>;
+    int x   = 42;
     V v(x);
-    const V &cv = v;
-    std::visit(obj, v);
-    assert(Fn::check_call<int &>(Val));
-    std::visit(obj, cv);
-    assert(Fn::check_call<int &>(Val));
-    std::visit(obj, std::move(v));
-    assert(Fn::check_call<int &>(Val));
-    std::visit(obj, std::move(cv));
-    assert(Fn::check_call<int &>(Val));
+    const V& cv = v;
+
+#  if _LIBCPP_STD_VER >= 26
+    // member
+    {
+      v.visit(obj);
+      assert(Fn::check_call<int&>(val));
+      cv.visit(obj);
+      assert(Fn::check_call<int&>(val));
+      std::move(v).visit(obj);
+      assert(Fn::check_call<int&>(val));
+      std::move(cv).visit(obj);
+      assert(Fn::check_call<int&>(val));
+      assert(false);
+    }
+#  endif
+    // non-member
+    {
+      std::visit(obj, v);
+      assert(Fn::check_call<int&>(val));
+      std::visit(obj, cv);
+      assert(Fn::check_call<int&>(val));
+      std::visit(obj, std::move(v));
+      assert(Fn::check_call<int&>(val));
+      std::visit(obj, std::move(cv));
+      assert(Fn::check_call<int&>(val));
+    }
   }
   { // single argument - rvalue reference
-    using V = std::variant<int &&>;
-    int x = 42;
+    using V = std::variant<int&&>;
+    int x   = 42;
     V v(std::move(x));
-    const V &cv = v;
-    std::visit(obj, v);
-    assert(Fn::check_call<int &>(Val));
-    std::visit(obj, cv);
-    assert(Fn::check_call<int &>(Val));
-    std::visit(obj, std::move(v));
-    assert(Fn::check_call<int &&>(Val));
-    std::visit(obj, std::move(cv));
-    assert(Fn::check_call<int &&>(Val));
+    const V& cv = v;
+
+#  if _LIBCPP_STD_VER >= 26
+    // member
+    {
+      v.visit(obj);
+      assert(Fn::check_call<int&>(val));
+      cvstd::visit(obj);
+      assert(Fn::check_call<int&>(val));
+      std::move(v).visit(obj);
+      assert(Fn::check_call<int&&>(val));
+      std::move(cv).visit(obj);
+      assert(Fn::check_call<int&&>(val));
+    }
+#  endif
+    // non-member
+    {
+      std::visit(obj, v);
+      assert(Fn::check_call<int&>(val));
+      std::visit(obj, cv);
+      assert(Fn::check_call<int&>(val));
+      std::visit(obj, std::move(v));
+      assert(Fn::check_call<int&&>(val));
+      std::visit(obj, std::move(cv));
+      assert(Fn::check_call<int&&>(val));
+    }
   }
 #endif
   { // multi argument - multi variant
     using V = std::variant<int, std::string, long>;
-    V v1(42), v2("hello"), v3(43l);
-    std::visit(obj, v1, v2, v3);
-    assert((Fn::check_call<int &, std::string &, long &>(Val)));
-    std::visit(obj, std::as_const(v1), std::as_const(v2), std::move(v3));
-    assert((Fn::check_call<const int &, const std::string &, long &&>(Val)));
+    V v1(42), v2("hello"), v3(43L);
+
+    // non-member
+    {
+      std::visit(obj, v1, v2, v3);
+      assert((Fn::check_call<int&, std::string&, long&>(val)));
+      std::visit(obj, std::as_const(v1), std::as_const(v2), std::move(v3));
+      assert((Fn::check_call<const int&, const std::string&, long&&>(val)));
+    }
   }
   {
     using V = std::variant<int, long, double, std::string>;
-    V v1(42l), v2("hello"), v3(101), v4(1.1);
-    std::visit(obj, v1, v2, v3, v4);
-    assert((Fn::check_call<long &, std::string &, int &, double &>(Val)));
-    std::visit(obj, std::as_const(v1), std::as_const(v2), std::move(v3), std::move(v4));
-    assert((Fn::check_call<const long &, const std::string &, int &&, double &&>(Val)));
+    V v1(42L), v2("hello"), v3(101), v4(1.1);
+
+    // non-member
+    {
+      std::visit(obj, v1, v2, v3, v4);
+      assert((Fn::check_call<long&, std::string&, int&, double&>(val)));
+      std::visit(obj, std::as_const(v1), std::as_const(v2), std::move(v3), std::move(v4));
+      assert((Fn::check_call<const long&, const std::string&, int&&, double&&>(val)));
+    }
   }
   {
     using V = std::variant<int, long, double, int*, std::string>;
-    V v1(42l), v2("hello"), v3(nullptr), v4(1.1);
-    std::visit(obj, v1, v2, v3, v4);
-    assert((Fn::check_call<long &, std::string &, int *&, double &>(Val)));
-    std::visit(obj, std::as_const(v1), std::as_const(v2), std::move(v3), std::move(v4));
-    assert((Fn::check_call<const long &, const std::string &, int *&&, double &&>(Val)));
+    V v1(42L), v2("hello"), v3(nullptr), v4(1.1);
+
+    // non-member
+    {
+      std::visit(obj, v1, v2, v3, v4);
+      assert((Fn::check_call<long&, std::string&, int*&, double&>(val)));
+      std::visit(obj, std::as_const(v1), std::as_const(v2), std::move(v3), std::move(v4));
+      assert((Fn::check_call<const long&, const std::string&, int*&&, double&&>(val)));
+    }
   }
 }
 
 void test_return_type() {
   using Fn = ForwardingCallObject;
   Fn o...
[truncated]

@Zingam Zingam changed the title [libc++][variant] P2637R3: Member visit [libc++][variant] P2637R3: Member visit (for variant) Dec 28, 2023
@Zingam Zingam changed the title [libc++][variant] P2637R3: Member visit (for variant) [libc++][variant] P2637R3: Member visit (std::variant) Dec 28, 2023
@mordante mordante self-assigned this Dec 28, 2023
Copy link

github-actions bot commented Dec 30, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Member

@mordante mordante left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this!

test_call_operator_forwarding();
test_argument_forwarding();
test_return_type();
test_constexpr();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you not using our normal constexpr test methods?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My reasoning was to keep the member visit tests as close as possible to the original non-member function ones, for easier matching/reviewing as they are expected to produce the same results. I kept the structure and the names. Initially I wrote the tests in the same file before splitting them into two separate.

@H-G-Hristov H-G-Hristov requested a review from mordante January 18, 2024 19:55
@H-G-Hristov
Copy link
Contributor Author

This FreeBSD failure seems unrelated:

# .---command stderr------------
# | In file included from /usr/home/buildkite/.buildkite-agent/builds/freebsd-test-1/llvm-project/libcxx-ci/libcxx/test/std/algorithms/numeric.ops/transform.reduce/pstl.exception_handling.pass.cpp:20:
# | /home/buildkite/.buildkite-agent/builds/freebsd-test-1/llvm-project/libcxx-ci/libcxx/test/support/check_assertion.h:293:25: error: use of undeclared identifier 'SIGILL'
# |       if (exit_code_ == SIGILL || exit_code_ == SIGTRAP) {
# |                         ^
# | /home/buildkite/.buildkite-agent/builds/freebsd-test-1/llvm-project/libcxx-ci/libcxx/test/support/check_assertion.h:293:49: error: use of undeclared identifier 'SIGTRAP'
# |       if (exit_code_ == SIGILL || exit_code_ == SIGTRAP) {
# |                                                 ^
# | 2 errors generated.
# `-----------------------------
# error: command failed with exit status: 1

Copy link
Member

@mordante mordante left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly LGTM, I like to see it again for a quick look.

@H-G-Hristov H-G-Hristov requested a review from mordante January 20, 2024 19:55
Copy link
Member

@mordante mordante left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@Zingam
Copy link
Contributor

Zingam commented Jan 21, 2024

The FreeBSD CI failure is unrelated and should be fixed upstream. Merging.

@Zingam Zingam merged commit 3412bc7 into llvm:main Jan 21, 2024
Zingam added a commit that referenced this pull request Jan 21, 2024
…76449)

Implements parts of: `P2637R3` https://wg21.link/P2637R3
(https://eel.is/c++draft/variant.visit)

Implements:
`basic_format_arg.visit()`
`basic_format_arg.visit<R>()`
Deprecates:
`std::visit_format_arg()`

The tests are as close as possible to the non-member function tests.

To land after: #76447,
#76268

---------

Co-authored-by: Zingam <[email protected]>
@H-G-Hristov H-G-Hristov deleted the hgh/libcxx/P2637R3-member-visit-variant branch January 21, 2024 10:36
blueboxd pushed a commit to blueboxd/libcxx that referenced this pull request Feb 7, 2024
…76449)

Implements parts of: `P2637R3` https://wg21.link/P2637R3
(https://eel.is/c++draft/variant.visit)

Implements:
`basic_format_arg.visit()`
`basic_format_arg.visit<R>()`
Deprecates:
`std::visit_format_arg()`

The tests are as close as possible to the non-member function tests.

To land after: llvm/llvm-project#76447,
llvm/llvm-project#76268

---------

Co-authored-by: Zingam <[email protected]>
NOKEYCHECK=True
GitOrigin-RevId: 7d9b5aa65b09126031e1c2903605a7d34aea4bc1
frederick-vs-ja added a commit that referenced this pull request Oct 25, 2024
In C++20 mode, `__cpp_lib_optional` and `__cpp_lib_variant` should be
`202106L` due to DR P2231R1.

In C++26 mode, `__cpp_lib_variant` should be bumped to `202306L` due to
P2637R3.
- Clang 16/17 shouldn't get this bumping (as member `visit` requires
explicit object parameters), but it's very tricky to make the bumping
conditionally enabled. I _hope_ unconditionally bumping in C++26 will be
OK for LLVM 20 when the support for Clang 17 is dropped.

Related PRs:
- https://reviews.llvm.org/D102119
- #83335
- #76447
winner245 pushed a commit to winner245/llvm-project that referenced this pull request Oct 26, 2024
In C++20 mode, `__cpp_lib_optional` and `__cpp_lib_variant` should be
`202106L` due to DR P2231R1.

In C++26 mode, `__cpp_lib_variant` should be bumped to `202306L` due to
P2637R3.
- Clang 16/17 shouldn't get this bumping (as member `visit` requires
explicit object parameters), but it's very tricky to make the bumping
conditionally enabled. I _hope_ unconditionally bumping in C++26 will be
OK for LLVM 20 when the support for Clang 17 is dropped.

Related PRs:
- https://reviews.llvm.org/D102119
- llvm#83335
- llvm#76447
NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this pull request Nov 4, 2024
In C++20 mode, `__cpp_lib_optional` and `__cpp_lib_variant` should be
`202106L` due to DR P2231R1.

In C++26 mode, `__cpp_lib_variant` should be bumped to `202306L` due to
P2637R3.
- Clang 16/17 shouldn't get this bumping (as member `visit` requires
explicit object parameters), but it's very tricky to make the bumping
conditionally enabled. I _hope_ unconditionally bumping in C++26 will be
OK for LLVM 20 when the support for Clang 17 is dropped.

Related PRs:
- https://reviews.llvm.org/D102119
- llvm#83335
- llvm#76447
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants