1621865 - Variant::match() to also accept 2-arg lambda that takes the active index as well

Status: RESOLVED FIXED

(Core :: MFBT, enhancement)

Description

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

std::variant provides index() to get the 0-based index of the type currently stored in the variant.

I would like the same function in our mozilla::Variant.

I would use it to simplify and optimize serialization code in the profilers.

Comment 1

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1621865 - Variant::index() returns the 0-based index of the alternative that is currently held - r?froydnj

Comment 2

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Changing direction. Extracts from the conversation in https://phabricator.services.mozilla.com/D66542 :

froydnj:

I believe we have resisted adding index() or similar because we don't want people using switch (index()) or similar. We'd rather they use match() so that the compiler can do some exhaustiveness checking. tag() exists, but that basically exists for the purpose of IPDL serialization and, you'll note, is not exposed to the outside world.
If you want to add a friend declaration for your serialization code, I think that'd be fine, but in the absence of better information, I think we'd prefer to not expose index().

gerald:

How about an option for match() to accept a 2-argument generic lambda that takes both the tag and the correctly-typed reference?
E.g.: v.match([](size_t aIndex, const auto& aAlternative) { ... });
(The 1-arg would still work like before, of course.)
This would remove all the useless calls with the wrong alternatives.

froydnj:

Let's go with the proposed match variant; that might even let us remove the friend declarations in Variant, which would be a nice bonus.

Comment 3

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1621865 - In Variant::match(), forward the callback before invoking it - r?froydnj

std::forward(invocable)() ensures that the optimal callback is invoked, in
case there are different versions with different reference qualifiers.
E.g., an rvalue-reference-qualified member function could std::move() from class
members, because the object is about to be destroyed anyway.

Comment 4

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1621865 - Variant matchers may optionally take the current index as first parameter - r?froydnj

Depends on D66719

Comment 5

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

froydnj:

Let's go with the proposed match variant; that might even let us remove the friend declarations in Variant, which would be a nice bonus.

I had a look at the IPC classes, and unfortunately they still need access to the private Tag type when deserializing, so we can't get rid of the friend declarations -- yet!
(We could possibly make the Tag type public, but I didn't pursue further at this time; feel free to spawn another bug, or I make come back to it if one day I want to optimize my own deserialization; but it's not as critical in the profiler, so please don't wait for me.)

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6b56b8123441
In Variant::match(), forward the callback before invoking it - r=froydnj
https://hg.mozilla.org/integration/autoland/rev/59ff595abeed
Variant matchers may optionally take the current index as first parameter - r=froydnj

Comment 7

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/6b56b8123441
https://hg.mozilla.org/mozilla-central/rev/59ff595abeed