[libcxx-commits] [PATCH] D97394: [libc++] [C++2b] [P2162] Allow inheritance from std::variant.

Arthur O'Dwyer via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Wed Mar 24 16:53:15 PDT 2021

Quuxplusone accepted this revision as: Quuxplusone.
Quuxplusone added a comment.

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.

Comment at: libcxx/include/variant:340
+  return _VSTD::move(__vs);
Minor bikeshed: `&` `const&` `const&&` `&&` is a weird order to put these four overloads in :)

Comment at: libcxx/test/std/language.support/support.limits/support.limits.general/version.version.pass.cpp:4107
+# if defined(__cpp_concepts) && __cpp_concepts >= 201907L
 #   ifndef __cpp_lib_math_constants
Make sure to re-run the generator scripts before landing. This diff //should// go away, I think.

Comment at: libcxx/test/std/utilities/variant/variant.visit/visit.pass.cpp:421
+  static_assert(!has_visit<BadVariant2>(0));
+  static_assert(has_visit<std::variant<int> >(0));
+  static_assert(has_visit<GoodVariant>(0));
nit: `>>` is OK here

Comment at: libcxx/test/std/utilities/variant/variant.visit/visit.pass.cpp:357
+  // Check that visit does not take index nor valueless_by_exception members from the base class.
+  struct EvilVariantBase {
curdeius wrote:
> Quuxplusone wrote:
> > 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?
> Not sure if I correctly understood what test you suggested. Could you have a look please?
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. :)

  rG LLVM Github Monorepo



More information about the libcxx-commits mailing list