mirror of
https://github.com/facebook/sapling.git
synced 2024-10-06 23:07:18 +03:00
use rvalue-qual Future::ensure(): 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).ensure(...)` instead of `f.ensure(...)`. This syntactic change in the callsites forces callers to acknowledge the method's rvalue semantics. This diff started as a Codemod, then required manual fixes. Here were the codemod steps: * expr.ensure(...) ==> std::move(expr).ensure(...) // if expr is not already an xvalue * expr->ensure(...) ==> std::move(*expr).ensure(...) 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: D9332070 fbshipit-source-id: 882121fe82c05fdb196ce676db686b6bc254974b
This commit is contained in:
parent
de9416e41c
commit
dfac9dda4a
@ -432,7 +432,7 @@ void PrivHelper::setLogFileBlocking(folly::File logFile) {
|
||||
return;
|
||||
}
|
||||
|
||||
future.ensure([&evb] { evb.terminateLoopSoon(); });
|
||||
future = std::move(future).ensure([&evb] { evb.terminateLoopSoon(); });
|
||||
evb.loopForever();
|
||||
std::move(future).get();
|
||||
}
|
||||
|
@ -320,7 +320,8 @@ Future<InodePtr> TreeInode::getChildRecursive(RelativePathPiece path) {
|
||||
auto future = processor->next(inodePtrFromThis());
|
||||
// This ensure() callback serves to hold onto the unique_ptr,
|
||||
// and makes sure it only gets destroyed when the future is finally resolved.
|
||||
return future.ensure([p = std::move(processor)]() mutable { p.reset(); });
|
||||
return std::move(future).ensure(
|
||||
[p = std::move(processor)]() mutable { p.reset(); });
|
||||
}
|
||||
|
||||
InodeNumber TreeInode::getChildInodeNumber(PathComponentPiece name) {
|
||||
|
@ -216,7 +216,7 @@ TEST(FutureUnixSocket, receiveQueue) {
|
||||
});
|
||||
// Terminate the event loop after the final receive
|
||||
if (n == sendMessages.size() - 1) {
|
||||
future.ensure([&evb]() { evb.terminateLoopSoon(); });
|
||||
std::move(future).ensure([&evb]() { evb.terminateLoopSoon(); });
|
||||
}
|
||||
}
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user