Commit 35c96fe0 authored by Marshall Cline's avatar Marshall Cline Committed by Facebook Github Bot

use rvalue-qual Future::unwrap(): pass 2

Summary:
This is part of "the great r-valuification of folly::Future":

* This is something we should do for safety in general.
* Several of folly::Future's methods are lvalue-qualified even though they act as though they are rvalue-qualified, that is, they provide a postcondition that says, in effect, callers should act as though the method invalidated its `this` object (regardless of whether that invalidation was actual or logical).
* This violates the C++ principle to "Express ideas directly in code" (see Core Guidelines), and generally makes it more confusing for callers as well as hiding the actual semantics from tools (linters, compilers, etc.).
* This dichotomy and confusion has manifested itself by some failures around D7840699 since lvalue-qualification hides that operation's move-out semantics - leads to some use of future operations that are really not correct, but are not obviously incorrect.
* The goal of rvalueification is to make sure methods that are logically rvalue-qualified are actually rvalue-qualified, which forces callsites to acknowledge that rvalueification, e.g., `std::move(f).unwrap(...)` instead of `f.unwrap(...)`. This syntactic change in the callsites forces callers to acknowledge the method's rvalue semantics.

Codemod changes:

* expr.unwrap(...) ==> std::move(expr).unwrap(...)  // if expr is not already an xvalue
* expr->unwrap(...) ==> std::move(*expr).unwrap(...)

Note: operator precedence of that last step is safe - no need to parenthesize `expr`. Reason: `->` binds more tightly than unary `*`.

Reviewed By: LeeHowes

Differential Revision: D9350632

fbshipit-source-id: 54806eb982cf22c4c29d326d8ba894837125f291
parent c670567d
......@@ -30,7 +30,7 @@ TEST(Unwrap, simpleScenario) {
// Makes sure that unwrap() works when chaning Future's commands.
TEST(Unwrap, chainCommands) {
Future<Future<int>> future = makeFuture(makeFuture(5484));
auto unwrapped = future.unwrap().then([](int i){ return i; });
auto unwrapped = std::move(future).unwrap().then([](int i) { return i; });
EXPECT_EQ(5484, unwrapped.value());
}
......@@ -40,7 +40,7 @@ TEST(Unwrap, chainCommands) {
TEST(Unwrap, futureNotReady) {
Promise<Future<int>> p;
Future<Future<int>> future = p.getFuture();
Future<int> unwrapped = future.unwrap();
Future<int> unwrapped = std::move(future).unwrap();
// Sanity - should not be ready before the promise is fulfilled.
ASSERT_FALSE(unwrapped.isReady());
// Fulfill the promise and make sure the unwrapped future is now ready.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment