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

use rvalue-qual Future::within(...); pass 5

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).within(...)` instead of `f.within(...)`. This syntactic change in the callsites forces callers to acknowledge the method's rvalue semantics.

Codemod changes:

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

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

Reviewed By: yfeldblum

Differential Revision: D9511943

fbshipit-source-id: 0cf90e3198453dd997194c2c7f36cc1fa74906f5
parent 2e00dff8
...@@ -188,7 +188,7 @@ TEST(AsyncSSLSocketTest2, AttachDetachSSLContext) { ...@@ -188,7 +188,7 @@ TEST(AsyncSSLSocketTest2, AttachDetachSSLContext) {
auto f = client->getFuture(); auto f = client->getFuture();
client->connect(); client->connect();
EXPECT_TRUE(f.within(std::chrono::seconds(3)).get()); EXPECT_TRUE(std::move(f).within(std::chrono::seconds(3)).get());
} }
class ConnectClient : public AsyncSocket::ConnectCallback { class ConnectClient : public AsyncSocket::ConnectCallback {
...@@ -255,7 +255,7 @@ TEST(AsyncSSLSocketTest2, TestTLS12DefaultClient) { ...@@ -255,7 +255,7 @@ TEST(AsyncSSLSocketTest2, TestTLS12DefaultClient) {
auto c1 = std::make_unique<ConnectClient>(); auto c1 = std::make_unique<ConnectClient>();
auto f1 = c1->getFuture(); auto f1 = c1->getFuture();
c1->connect(server.getAddress()); c1->connect(server.getAddress());
EXPECT_TRUE(f1.within(std::chrono::seconds(3)).get()); EXPECT_TRUE(std::move(f1).within(std::chrono::seconds(3)).get());
} }
TEST(AsyncSSLSocketTest2, TestTLS12BadClient) { TEST(AsyncSSLSocketTest2, TestTLS12BadClient) {
...@@ -275,7 +275,7 @@ TEST(AsyncSSLSocketTest2, TestTLS12BadClient) { ...@@ -275,7 +275,7 @@ TEST(AsyncSSLSocketTest2, TestTLS12BadClient) {
c2->setCtx(clientCtx); c2->setCtx(clientCtx);
auto f2 = c2->getFuture(); auto f2 = c2->getFuture();
c2->connect(server.getAddress()); c2->connect(server.getAddress());
EXPECT_FALSE(f2.within(std::chrono::seconds(3)).get()); EXPECT_FALSE(std::move(f2).within(std::chrono::seconds(3)).get());
} }
} // namespace folly } // namespace folly
......
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