From: Sven Over <over@fb.com>
Date: Sun, 2 Oct 2016 17:22:13 +0000 (-0700)
Subject: Fix folly::Partial copy/move construction
X-Git-Tag: v2016.10.03.00~1
X-Git-Url: http://plrg.eecs.uci.edu/git/?a=commitdiff_plain;h=806a09e4bc1624dadee3622e9acbd4b3fa5cf5af;p=folly.git

Fix folly::Partial copy/move construction

Summary:
Any attempt to copy or move an object returned by folly::partial
yields a compiler error because it is invoking the constructor
for wrapping callable objects, which triggers a type mismatch.

This diff fixes that by explicitly naming the default implementations
of copy and move constructors.

This diff also adds additional tests that fail to compile without this fix.

And then this diff also moves the Partial class into folly::detail,
because it is not meant to be named in user code, but only returned
by the folly::partial function.

Reviewed By: mhx

Differential Revision: D3923809

fbshipit-source-id: a8883951afd2a1999acbfffc51296393b058f860
---

diff --git a/folly/Partial.h b/folly/Partial.h
index 248a86a9..460df9a5 100644
--- a/folly/Partial.h
+++ b/folly/Partial.h
@@ -20,6 +20,13 @@
 
 namespace folly {
 
+namespace detail {
+namespace partial {
+
+// helper type to make sure that the templated constructor in Partial does
+// not accidentally act as copy or move constructor
+struct PartialConstructFromCallable {};
+
 template <typename F, typename Tuple>
 class Partial {
  private:
@@ -28,7 +35,7 @@ class Partial {
 
  public:
   template <typename Callable, typename... Args>
-  Partial(Callable&& callable, Args&&... args)
+  Partial(PartialConstructFromCallable, Callable&& callable, Args&&... args)
       : f_(std::forward<Callable>(callable)),
         stored_args_(std::forward<Args>(args)...) {}
 
@@ -69,6 +76,9 @@ class Partial {
   }
 };
 
+} // namespace partial
+} // namespace detail
+
 /**
  * Partially applies arguments to a callable
  *
@@ -92,10 +102,12 @@ class Partial {
  * and passed to the original callable.
  */
 template <typename F, typename... Args>
-auto partial(F&& f, Args&&... args) -> Partial<
+auto partial(F&& f, Args&&... args) -> detail::partial::Partial<
     typename std::decay<F>::type,
     std::tuple<typename std::decay<Args>::type...>> {
-  return {std::forward<F>(f), std::forward<Args>(args)...};
+  return {detail::partial::PartialConstructFromCallable{},
+          std::forward<F>(f),
+          std::forward<Args>(args)...};
 }
 
 } // namespace folly
diff --git a/folly/test/PartialTest.cpp b/folly/test/PartialTest.cpp
index 1d19e020..cbe1851b 100644
--- a/folly/test/PartialTest.cpp
+++ b/folly/test/PartialTest.cpp
@@ -16,6 +16,7 @@
 
 #include <memory>
 
+#include <folly/Function.h>
 #include <folly/Partial.h>
 
 #include <folly/portability/GTest.h>
@@ -132,3 +133,15 @@ TEST(Partial, MoveOnly) {
 
   EXPECT_EQ(560, *result);
 }
+
+TEST(Partial, WrapInStdFunction) {
+  auto p1 = partial(&add3, 2);
+  std::function<int(int, int)> func = p1;
+  EXPECT_EQ(234, func(3, 4));
+}
+
+TEST(Partial, WrapInFollyFunction) {
+  auto p1 = partial(&add3, 2);
+  folly::Function<int(int, int)> func = p1;
+  EXPECT_EQ(234, func(3, 4));
+}