On Mon, Aug 12, 2019 at 11:16:17PM +0200, Emilio Cobos Álvarez wrote:
On 8/12/19 10:38 PM, Bryce Seager van Dyk wrote:
On Monday, August 12, 2019 at 8:41:26 AM UTC-7, Emilio Cobos Álvarez wrote:
Neat! Thanks for doing this. It should allow for broader use of Result in media
code.
Are there any footguns to watch out for when moving into the Result? Looking at the
changes it appears that if my return type is Result<BigStruct> then I can just
return a BigStruct? I.e. I don't need to do explicitly invoke anything else to avoid
unintended copies.
I think you need to use `return std::move(bigStruct);`, since
otherwise you peek the `const T&` constructor rather than the `T&&`
constructor. Before my patch, you always get a copy when returning it
(because Result only took const references).
With my patch, if you `return bigStruct;`, your code will compile and
do the same copy, but you'll get a warning from the compiler with
-Wreturn-std-move, such as:
Note that if you construct the returned struct in the return
statement with something like:
return BigStruct {
...,
};
it should still invoke the Result<> constructor with move
semantics. But either way, that's going to result in a copy when
the Result is constructed (unless the compiler is really clever).
The only way to avoid the copy altogether would be to create the
result with a default initialized value, fill it in, and then
return it without an std::move. Which is easier said than done.
It might make sense to add a `.emplace()` method to make that
easier...
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
