folly::fibers: do not move out of lvalue references
authorSven Over <over@fb.com>
Mon, 28 Mar 2016 21:13:24 +0000 (14:13 -0700)
committerFacebook Github Bot 6 <facebook-github-bot-6-bot@fb.com>
Mon, 28 Mar 2016 21:20:22 +0000 (14:20 -0700)
Summary:If you pass an lvalue to folly::fibers::Fiber::setFunction or
setFunctionFinally, you do not expect that those methods move
the function out of your lvalue.

This diff uses std::forward for perfect forwarding, so if an lvalue
is passed, a copy is made.

If and when when start using folly::Function for storing the
functions in the Fiber object, passing an lvalue will trigger a
compiler error because folly::Function is not copyable. That is
a good thing, as it enforces calling setFunction with std::move
if you use a named object, making clear that the named object
gets moved out of. Often people will pass temporary objects
(like a lambda defined i- place), which a rvalues anyway, so
this will not be a problem anyway.

Reviewed By: andriigrynenko

Differential Revision: D3102685

fb-gh-sync-id: 87bf3135f7f6630766f97be351599ff488e4b796
fbshipit-source-id: 87bf3135f7f6630766f97be351599ff488e4b796

folly/experimental/fibers/Fiber-inl.h
folly/experimental/fibers/FiberManager-inl.h

index 5366e6860c49e04cef458731895259305d16c047..68ee253e6a37bc4dc423f362215a77a0643d4ebe 100644 (file)
@@ -22,7 +22,7 @@ namespace folly { namespace fibers {
 template <typename F>
 void Fiber::setFunction(F&& func) {
   assert(state_ == INVALID);
-  func_ = std::move(func);
+  func_ = std::forward<F>(func);
   state_ = NOT_STARTED;
 }
 
@@ -30,8 +30,8 @@ template <typename F, typename G>
 void Fiber::setFunctionFinally(F&& resultFunc,
                                G&& finallyFunc) {
   assert(state_ == INVALID);
-  resultFunc_ = std::move(resultFunc);
-  finallyFunc_ = std::move(finallyFunc);
+  resultFunc_ = std::forward<F>(resultFunc);
+  finallyFunc_ = std::forward<G>(finallyFunc);
   state_ = NOT_STARTED;
 }
 
index 1e6ed03583734125e8482f66a50066e962ab6dcf..ff40265e76af2d83f3a9b12e44437440de2482f3 100644 (file)
@@ -355,7 +355,7 @@ struct FiberManager::AddTaskFinallyHelper {
    public:
     Finally(G&& finally,
             FiberManager& fm) :
-        finally_(std::move(finally)),
+        finally_(std::forward<G>(finally)),
         fm_(fm) {
     }
 
@@ -430,12 +430,13 @@ void FiberManager::addTaskFinally(F&& func, G&& finally) {
     auto finallyLoc = static_cast<typename Helper::Finally*>(
       static_cast<void*>(funcLoc + 1));
 
-    new (finallyLoc) typename Helper::Finally(std::move(finally), *this);
+    new (finallyLoc) typename Helper::Finally(std::forward<G>(finally), *this);
     new (funcLoc) typename Helper::Func(std::move(func), *finallyLoc);
 
     fiber->setFunctionFinally(std::ref(*funcLoc), std::ref(*finallyLoc));
   } else {
-    auto finallyLoc = new typename Helper::Finally(std::move(finally), *this);
+    auto finallyLoc =
+        new typename Helper::Finally(std::forward<G>(finally), *this);
     auto funcLoc = new typename Helper::Func(std::move(func), *finallyLoc);
 
     fiber->setFunctionFinally(std::ref(*funcLoc), std::ref(*finallyLoc));