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

[Michael Park via Phabricator via libcxx-commits]

mpark added a comment.

The improvements are from enabling compilers to inline the invocation by using a `switch`.
Compilers currently can't seem to inline through function pointers even if it's `constexpr`.



================
Comment at: libcxx/include/variant:541
+    return
+#ifdef _LIBCPP_DISABLE_SWITCH_VISIT_IN_VARIANT
+        false;
----------------
ldionne wrote:
> 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.
The only real use case is fully testing the manual vtable approach. Perhaps we can do that through a different mechanism?


================
Comment at: libcxx/test/libcxx/utilities/variant/variant.visit/visit_without_switch.pass.cpp:2
+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
----------------
ldionne wrote:
> 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.
No, there's no difference. I was wondering if there's something I could do with `ADDITONAL_COMPILE_FLAGS` or something but I couldn't find anything. Do you have a suggestion?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85420