Implicit constructors for SkFunction are much more readable.
authormtklein <mtklein@chromium.org>
Wed, 1 Apr 2015 20:08:50 +0000 (13:08 -0700)
committerCommit bot <commit-bot@chromium.org>
Wed, 1 Apr 2015 20:08:50 +0000 (13:08 -0700)
BUG=skia:

Review URL: https://codereview.chromium.org/1052663004

src/core/SkFunction.h
tests/FunctionTest.cpp

index 98ae789..43438dd 100644 (file)
@@ -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
index 21b6011..01ba588 100644 (file)
@@ -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);
 }