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

Tue Aug 11 13:33:13 PDT 2020

ldionne added a comment.

In D85420#2211176 <https://reviews.llvm.org/D85420#2211176>, @mpark wrote:

> 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`.

Interesting.

================
Comment at: libcxx/include/variant:541
+    return
+#ifdef _LIBCPP_DISABLE_SWITCH_VISIT_IN_VARIANT
+        false;
----------------
mpark wrote:
> 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?
I think if we test the overall `std::visit` with all sizes of variants, then we'll be exercising both the switch and the manual vtable implementations to the extent they are used. I believe that should be sufficient, and it would also remove the duplication in the tests IIUC. What do you think about that?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85420