[PATCH] D99560: Utility to construct visitors from lambdas.

Tue Apr 6 15:30:15 PDT 2021

scott.linder added a comment.

In D99560#2667272 <https://reviews.llvm.org/D99560#2667272>, @ezhulenev wrote:

> `std::forward` with inheritance does not remove a need to invoke a copy constructors, I managed to implement `overloaded` using a struct member and super ugly SFINAE that does properly captures lambdas by reference when lambda is a lvalue, however it still does need to copy when lambda is rvalue: https://godbolt.org/z/dqshPW6h1 (top most example, I just count the number of `HeavyStuff::HeavyStuff(HeavyStuff const&)` copy constructor calls in the generated code). But it does look much worse than inheritance.

As you mention, this version doesn't work for e.g. move-only lambdas, see https://godbolt.org/z/T659nM5fc (I essentially just made `HeavyStuff` move-only, and captured it in both lambdas via move).

I think by making the members universal references we can support move-only lambda types. I tried to make the necessary changes in https://godbolt.org/z/9YTq9d6r4 and it seems to be compiling. I'm still not confident it is completely correct, and I imagine there are some additional `operator() const` overloads needed, as well as marking things like the constructors `constexpr` where applicable, but I think in principle we should be able to do perfectly-forwarded `makeVisitor`.

As a note, I made some other changes to make this simpler, including using derivation to replace the `ov` member, as otherwise it is harder to force the `ov` member to be a universal reference type (because for some deduced type `T` and some concrete type template `foo`: `T && m;` is a universal reference type and `foo<T> && m;` is just an rvalue reference). I also think I avoided some of the extra SFINAE parameters needed by using `-> decltype(...)`, but I'm not sure this is completely correct either?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99560