1418624 - fix Result::unwrap to be less terrible

Status: RESOLVED FIXED

(Core :: MFBT, enhancement)

Description

[Nathan Froyd [:froydnj]]

Bug 1417101 introduces code like:

Result<Maybe<Foo>, nsresult> DoSomething(...);

Where Foo is rather large, about 20 words.  The above is then used in code:

Result<Maybe<Foo>, nsresult> r = DoSomething(...);

if (r.isErr()) {
  return;
}

Maybe<Foo> f = r.unwrap();

Result<V, E>::unwrap() returns V by value in all cases, even when it might make sense to return V&.  (Some of the underlying implementations of Result<V, E> *have* to return V by value, but several of them could just as well return V&.)  For code like the above, this causes a lot of needless copying of Foo.

Granted, even if unwrap() returned V&, the above code could still be written, and that would be unfortunate.  But as it stands, the user *has* to do the inefficient thing; we should be giving the user a fighting chance to write efficient code.

Perhaps Result should have something like Variant::extract, where it's more obvious that we're returning a (ideally moved) value, and then we should change unwrap() to return references where possible?

Comment 2

[Nathan Froyd [:froydnj]]

Alternative idea: r-value ref-qualify unwrap(), so people have to write std::move(result).unwrap(), so our use-after-move analysis stops you from re-using result after unwrap()?

Comment 3

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1418624 - Allow mozilla::Result to be moved, make unwrap{,Err}() move, and add inspect() APIs that return references.

Also adjust some of the callers that were either calling unwrap() repeatedly on
the same result, or were doing silly copies, to use inspect().

We could try to use stuff like:

https://clang.llvm.org/docs/AttributeReference.html#consumed-annotation-checking

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9bc4747798bc
Allow mozilla::Result to be moved, make unwrap{,Err}() move, and add inspect() APIs that return references. r=froydnj

Comment 5

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/9bc4747798bc