Overload Optional::value() on object reference type.
authorNick Terrell <terrelln@fb.com>
Mon, 20 Jul 2015 16:03:11 +0000 (09:03 -0700)
committerSara Golemon <sgolemon@fb.com>
Mon, 20 Jul 2015 19:26:32 +0000 (12:26 -0700)
commit1470961d6cb80e5f2dff29eff0254d5d55b9267d
tree95f4a7de2183ba5d286901c0c5be688f0cc9f894
parentad7e7f72235d3803b0077c7acbd082c79eb99709
Overload Optional::value() on object reference type.

Summary: `folly::Optional` returns the stored value by reference when the object
is an rvalue.  This causes three issues:

  * If the user calls `value()` on an rvalue `Optional`, and assigns the result
    to a new variable, the copy constructor gets called, instead of the move
    constructor.  This causes the added test `value_move` to not compile.
  * If the user calls `value()` on an rvalue `Optional`, and assigns the result
    to a const lvalue reference, they might expect the lifetime to be extended
    when it isn't. See the added test `value_life_extention`.
  * Assigning the results of `value()` on an rvalue `Optional` to a mutable
    lvalue reference compiled in the old code, when it shouldn't, because that
    is always a dangling reference as far as I can tell.

I'm not sure how strict `folly` is with compatibility, but I believe the
breakage would be minimal, and any code that gets broken probably deserves it.

I'm not exactly sure who I should add as a reviewer, so hopefully herald has got
my back.

Reviewed By: @yfeldblum

Differential Revision: D2249548
folly/Optional.h
folly/test/OptionalTest.cpp