[libcxx-commits] [PATCH] D85420: [libcxx/variant] Introduce `switch`-based mechanism for `std::visit`.

Tue Aug 11 09:23:05 PDT 2020

ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.

Geez, that's some nice speedup for smaller variants! Do you have an explanation for why the code is so much better?

================
Comment at: libcxx/include/variant:541
+    return
+#ifdef _LIBCPP_DISABLE_SWITCH_VISIT_IN_VARIANT
+        false;
----------------
I don't think the ability to disable the switch implementation is useful -- it'll just lead to an unused customization point. Or do you have a use case for it? That could provide interesting information about this patch.

================
Comment at: libcxx/test/libcxx/utilities/variant/variant.visit/visit_without_switch.pass.cpp:2
+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
----------------
Is there any difference between this file and `libcxx/test/std/utilities/variant/variant.visit/visit.pass.cpp` beyond the `#define _LIBCPP_DISABLE_SWITCH_VISIT_IN_VARIANT` bit? If not, we should structure the tests to avoid such duplication IMO.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85420/new/

https://reviews.llvm.org/D85420