1500811 - Maybe and Variant should provide rvalue-qualified accessors

Status: RESOLVED FIXED

(Core :: MFBT, enhancement)

Description

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

Currently Maybe::operator*(), Variant::as<U>(), and other similar accessors don't have different implementations for rvalue-qualified `this` objects, which means that if the Maybe object is an rvalue (e.g., returned from a function, or explicitly move()'d), its contained object is not efficiently moved when possible.

As a quick exercise, I tried `Maybe::operator*() && = delete;` to check if we had such cases, and the build quickly failed with a few errors.
I did not examine the code further, maybe all these objects are small&simple enough that copying is just as good as moving? But it at least shows that this situation exists.

And from what I've read, the `const &&` option should be implemented as well, as it could happen in some situations. Note that it is provided in std::optional: https://en.cppreference.com/w/cpp/utility/optional/operator*

Comment 1

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

The cppreference link above doesn't work (Bugzilla doesn't include the final '*' -- I've opened bug 1500883 to see if it should&could be fixed).

In the meantime this one should work:
https://en.cppreference.com/w/cpp/utility/optional/operator%2A

Comment 2

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

Attachment: Bug 1500811 - Maybe::operator*()&&, ref()&&, value()&& - r?sg

There are also equivalents for const&&, which should be rare.

The change in SavedStacks.cpp was necessary because cache->insert takes an lvalue-ref, but *lookup.framePtr() now returns an rvalue-ref (as it should).

Comment 3

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

Attachment: Bug 1500811 - Variant::as()&&, match()&& - r?sg

as<...>() on an rvalue variant returns the contained value as rvalue-reference.
match() on an rvalue variant forwards the contained value as rvalue-reference.
So now the contained values can be efficiently moved-from.

There are also equivalents for const&&, which should be rare.

Depends on D98049

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a8f88652ea6d
Maybe::operator*()&&, ref()&&, value()&& - r=sg,sfink
https://hg.mozilla.org/integration/autoland/rev/273d6825ba47
Variant::as()&&, match()&& - r=sg

Comment 5

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/a8f88652ea6d
https://hg.mozilla.org/mozilla-central/rev/273d6825ba47