Provide Convenient Lambda Overload Set

[daniel-j-h]

Suppose you're using the common Either sumtype, providing a choice in its type constructors

template <typename Left, typename Right>
using Either = variant<Left, Right>;

and your API returns either a struct Error {}; or a struct Response {}; type as in Either<Error, Response>. Providing a visitor for this variant you have to write the following handler

struct Matcher {
  std::string operator()(Response) const { return "ok"; }
  std::string operator()(Error) const { return "error"; }
};

Matcher match;
apply_visitor(match, rv);

Instead, what about:

match([] (Response) { return "ok"; },
      [] (Error)    { return "error"; });

The idea is to inherit from the lambda's anonymous type pulling in its operator()

template <typename Fn>
struct Matcher : Fn {
  using Fn::operator();
};

template <typename Fn>
typename Matcher<Fn>::type match(Fn fn) {
  return Matcher<Fn>(fn);
}

This gets even better once you realize you no longer have to manually add member attributes and write constructors for your handle types. Simple lambda captures are the equivalent

match([&] (Response) { responses += 1; },
      [&] (Error e)  { log_error(e); });

Here's a proof on concept. It probably needs some more time, refinements and eyes to make this production ready, but you get the idea: https://gist.github.com/daniel-j-h/04efdc54dae50c1b9bff688ec354e693

Discussion:

• match is a bad / overloaded name, not set in stone
• can / should this be a free standing function / variant member function / included in variant at all
• should match only create the visitor or combine creating and applying the visitor instead

cc @kkaefer @artemp @joto

[artemp]

@daniel-j-h - Great idea ^ ! How about standalone function in variant_lambda.hpp header so it can be optionally included by client code ?

[kkaefer]

Why an extra file? It's literally 20 lines of code?

[artemp]

@kkaefer - variant.hpp is getting quite large and not everyone will use lambdas. This is why we have variant_io.hpp,recursive_wrapper.hpp etc. Modularity rather than encapsulation !

[kkaefer]

Fair point, but then we also should move the visitor functions to an extra file?

[daniel-j-h]

Any updates on the issues raised above? I still think it would be great to get this in. Open questions:

• [ ] separate file vs. variant.hpp
• [ ] decide on a name (match?)
• [ ] free standing function vs member function
• [ ] similarly, should match only create a visitor or already apply it?

[artemp]

@daniel-j-h

• separate file
• your call, match sounds reasonable to me
• free standing function always better ( as in boost design philosophy)
• ?? pls give an example

PR is most welcome!

[daniel-j-h]

auto v = match([](Left){},
               [](Right){});

apply_visitor(v, my_variant);

vs

match(v, [](Left){},
         [](Right){});

[artemp]

@daniel-j-h : can we have both ? :)

[lightmare]

D'oh! I've been working on this in august but then got totally distracted and forgot to comment. I even had questions regarding current interface but I'll have to go through the code to recall what they were.

My suggestions:

• in variant.hpp, because
  • it's just a few lines, that's nothing added to thousands of lines included from standard headers; parsing templates is not hard, instantiation is
  • this is not a question of modularity, it's essentially syntax sugar; it should be available right off the bat
• free function make_visitor
• visit member function overload for 2+ args, that will visit(make_visitor(...))

[lightmare]

One of the questions about current interface: why is variant::visit static?

[artemp]

why is variant::visit static?

Because variant::visit doesn't modify internal state and making it static gives compiler some hints on how to generate binary code. Why are you thinking it shouldn't be static ?

[artemp]

this is not a question of modularity, it's essentially syntax sugar; it should be available right off the bat

Well, the fact it's a syntactic sugar and not a part of core interface makes strong argument towards a separate header. In principal, I'd like to see more boost like approach to organising header-only libraries. We can include <detail/match.hpp> in <variant.hpp> as an experiment to see if compile/parse times are acceptable but not the other way around !

[lightmare]

To me that sounds like having unique_ptr and make_unique in different headers.

I see this proposal as the way to write quick one-off visitors, not some outlandish feature that deserves a usage barrier. If there's something that deserves being moved into a separate header, then it's binary visitor.

Anyway, if you want to chop variant into smaller pieces, then I prefer having variant.hpp include all of them rather than an arbitrary subset.

edit: here's my take on @daniel-j-h's original proposal.

[artemp]

Anyway, if you want to chop variant into smaller pieces, then I prefer having variant.hpp include all of them rather than an arbitrary subset.

^ This is exactly my intention and we need to start somewhere - this is a good place to start.

[artemp]

@lightmare - +1 for re-factoring binary_visitor into separate header

[artemp]

This is a good feature and it feels like a good time to get a consensus and move on this :).

@daniel-j-h do you have any feedback re: lightmare's take ? I like make_visitor interface. Also, you added MIT license to the original gist while variant is BSD - just wondering.

To summarise: this issue needs a PR we can all work on to get this into the next release.

/cc @springmeyer @daniel-j-h @lightmare

[daniel-j-h]

No reason for the MIT vs BSD license - some users pinged me via mail and wanted to know under which conditions they can just drop it into their projects.

Also a user reported my original proof of concept (Gist linked above) not properly handling degenerate variants (variants with a single type). I just updated the Gist adding the missing constructor for the recursion's base-case.

We should have tests for edge cases like this, for sure.

@lightmare's take looks good overall - we could take the functions as forwarding references and std::forward them on, e.g. here.

[lightmare]

we could take the functions as forwarding references and std::forward them on

I don't remember why I made it take arguments by value. Should've made a comment, I guess ;) Maybe it was because I only wanted to show how it'd work, without unnecessary clutter of proper implementation. Or maybe I thought it wasn't necessary.

If make_visitor and unary_visitor constructor take arguments by value, and only std::move when passing them, they will be copied/moved into make_visitor, then moved into unary_visitor constructor, then moved into its bases.

On the other hand, if make_visitor takes forwarding references, unary_visitor has to inherit from std::decayed functions, and the copying/moving happens in base class constructors. So you avoid two moves that I think the compiler shouldn't have trouble optimizing away.