From 03e5161bed3bdfa0d9fb0867378237bac7d8bd12 Mon Sep 17 00:00:00 2001
From: mtklein <mtklein@chromium.org>
Date: Wed, 1 Apr 2015 13:08:50 -0700
Subject: [PATCH] Implicit constructors for SkFunction are much more readable.

BUG=skia:

Review URL: https://codereview.chromium.org/1052663004
---
 src/core/SkFunction.h  |  7 +++----
 tests/FunctionTest.cpp | 35 ++++++++++++++++-------------------
 2 files changed, 19 insertions(+), 23 deletions(-)

diff --git a/src/core/SkFunction.h b/src/core/SkFunction.h
index 98ae789cae..43438dd9e2 100644
--- a/src/core/SkFunction.h
+++ b/src/core/SkFunction.h
@@ -18,20 +18,20 @@ template <typename> class SkFunction;
 template <typename R, typename... Args>
 class SkFunction<R(Args...)> : SkNoncopyable {
 public:
-    explicit SkFunction(R (*fn)(Args...)) : fVTable(GetFunctionPointerVTable()) {
+    SkFunction(R (*fn)(Args...)) : fVTable(GetFunctionPointerVTable()) {
         // We've been passed a function pointer.  We'll just store it.
         fFunction = reinterpret_cast<void*>(fn);
     }
 
     template <typename Fn>
-    explicit SkFunction(Fn fn, SK_WHEN_C((sizeof(Fn) > sizeof(void*)), void*) = nullptr)
+    SkFunction(Fn fn, SK_WHEN_C((sizeof(Fn) > sizeof(void*)), void*) = nullptr)
             : fVTable(GetOutlineVTable<Fn>()) {
         // We've got a functor larger than a pointer.  We've go to copy it onto the heap.
         fFunction = SkNEW_ARGS(Fn, (Forward(fn)));
     }
 
     template <typename Fn>
-    explicit SkFunction(Fn fn, SK_WHEN_C((sizeof(Fn) <= sizeof(void*)), void*) = nullptr)
+    SkFunction(Fn fn, SK_WHEN_C((sizeof(Fn) <= sizeof(void*)), void*) = nullptr)
             : fVTable(GetInlineVTable<Fn>()) {
         // We've got a functor that fits in a pointer.  We copy it right inline.
         fFunction = NULL;  // Quiets a (spurious) warning that fFunction might be uninitialized.
@@ -96,7 +96,6 @@ private:
 
 // TODO:
 //   - is it worth moving fCall out of the VTable into SkFunction itself to avoid the indirection?
-//   - should constructors be implicit?
 //   - make SkFunction copyable
 
 #endif//SkFunction_DEFINED
diff --git a/tests/FunctionTest.cpp b/tests/FunctionTest.cpp
index 21b60110ee..01ba5885a8 100644
--- a/tests/FunctionTest.cpp
+++ b/tests/FunctionTest.cpp
@@ -19,35 +19,32 @@ struct AddFive {
     int operator()(int x) { return x + 5; };
 };
 
+class MoveOnlyAdd5 : SkNoncopyable {
+public:
+    MoveOnlyAdd5() {}
+    MoveOnlyAdd5(MoveOnlyAdd5&&) {}
+    MoveOnlyAdd5& operator=(MoveOnlyAdd5&&) { return *this; }
+
+    int operator()(int x) { return x + 5; }
+};
+
 DEF_TEST(Function, r) {
     // We should be able to turn a static function, an explicit functor, or a lambda
     // all into an SkFunction equally well.
-    test_add_five(r, SkFunction<int(int)>(&add_five));
-    test_add_five(r, SkFunction<int(int)>(AddFive()));
-    test_add_five(r, SkFunction<int(int)>([](int x) { return x + 5; }));
+    test_add_five(r, &add_five);
+    test_add_five(r, AddFive());
+    test_add_five(r, [](int x) { return x + 5; });
 
     // AddFive and the lambda above are both small enough to test small-object optimization.
     // Now test a lambda that's much too large for the small-object optimization.
     int a = 1, b = 1, c = 1, d = 1, e = 1;
-    test_add_five(r, SkFunction<int(int)>([&](int x) { return x + a + b + c + d + e; }));
-}
-
-DEF_TEST(Function_forwarding, r) {
-    class MoveOnlyAdd5 : SkNoncopyable {
-    public:
-        MoveOnlyAdd5() {}
-        MoveOnlyAdd5(MoveOnlyAdd5&&) {}
-        MoveOnlyAdd5& operator=(MoveOnlyAdd5&&) { return *this; }
-
-        int operator()(int x) { return x + 5; }
-    };
+    test_add_five(r, [&](int x) { return x + a + b + c + d + e; });
 
     // Makes sure we forward the functor when constructing SkFunction.
-    test_add_five(r, SkFunction<int(int)>(MoveOnlyAdd5()));
+    test_add_five(r, MoveOnlyAdd5());
 
     // Makes sure we forward arguments when calling SkFunction.
-    SkFunction<int(int, MoveOnlyAdd5&&, int)> b([](int x, MoveOnlyAdd5&& f, int y) {
+    REPORTER_ASSERT(r, [](int x, MoveOnlyAdd5&& f, int y) {
         return x * f(y);
-    });
-    REPORTER_ASSERT(r, b(2, MoveOnlyAdd5(), 4) == 18);
+    }(2, MoveOnlyAdd5(), 4) == 18);
 }