[libc++] [C++2b] [P2162] Allow inheritance from std::variant.

[curdeius]

Summary

This patch changes the variant even in pre-C++2b.
It should not break anything, only allow use cases that didn't work previously.

Notes:
__as_variant is used in __visitation::__variant::__visit_alt, but I haven't used it in __visitation::__variant::__visit_alt_at.
That's because it is used only in __visit_value_at, which in turn is always used on variant specializations (that's in comparison operators).

• https://wg21.link/P2162

[BRevzin]

Previous standards didn't disallow this AFAIK.

I would describe it more as the previous standard underspecified whether this was allowed or not. libc++ already supported the use-cases that this paper was trying to standardize supporting.

This looks good to me but @mpark's opinion is more relevant here.

[curdeius]

• Fix gcc build.
• Add test for visiting a non-evil derived variant.

[ldionne]

Previous standards didn't disallow this AFAIK. But I'm OK to make this change C++2b-only.

I think it's fine to backport this to all versions of C++. If someone was taking advantage of this underspecification before C++20, we're only doing them a service to break their code sooner in previous C++ versions (that way they won't break when they upgrade).

What else should be tested?

We don't have a test that std::visit with something that isn't derived from a std::variant doesn't work in a SFINAE-friendly manner. Basically, testing this part of the paper: + Constraints: Vi is a valid type for all 0 <= i < n.

Is there a way to avoid __as_variant? Should it be a goal?

I don't think this should be a goal. The Standard describes it that way, I think it's fine.

This looks pretty good to me except for the comments I left. I would love for @mpark to take a look - his opinion overrides mine everywhere since he's the variant grand master.

libcxx/include/variant

331

No need for inline on these, they are already inline by way of being templates.

1669–1672

Calling __as_variant(__vs).valueless_by_exception() here instead would be closer to the Standard and would allow getting rid of __valueless_by_exception -- WDYT?

libcxx/test/std/utilities/variant/variant.relops/relops.pass.cpp

274 ↗

(On Diff #326316)

Do we really want to add [[maybe_unused]] to all typedefs in the test suite? I think we should just disable the unused typedef warning in the test suite if that's causing problems, that'll be much lighter notation-wise.

libcxx/test/std/utilities/variant/variant.visit/visit.pass.cpp

367

Maybe add a short comment explaining what we expect and why it's useful to test this? I like to put myself in the shoes of my future self when I don't have the paper in front of me - I wouldn't understand why this test exists. If nothing else, perhaps mentioning the paper number is sufficient.

379

Is there a reason why you're only calling those in an unevaluated context? Can't we actually call and "run" them (even though I understand it's trivial)?

[tcanens]

libcxx/test/std/utilities/variant/variant.visit/visit.pass.cpp

379

Constructing the type_info base class might be a bit tricky...

[curdeius]

libcxx/include/variant

1669–1672

Yes, I agree. That's actually because I started by this part and only later added __as_variant. I'll use __as_variant and remove __valueless_by_exception.

libcxx/test/std/utilities/variant/variant.relops/relops.pass.cpp

274 ↗

(On Diff #326316)

Indeed, it will be cleaner just do ignore this particular warning in this test.

libcxx/test/std/utilities/variant/variant.visit/visit.pass.cpp

367

Will do.

379

Indeed, that was the reason why I just used unevaluated context.
I'll try to do something else instead, because the reason for using type_info that both variant and type_info has __impl member that is ambiguous when calling visit (prior to this patch).

[ldionne]

libcxx/test/std/utilities/variant/variant.visit/visit.pass.cpp

379

Oh, I hadn't thought of that. Yes, that's obvious now :-).

Feel free to disregard this comment and leave it unevaluated, or to do whatever you were thinking about with a hand-made class.

[• Quuxplusone]

libcxx/test/std/utilities/variant/variant.visit/visit.pass.cpp

376

Would it be appropriate to also test the behavior of struct X : std::variant<int>, std::variant<long> {} and struct Y : private std::variant<int> {}? I don't think we expect either of those to support std::visit, but I wonder if they're supposed to SFINAE away cleanly or give some awful template error message or what.

libcxx/utils/generate_feature_test_macro_components.py

630

Since you're making the change in all language versions (IIUC), shouldn't the feature-test macro have the higher value in all language versions? (Has this situation ever come up in WG21 before? what did the Committee think vendors should do?)

[curdeius]

libcxx/test/std/utilities/variant/variant.visit/visit.pass.cpp

376

I don't see how it may be SFINAE-friendly (but I haven't checked).

libcxx/utils/generate_feature_test_macro_components.py

630

Ha, good catch. No idea what should be done, probably always setting to the higher value is OK.

[curdeius]

• Use as_variant instead of valueless_by_exception.
• Add tests without type_info.
• Remove inline.

libcxx/test/std/utilities/variant/variant.visit/visit.pass.cpp

376

Well, I've checked those and they fail with more or less cryptic messages, but still I find them pretty nice.
E.g.

struct BadVariant : std::variant<int>, std::variant<double> {
  using std::variant<int>::variant;
};
std::visit([](auto x) { assert(x == 12); }, BadVariant{12});

fails with:

/home/marek/llvm-project/libcxx/include/variant:1669:36: error: no matching function for call to '__as_variant'
  const bool __valueless = (... || _VSTD::__as_variant(__vs).valueless_by_exception());
                                   ^~~~~~~~~~~~~~~~~~~
/home/marek/llvm-project/libcxx/include/__config:779:15: note: expanded from macro '_VSTD'
#define _VSTD std::_LIBCPP_ABI_NAMESPACE
              ^
/home/marek/llvm-project/libcxx/include/variant:1680:10: note: in instantiation of function template specialization 'std::__throw_if_valueless<BadVariant>' requested here
  _VSTD::__throw_if_valueless(_VSTD::forward<_Vs>(__vs)...);
         ^
/home/marek/llvm-project/libcxx/test/std/utilities/variant/variant.visit/visit.pass.cpp:397:8: note: in instantiation of function template specialization 'std::visit<(lambda at /home/marek/llvm-project/libcxx/test/std/utilities/variant/variant.visit/visit.pass.cpp:397:14), BadVariant>' requested here
  std::visit([](auto x) { assert(x == 12); }, BadVariant{12});
       ^
/home/marek/llvm-project/libcxx/include/variant:326:1: note: candidate template ignored: failed template argument deduction
__as_variant(variant<_Types...>& __vs) noexcept {
^
/home/marek/llvm-project/libcxx/include/variant:332:1: note: candidate template ignored: failed template argument deduction
__as_variant(const variant<_Types...>& __vs) noexcept {
^
...

I think there's nothing we can do about it.

libcxx/utils/generate_feature_test_macro_components.py

630

@ldionne, what's your opinion on this?

[tcanens]

libcxx/test/std/utilities/variant/variant.visit/visit.pass.cpp

376

The wording is pretty clear that this case needs to SFINAE.

[curdeius]

libcxx/test/std/utilities/variant/variant.visit/visit.pass.cpp

372

s/pick/picks/.
Will fix typos locally for the moment. Here and in relops.

[curdeius]

libcxx/test/std/utilities/variant/variant.visit/visit.pass.cpp

376

Right. I might need to test it in a different manner though.

[ldionne]

libcxx/utils/generate_feature_test_macro_components.py

630

Yes, I think we need to always use the higher value. Otherwise, we're pretending that we implement a slightly different version of std::variant than we really are.

[curdeius]

• Make visit SFINAE-friendly.
• Make visit<R> SFINAE-friendly.
• Bump feature-test macro for C++17 onwards.

[curdeius]

• Fix csv status.
• Fmt.

[• Quuxplusone]

libcxx/test/std/utilities/variant/variant.relops/relops.pass.cpp

293 ↗

(On Diff #329585)

Here you don't mean "visit", you mean "relops." Actually I think lines 289-320 in this file are unnecessary.

libcxx/test/std/utilities/variant/variant.visit/visit.pass.cpp

357

From the outside, it seems vastly more important to verify that visit does not take std::get<Idx>(v) specializations from the base class. Could you add a test for that?
Of course it probably isn't necessary to test all the possible entry-points, right? they should all work pretty much the same?

391

Nit: mark this operator() as const, for good style.

408

Nit: > > should be >> since this test needn't run in C++03 mode.

Non-nit: Please write 0, not int().

409

Please also static_assert(!has_visit<int>).
Please also test struct BadVariant2 : private std::variant<long, float> {} (!has_visit).
Please also test struct GoodVariant : public std::variant<long, float> {} (has_visit).
Optionally also test struct GoodVariant2 : public GoodVariant {} (has_visit).

[curdeius]

• Remove relops changes.
• Add has_visit tests.
• Add test for get<Index>.

[curdeius]

libcxx/test/std/utilities/variant/variant.visit/visit.pass.cpp

357

Not sure if I correctly understood what test you suggested. Could you have a look please?

[• Quuxplusone]

LGTM!
My understanding is that it's still blocked on @ldionne to re-review and see if his (very long-ago) comments have been sufficiently addressed.

libcxx/include/variant

340

Minor bikeshed: & const& const&& && is a weird order to put these four overloads in :)

libcxx/test/std/language.support/support.limits/support.limits.general/version.version.pass.cpp

4059

Make sure to re-run the generator scripts before landing. This diff should go away, I think.

libcxx/test/std/utilities/variant/variant.visit/visit.pass.cpp

357

Yes, that's what I meant. :) Except let's make one of those MyVariant arguments a const reference too, for extra overload-resolution matchy-matchiness.

const MyVariant v1 = 42;
std::visit([](auto x) { assert(x == 42); }, v1);
std::visit([](auto x) { assert(x == -1.25f); }, MyVariant(-1.25f));

Drive-by changed the float to a value that can be represented exactly in base-2, because I'm paranoid about floats. :)

421

nit: >> is OK here

[curdeius]

• Change order of __as_variant overloads.
• Test &, const &, &&, const &&.
• Fmt.
• Rebase.

[curdeius]

libcxx/include/variant

340

I changed to paper's order (&, const &, &&, const &&).

[ldionne]

LGTM with minor nitpick. Thanks!

libcxx/include/variant

1680

void_t here is not necessary. Just typename = decltype(...) should be sufficient for SFINAE. Same below.

[curdeius]

libcxx/include/variant

1680

@ldionne, but... _Vs is a parameter pack. How can that be done without void_t?

[ldionne]

Ship it!

libcxx/include/variant

1680

My bad, I looked too quickly.

libcxx/docs/Cxx2bStatusPaperStatus.csv

 "Paper #","Group","Paper Name","Meeting","Status","First released version"
 "`P0881R7 <https://wg21.link/P0881R7>`__","LWG","A Proposal to add stacktrace library","Autumn 2020","",""
 "`P0943R6 <https://wg21.link/P0943R6>`__","LWG","Support C atomics in C++","Autumn 2020","",""
 "`P1048R1 <https://wg21.link/P1048R1>`__","LWG","A proposal for a type trait to detect scoped enumerations","Autumn 2020","|Complete|","12.0"
 "`P1679R3 <https://wg21.link/P1679R3>`__","LWG","string contains function","Autumn 2020","|Complete|","12.0"
 "","","","","",""
 "`P1682R3 <https://wg21.link/P1682R3>`__","LWG","std::to_underlying for enumerations","February 2021","|Complete|","13.0"
 "`P2017R1 <https://wg21.link/P2017R1>`__","LWG","Conditionally borrowed ranges","February 2021","",""
 "`P2160R1 <https://wg21.link/P2160R1>`__","LWG","Locks lock lockables","February 2021","",""
-"`P2162R2 <https://wg21.link/P2162R2>`__","LWG","Inheriting from std::variant","February 2021","",""
+"`P2162R2 <https://wg21.link/P2162R2>`__","LWG","Inheriting from std::variant","February 2021","|Complete|","13.0"
 "`P2212R2 <https://wg21.link/P2212R2>`__","LWG","Relax Requirements for time_point::clock","February 2021","",""
 "`P2259R1 <https://wg21.link/P2259R1>`__","LWG","Repairing input range adaptors and counted_iterator","February 2021","",""
 "","","","","",""

libcxx/docs/FeatureTestMacroTable.rst

.. table:: Current Status
     ``__cpp_lib_transparent_operators``               ``201510L``
     ------------------------------------------------- -----------------
     ``__cpp_lib_type_trait_variable_templates``       ``201510L``
     ------------------------------------------------- -----------------
     ``__cpp_lib_uncaught_exceptions``                 ``201411L``
     ------------------------------------------------- -----------------
     ``__cpp_lib_unordered_map_try_emplace``           ``201411L``
     ------------------------------------------------- -----------------
-    ``__cpp_lib_variant``                             ``201606L``
+    ``__cpp_lib_variant``                             ``202102L``
     ------------------------------------------------- -----------------
     ``__cpp_lib_void_t``                              ``201411L``
     ------------------------------------------------- -----------------
     **C++ 20**
     -------------------------------------------------------------------
     ``__cpp_lib_array_constexpr``                     ``201811L``
     ------------------------------------------------- -----------------
     ``__cpp_lib_assume_aligned``                      *unimplemented*

libcxx/include/variant

std::tuple_element_t<
       __choose_index_type(_NumAlts),
       std::tuple<unsigned char, unsigned short, unsigned int>
   >;
 #endif
 
 template <class _IndexType>
 constexpr _IndexType __variant_npos = static_cast<_IndexType>(-1);
 
+template <class... _Types>
+class _LIBCPP_TEMPLATE_VIS variant;
+
+template <class... _Types>
+_LIBCPP_INLINE_VISIBILITY constexpr variant<_Types...>&
+__as_variant(variant<_Types...>& __vs) noexcept {
+  return __vs;
+}
+
+template <class... _Types>
+_LIBCPP_INLINE_VISIBILITY constexpr const variant<_Types...>&
+__as_variant(const variant<_Types...>& __vs) noexcept {
+  return __vs;
+}
+
+template <class... _Types>
+_LIBCPP_INLINE_VISIBILITY constexpr variant<_Types...>&&
+__as_variant(variant<_Types...>&& __vs) noexcept {
+  return _VSTD::move(__vs);
+}
+
+template <class... _Types>
+_LIBCPP_INLINE_VISIBILITY constexpr const variant<_Types...>&&
+__as_variant(const variant<_Types...>&& __vs) noexcept {
+  return _VSTD::move(__vs);
+}
+
 namespace __find_detail {
 
 template <class _Tp, class... _Types>
 inline _LIBCPP_INLINE_VISIBILITY
 constexpr size_t __find_index() {
   constexpr bool __matches[] = {is_same_v<_Tp, _Types>...};
   size_t __result = __not_found;
   for (size_t __i = 0; __i < sizeof...(_Types); ++__i) {
return __base::__visit_alt_at(__index,
                                   _VSTD::forward<_Visitor>(__visitor),
                                   _VSTD::forward<_Vs>(__vs).__impl...);
   }
 
   template <class _Visitor, class... _Vs>
   inline _LIBCPP_INLINE_VISIBILITY
   static constexpr decltype(auto) __visit_alt(_Visitor&& __visitor,
                                               _Vs&&... __vs) {
-    return __base::__visit_alt(_VSTD::forward<_Visitor>(__visitor),
-                               _VSTD::forward<_Vs>(__vs).__impl...);
+    return __base::__visit_alt(
+        _VSTD::forward<_Visitor>(__visitor),
+        _VSTD::__as_variant(_VSTD::forward<_Vs>(__vs)).__impl...);
   }
 
   template <class _Visitor, class... _Vs>
   inline _LIBCPP_INLINE_VISIBILITY
   static constexpr decltype(auto)
   __visit_value_at(size_t __index, _Visitor&& __visitor, _Vs&&... __vs) {
     return __visit_alt_at(
         __index,
         __make_value_visitor(_VSTD::forward<_Visitor>(__visitor)),
         _VSTD::forward<_Vs>(__vs)...);
   }
 
   template <class _Visitor, class... _Vs>
   inline _LIBCPP_INLINE_VISIBILITY
   static constexpr decltype(auto) __visit_value(_Visitor&& __visitor,
                                                 _Vs&&... __vs) {
     return __visit_alt(
         __make_value_visitor(_VSTD::forward<_Visitor>(__visitor)),
         _VSTD::forward<_Vs>(__vs)...);
   }
+
 #if _LIBCPP_STD_VER > 17
   template <class _Rp, class _Visitor, class... _Vs>
   inline _LIBCPP_INLINE_VISIBILITY
   static constexpr _Rp __visit_value(_Visitor&& __visitor,
                                      _Vs&&... __vs) {
     return __visit_alt(
         __make_value_visitor<_Rp>(_VSTD::forward<_Visitor>(__visitor)),
         _VSTD::forward<_Vs>(__vs)...);
constexpr bool operator>=(const variant<_Types...>& __lhs,
   if (__lhs.index() > __rhs.index()) return true;
   if (__lhs.index() < __rhs.index()) return false;
   return __variant::__visit_value_at(
       __lhs.index(), __convert_to_bool<greater_equal<>>{}, __lhs, __rhs);
 }
 
 template <class... _Vs>
 inline _LIBCPP_INLINE_VISIBILITY
-_LIBCPP_AVAILABILITY_THROW_BAD_VARIANT_ACCESS
-constexpr void __throw_if_valueless(_Vs&&... __vs) {
-  const bool __valueless = (... || __vs.valueless_by_exception());
+    _LIBCPP_AVAILABILITY_THROW_BAD_VARIANT_ACCESS constexpr void
+    __throw_if_valueless(_Vs&&... __vs) {
+  const bool __valueless =
+      (... || _VSTD::__as_variant(__vs).valueless_by_exception());
   if (__valueless) {
-      __throw_bad_variant_access();
   }
 }
 
-template <class _Visitor, class... _Vs>
+template <
+    class _Visitor, class... _Vs,
+    typename = void_t<decltype(_VSTD::__as_variant(_VSTD::declval<_Vs>()))...> >
 inline _LIBCPP_INLINE_VISIBILITY
-_LIBCPP_AVAILABILITY_THROW_BAD_VARIANT_ACCESS
-constexpr decltype(auto) visit(_Visitor&& __visitor, _Vs&&... __vs) {
+    _LIBCPP_AVAILABILITY_THROW_BAD_VARIANT_ACCESS constexpr
+    decltype(auto) visit(_Visitor&& __visitor, _Vs&&... __vs) {
   using __variant_detail::__visitation::__variant;
   _VSTD::__throw_if_valueless(_VSTD::forward<_Vs>(__vs)...);
   return __variant::__visit_value(_VSTD::forward<_Visitor>(__visitor),
                                   _VSTD::forward<_Vs>(__vs)...);
 }
 
 #if _LIBCPP_STD_VER > 17
-template <class _Rp, class _Visitor, class... _Vs>
+template <
+    class _Rp, class _Visitor, class... _Vs,
+    typename = void_t<decltype(_VSTD::__as_variant(_VSTD::declval<_Vs>()))...> >
 inline _LIBCPP_INLINE_VISIBILITY
-_LIBCPP_AVAILABILITY_THROW_BAD_VARIANT_ACCESS
-constexpr _Rp visit(_Visitor&& __visitor, _Vs&&... __vs) {
+    _LIBCPP_AVAILABILITY_THROW_BAD_VARIANT_ACCESS constexpr _Rp
+    visit(_Visitor&& __visitor, _Vs&&... __vs) {
   using __variant_detail::__visitation::__variant;
   _VSTD::__throw_if_valueless(_VSTD::forward<_Vs>(__vs)...);
   return __variant::__visit_value<_Rp>(_VSTD::forward<_Visitor>(__visitor),
                                        _VSTD::forward<_Vs>(__vs)...);
 }
 #endif
 
 struct _LIBCPP_TEMPLATE_VIS monostate {};

libcxx/include/version

 __cpp_lib_transparent_operators                         201510L <functional> <memory>
                                                         201210L // C++14
 __cpp_lib_tuple_element_t                               201402L <tuple>
 __cpp_lib_tuples_by_type                                201304L <tuple> <utility>
 __cpp_lib_type_trait_variable_templates                 201510L <type_traits>
 __cpp_lib_uncaught_exceptions                           201411L <exception>
 __cpp_lib_unordered_map_try_emplace                     201411L <unordered_map>
 __cpp_lib_unwrap_ref                                    201811L <functional>
-__cpp_lib_variant                                       201606L <variant>
+__cpp_lib_variant                                       202102L <variant>
 __cpp_lib_void_t                                        201411L <type_traits>
 
 */
 
 #include <__config>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header

 # define __cpp_lib_shared_ptr_weak_type                 201606L
 # define __cpp_lib_string_view                          201606L
 // # define __cpp_lib_to_chars                             201611L
 # undef  __cpp_lib_transparent_operators
 # define __cpp_lib_transparent_operators                201510L
 # define __cpp_lib_type_trait_variable_templates        201510L
 # define __cpp_lib_uncaught_exceptions                  201411L
 # define __cpp_lib_unordered_map_try_emplace            201411L
-# define __cpp_lib_variant                              201606L
+# define __cpp_lib_variant                              202102L
 # define __cpp_lib_void_t                               201411L
 #endif
 
 #if _LIBCPP_STD_VER > 17
 # undef  __cpp_lib_array_constexpr
 # define __cpp_lib_array_constexpr                      201811L
 // # define __cpp_lib_assume_aligned                       201811L
 # if !defined(_LIBCPP_HAS_NO_THREADS)

libcxx/test/std/language.support/support.limits/support.limits.general/variant.version.pass.cpp

 //
 // clang-format off
 
 // <variant>
 
 // Test the feature test macros defined by <variant>
 
 /*  Constant             Value
-    __cpp_lib_variant    201606L [C++17]
+    __cpp_lib_variant    202102L [C++17]
 */
 
 #include <variant>
 #include "test_macros.h"
 
 #if TEST_STD_VER < 14
 
 # ifdef __cpp_lib_variant
 #   error "__cpp_lib_variant should not be defined before c++17"
 # endif
 
 #elif TEST_STD_VER == 14
 
 # ifdef __cpp_lib_variant
 #   error "__cpp_lib_variant should not be defined before c++17"
 # endif
 
 #elif TEST_STD_VER == 17
 
 # ifndef __cpp_lib_variant
 #   error "__cpp_lib_variant should be defined in c++17"
 # endif
-# if __cpp_lib_variant != 201606L
-#   error "__cpp_lib_variant should have the value 201606L in c++17"
+# if __cpp_lib_variant != 202102L
+#   error "__cpp_lib_variant should have the value 202102L in c++17"
 # endif
 
 #elif TEST_STD_VER == 20
 
 # ifndef __cpp_lib_variant
 #   error "__cpp_lib_variant should be defined in c++20"
 # endif
-# if __cpp_lib_variant != 201606L
-#   error "__cpp_lib_variant should have the value 201606L in c++20"
+# if __cpp_lib_variant != 202102L
+#   error "__cpp_lib_variant should have the value 202102L in c++20"
 # endif
 
 #elif TEST_STD_VER > 20
 
 # ifndef __cpp_lib_variant
 #   error "__cpp_lib_variant should be defined in c++2b"
 # endif
-# if __cpp_lib_variant != 201606L
-#   error "__cpp_lib_variant should have the value 201606L in c++2b"
+# if __cpp_lib_variant != 202102L
+#   error "__cpp_lib_variant should have the value 202102L in c++2b"
 # endif
 
 #endif // TEST_STD_VER > 20
 
 int main(int, char**) { return 0; }

libcxx/test/std/language.support/support.limits/support.limits.general/version.version.pass.cpp

/*  Constant                                       Value
     __cpp_lib_transparent_operators                201210L [C++14]
                                                    201510L [C++17]
     __cpp_lib_tuple_element_t                      201402L [C++14]
     __cpp_lib_tuples_by_type                       201304L [C++14]
     __cpp_lib_type_trait_variable_templates        201510L [C++17]
     __cpp_lib_uncaught_exceptions                  201411L [C++17]
     __cpp_lib_unordered_map_try_emplace            201411L [C++17]
     __cpp_lib_unwrap_ref                           201811L [C++20]
-    __cpp_lib_variant                              201606L [C++17]
+    __cpp_lib_variant                              202102L [C++17]
     __cpp_lib_void_t                               201411L [C++17]
 */
 
 #include <version>
 #include "test_macros.h"
 
 #if TEST_STD_VER < 14
 

 
 # ifdef __cpp_lib_unwrap_ref
 #   error "__cpp_lib_unwrap_ref should not be defined before c++20"
 # endif
 
 # ifndef __cpp_lib_variant
 #   error "__cpp_lib_variant should be defined in c++17"
 # endif
-# if __cpp_lib_variant != 201606L
-#   error "__cpp_lib_variant should have the value 201606L in c++17"
+# if __cpp_lib_variant != 202102L
+#   error "__cpp_lib_variant should have the value 202102L in c++17"
 # endif
 
 # ifndef __cpp_lib_void_t
 #   error "__cpp_lib_void_t should be defined in c++17"
 # endif
 # if __cpp_lib_void_t != 201411L
 #   error "__cpp_lib_void_t should have the value 201411L in c++17"
 # endif

 # endif
 # if __cpp_lib_unwrap_ref != 201811L
 #   error "__cpp_lib_unwrap_ref should have the value 201811L in c++20"
 # endif
 
 # ifndef __cpp_lib_variant
 #   error "__cpp_lib_variant should be defined in c++20"
 # endif
-# if __cpp_lib_variant != 201606L
-#   error "__cpp_lib_variant should have the value 201606L in c++20"
+# if __cpp_lib_variant != 202102L
+#   error "__cpp_lib_variant should have the value 202102L in c++20"
 # endif
 
 # ifndef __cpp_lib_void_t
 #   error "__cpp_lib_void_t should be defined in c++20"
 # endif
 # if __cpp_lib_void_t != 201411L
 #   error "__cpp_lib_void_t should have the value 201411L in c++20"
 # endif

 
 # ifndef __cpp_lib_map_try_emplace
 #   error "__cpp_lib_map_try_emplace should be defined in c++2b"
 # endif
 # if __cpp_lib_map_try_emplace != 201411L
 #   error "__cpp_lib_map_try_emplace should have the value 201411L in c++2b"
 # endif
 
 # if defined(__cpp_concepts) && __cpp_concepts >= 201907L
 #   ifndef __cpp_lib_math_constants
 #     error "__cpp_lib_math_constants should be defined in c++2b"
 #   endif
 #   if __cpp_lib_math_constants != 201907L
 #     error "__cpp_lib_math_constants should have the value 201907L in c++2b"
 #   endif
 # else
 #   ifdef __cpp_lib_math_constants

 # endif
 # if __cpp_lib_unwrap_ref != 201811L
 #   error "__cpp_lib_unwrap_ref should have the value 201811L in c++2b"
 # endif
 
 # ifndef __cpp_lib_variant
 #   error "__cpp_lib_variant should be defined in c++2b"
 # endif
-# if __cpp_lib_variant != 201606L
-#   error "__cpp_lib_variant should have the value 201606L in c++2b"
+# if __cpp_lib_variant != 202102L
+#   error "__cpp_lib_variant should have the value 202102L in c++2b"
 # endif
 
 # ifndef __cpp_lib_void_t
 #   error "__cpp_lib_void_t should be defined in c++2b"
 # endif
 # if __cpp_lib_void_t != 201411L
 #   error "__cpp_lib_void_t should have the value 201411L in c++2b"
 # endif
 
 #endif // TEST_STD_VER > 20
 
 int main(int, char**) { return 0; }

libcxx/test/std/utilities/variant/variant.visit/visit.pass.cpp

void test_caller_accepts_nonconst() {
   struct A {};
   struct Visitor {
     void operator()(A&) {}
   };
   std::variant<A> v;
   std::visit(Visitor{}, v);
 }
 
+struct MyVariant : std::variant<short, long, float> {};
+
+namespace std {
+template <size_t Index>
+void get(const MyVariant&) {
+  assert(false);
+}
+} // namespace std
+
+void test_derived_from_variant() {
+  auto v1 = MyVariant{42};
+  const auto cv1 = MyVariant{142};
+  std::visit([](auto x) { assert(x == 42); }, v1);
+  std::visit([](auto x) { assert(x == 142); }, cv1);
+  std::visit([](auto x) { assert(x == -1.25f); }, MyVariant{-1.25f});
+  std::visit([](auto x) { assert(x == 42); }, std::move(v1));
+  std::visit([](auto x) { assert(x == 142); }, std::move(cv1));
+
+  // Check that visit does not take index nor valueless_by_exception members from the base class.
+  struct EvilVariantBase {
+    int index;
+    char valueless_by_exception;
+  };
+
+  struct EvilVariant1 : std::variant<int, long, double>,
+                        std::tuple<int>,
+                        EvilVariantBase {
+    using std::variant<int, long, double>::variant;
+  };
+
+  std::visit([](auto x) { assert(x == 12); }, EvilVariant1{12});
+  std::visit([](auto x) { assert(x == 12.3); }, EvilVariant1{12.3});
+
+  // Check that visit unambiguously picks the variant, even if the other base has __impl member.
+  struct ImplVariantBase {
+    struct Callable {
+      bool operator()();
+    };
+
+    Callable __impl;
+  };
+
+  struct EvilVariant2 : std::variant<int, long, double>, ImplVariantBase {
+    using std::variant<int, long, double>::variant;
+  };
+
+  std::visit([](auto x) { assert(x == 12); }, EvilVariant2{12});
+  std::visit([](auto x) { assert(x == 12.3); }, EvilVariant2{12.3});
+}
+
+struct any_visitor {
+  template <typename T>
+  void operator()(const T&) const {}
+};
+
+template <typename T, typename = decltype(std::visit(
+                          std::declval<any_visitor&>(), std::declval<T>()))>
+constexpr bool has_visit(int) {
+  return true;
+}
+
+template <typename T>
+constexpr bool has_visit(...) {
+  return false;
+}
+
+void test_sfinae() {
+  struct BadVariant : std::variant<short>, std::variant<long, float> {};
+  struct BadVariant2 : private std::variant<long, float> {};
+  struct GoodVariant : std::variant<long, float> {};
+  struct GoodVariant2 : GoodVariant {};
+
+  static_assert(!has_visit<int>(0));
+  static_assert(!has_visit<BadVariant>(0));
+  static_assert(!has_visit<BadVariant2>(0));
+  static_assert(has_visit<std::variant<int>>(0));
+  static_assert(has_visit<GoodVariant>(0));
+  static_assert(has_visit<GoodVariant2>(0));
+}
+
 int main(int, char**) {
   test_call_operator_forwarding();
   test_argument_forwarding();
   test_return_type();
   test_constexpr();
   test_exceptions();
   test_caller_accepts_nonconst();
+  test_derived_from_variant();
+  test_sfinae();
 
   return 0;
 }

libcxx/test/std/utilities/variant/variant.visit/visit_return_type.pass.cpp

auto test_lambda = [](int arg) constexpr {
     std::variant<int> v = 101;
     std::visit<void>([arg](int& x) constexpr { x = arg; }, v);
     return std::get<int>(v);
   };
 
   static_assert(test_lambda(202) == 202, "");
 }
 
+void test_derived_from_variant() {
+  struct MyVariant : std::variant<short, long, float> {};
+
+  std::visit<bool>(
+      [](auto x) {
+        assert(x == 42);
+        return true;
+      },
+      MyVariant{42});
+  std::visit<bool>(
+      [](auto x) {
+        assert(x == -1.3f);
+        return true;
+      },
+      MyVariant{-1.3f});
+
+  // Check that visit does not take index nor valueless_by_exception members from the base class.
+  struct EvilVariantBase {
+    int index;
+    char valueless_by_exception;
+  };
+
+  struct EvilVariant1 : std::variant<int, long, double>,
+                        std::tuple<int>,
+                        EvilVariantBase {
+    using std::variant<int, long, double>::variant;
+  };
+
+  std::visit<bool>(
+      [](auto x) {
+        assert(x == 12);
+        return true;
+      },
+      EvilVariant1{12});
+  std::visit<bool>(
+      [](auto x) {
+        assert(x == 12.3);
+        return true;
+      },
+      EvilVariant1{12.3});
+
+  // Check that visit unambiguously picks the variant, even if the other base has __impl member.
+  struct ImplVariantBase {
+    struct Callable {
+      bool operator()();
+    };
+
+    Callable __impl;
+  };
+
+  struct EvilVariant2 : std::variant<int, long, double>, ImplVariantBase {
+    using std::variant<int, long, double>::variant;
+  };
+
+  std::visit<bool>(
+      [](auto x) {
+        assert(x == 12);
+        return true;
+      },
+      EvilVariant2{12});
+  std::visit<bool>(
+      [](auto x) {
+        assert(x == 12.3);
+        return true;
+      },
+      EvilVariant2{12.3});
+}
+
+struct any_visitor {
+  template <typename T>
+  bool operator()(const T&) {
+    return true;
+  }
+};
+
+template <typename T, typename = decltype(std::visit<bool>(
+                          std::declval<any_visitor&>(), std::declval<T>()))>
+constexpr bool has_visit(int) {
+  return true;
+}
+
+template <typename T>
+constexpr bool has_visit(...) {
+  return false;
+}
+
+void test_sfinae() {
+  struct BadVariant : std::variant<short>, std::variant<long, float> {};
+
+  static_assert(has_visit<std::variant<int> >(int()));
+  static_assert(!has_visit<BadVariant>(int()));
+}
+
 int main(int, char**) {
   test_call_operator_forwarding<void>();
   test_argument_forwarding<void>();
   test_return_type<void>();
   test_constexpr_void();
   test_exceptions<void>();
   test_caller_accepts_nonconst<void>();
   test_call_operator_forwarding<int>();
   test_argument_forwarding<int>();
   test_return_type<int>();
   test_constexpr_int();
   test_exceptions<int>();
   test_caller_accepts_nonconst<int>();
   test_constexpr_explicit_side_effect();
+  test_derived_from_variant();
+  test_sfinae();
 
   return 0;
 }

libcxx/utils/generate_feature_test_macro_components.py

feature_test_macros = [ add_version_header(x) for x in [
     "values": { "c++17": 201411 },
     "headers": ["unordered_map"],
   }, {
     "name": "__cpp_lib_unwrap_ref",
     "values": { "c++20": 201811 },
     "headers": ["functional"],
   }, {
     "name": "__cpp_lib_variant",
-    "values": { "c++17": 201606 },
+    "values": { "c++17": 202102 },
     "headers": ["variant"],
   }, {
     "name": "__cpp_lib_void_t",
     "values": { "c++17": 201411 },
     "headers": ["type_traits"],
   }
 ]]