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 248a86a987a3f9be64bb52aa5e5eb2d4ba3a0af2..460df9a5875e884ec532c352da9371b0aaba413f 100644 (file)
--- 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 1d19e020d3734b67bd9899d10c2f4419d5089f17..cbe1851bbc9d121b6ab7ac3f83401dbe205643aa 100644 (file)
--- 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));
+}