Closed Bug 1429613 Opened 2 years ago Closed 9 months ago

Variant::match accepting lambdas, or one generic lambda


(Core :: MFBT, enhancement, P3)




Tracking Status
firefox59 --- wontfix
firefox68 --- fixed


(Reporter: gerald, Assigned: jorendorff)


(Depends on 1 open bug)



(3 files, 1 obsolete file)

Currently the recommended way to handle all possibly options of a Variant is through the Variant::match() function, which takes a single struct that must contain a `match(T&)` function for each option.
This can be a bit heavy and difficult to read, separating the Variant::match call from the actions:
> Variant<A,B,C> vabc;
> struct Matcher
> {
>   char* match(A&) {...}
>   char* match(B&) {...}
>   char* match(C&) {...}
> };
> vabc.match(Matcher());

It would be great if instead we could call a matcher function directly with a bunch of lambdas, e.g.:
> vabc.fmatch(
>   [](A&) {...},
>   [](B&) {...},
>   [](C&) {...});

It could also accept one C++14 generic lambda, e.g.:
> vabc.fmatch([](auto&&) {...});
Blocks: 1429616
Seth and/or fitzgen talked about this when Variant was first introduced, but it wasn't really practical then.  I'd be in favor of supporting multiple lambdas, which doesn't seem *too* hard.  I think boost::variant supported multiple lambdas in any order (!), but let's require the lambdas to be matched in the order of the types declared in the Variant.  If people really don't like that, we can try the more complicated scheme.

Bonus points would be converting .match() to the lambda form everywhere, which is probably clearer.
Priority: -- → P3
Depends on: 1487933

Rebasing this stack. One patch done and building. Easy going so far.

Assignee: nobody → jorendorff

Mechanical change from Matcher::match(...) to Matcher::operator()(...).
This will now permit the use of generic lambdas, and facilitate the
implementation of multi-lambda match.

Mmmmaybe land this?

I spent a couple hours yesterday morning implementing a Variant::fmatch method template from scratch, very similar to this stack only without the renaming and otherwise worse in a couple ways. Then I searched Bugzilla.

Attachment #9053619 - Attachment description: Bug 1429613 - Add `Variant<Ts...>::fmatch(F...)`, an alternative to match() that takes lambdas. → (alternate approach, for the record) Bug 1429613 - Add `Variant<Ts...>::fmatch(F...)`, an alternative to match() that takes lambdas.

The patch uploaded in comment 8 is how it would look the other way (adding .fmatch() without changing the existing .match() template), stealing a few good ideas from Gerald's patches.

I like Gerald's stack better anyway.

Pushed by
Variant matcher callbacks renamed from `match` to `operator()` - r=froydnj
Variadic Variant::match, takes one function object per option - r=froydnj
Using upgraded Variant::match where appropriate - r=froydnj
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Attachment #9053619 - Attachment description: (alternate approach, for the record) Bug 1429613 - Add `Variant<Ts...>::fmatch(F...)`, an alternative to match() that takes lambdas. → Bug 1429613 - Add `Variant<Ts...>::fmatch(F...)`, an alternative to match() that takes lambdas.
Attachment #9053619 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.