1645328 - Add in-place constructor to Maybe

Status: RESOLVED FIXED

(Core :: MFBT, enhancement)

Description

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

The only way to construct a Maybe with a value is through Some(...), which requires the type to be copyable or movable.
This prevents some use cases, like storing a non-copyable non-movable object.

To allow this, we could add an in-place constructor, similar to template <typename... Args> std::optional::optional(std::in_place_t, Args&&...).

Comment 1

[Simon Giesecke [:sg] [he/him]]

I stumbled upon that a few times as well.

We essentially have the machine for that already through Maybe::emplace, we probably just need to add constructor that calls that.

Comment 2

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

emplace assumes (and even MOZ_DIAGNOSTIC_ASSERTs) that we already have Maybe in the default Nothing state (so mIsSome is set to false first), I was hoping the in-place constructor would skip that step and initialize all members to their final values at once.

Comment 3

[Simon Giesecke [:sg] [he/him]]

Yes, that's right. The original initialization and the MOZ_DIAGNOSTIC_ASSERT might be typically optimized away though? But to be on the "safe" side, the cosntruction could be done right away, yes.

Comment 4

[Simon Giesecke [:sg] [he/him]]

Attachment: Bug 1645328 - Add in-place constructor to Maybe. r=froydnj

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5200dd8a0de5
Add in-place constructor to Maybe. r=froydnj

Comment 6

[Cristian Brindusan [:cbrindusan]]

Backed out changeset 5200dd8a0de5 (Bug 1645328) for cppunit assertion failures at Maybe.h.
https://hg.mozilla.org/integration/autoland/rev/59d491706a9a143c4e27b9147c2257a4b10755f8

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=5200dd8a0de590d710ec23555e65d77d7be6495d&selectedTaskRun=HublIoOUQgyAVv7Ustx7Zg.0

Failure log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=306139440&repo=autoland&lineNumber=1576

Comment 7

[Simon Giesecke [:sg] [he/him]]

I fixed the patch now and am going to re-land it. Sorry, I introduced this defect by some last-minute changes :(

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9497e6381139
Add in-place constructor to Maybe. r=froydnj

Comment 9

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/9497e6381139