Make SkFunction copyable so it can go in containers.

This totally overhauls the implementation to use ordinary inheritance-based type erasure.  I give up for now getting my manual vtable shenanigans to work with MSVC.  Still those same "expected ; before ), also expected ) before ;" errors.

I added support for uninitialized SkFunctions and operator=(), because it was fairly straightforward with this implementation.

The main downside here is that I've removed the inline implementation.  All SkFunctions involve a heap allocation, even when just wrapping function pointers.

BUG=skia:

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

src/core/SkFunction.h

@@ -8,94 +8,68 @@
 #ifndef SkFunction_DEFINED
 #define SkFunction_DEFINED
 
-// TODO: document
+// TODO: document, more pervasive move support in constructors, small-Fn optimization
 
+#include "SkTemplates.h"
 #include "SkTypes.h"
-#include "SkTLogic.h"
 
 template <typename> class SkFunction;
 
 template <typename R, typename... Args>
-class SkFunction<R(Args...)> : SkNoncopyable {
+class SkFunction<R(Args...)> {
 public:
-    SkFunction(R (*fn)(Args...)) : fVTable(GetFunctionPointerVTable()) {
+    SkFunction() {}
-        // We've been passed a function pointer.  We'll just store it.
-        fFunction = reinterpret_cast<void*>(fn);
-    }
 
     template <typename Fn>
-    SkFunction(Fn fn, SK_WHEN_C((sizeof(Fn) > sizeof(void*)), void*) = nullptr)
+    SkFunction(const Fn& fn) : fFunction(SkNEW_ARGS(LambdaImpl<Fn>, (fn))) {}
-            : fVTable(GetOutlineVTable<Fn>()) {
+
-        // We've got a functor larger than a pointer.  We've go to copy it onto the heap.
+    SkFunction(R (*fn)(Args...)) : fFunction(SkNEW_ARGS(FnPtrImpl, (fn))) {}
-        fFunction = SkNEW_ARGS(Fn, (Forward(fn)));
+
+    SkFunction(const SkFunction& other) { *this = other; }
+    SkFunction& operator=(const SkFunction& other) {
+        if (this != &other) {
+            fFunction.reset(other.fFunction ? other.fFunction->clone() : nullptr);
+        }
+        return *this;
     }
 
-    template <typename Fn>
+    R operator()(Args... args) const {
-    SkFunction(Fn fn, SK_WHEN_C((sizeof(Fn) <= sizeof(void*)), void*) = nullptr)
+        SkASSERT(fFunction.get());
-            : fVTable(GetInlineVTable<Fn>()) {
+        return fFunction->call(Forward(args)...);
-        // 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.
-        SkNEW_PLACEMENT_ARGS(&fFunction, Fn, (Forward(fn)));
     }
 
-    ~SkFunction() { fVTable.fCleanUp(fFunction); }
-
-    R operator()(Args... args) { return fVTable.fCall(fFunction, Forward(args)...); }
-
 private:
     // ~= std::forward.  This moves its argument if possible, falling back to a copy if not.
     template <typename T> static T&& Forward(T& v) { return (T&&)v; }
 
-    struct VTable {
+    struct Interface {
-        R (*fCall)(void*, Args...);
+        virtual ~Interface() {}
-        void (*fCleanUp)(void*);
+        virtual R call(Args...) const = 0;
+        virtual Interface* clone() const = 0;
     };
 
-    // Used when fFunction is a function pointer of type R(*)(Args...).
-    static const VTable& GetFunctionPointerVTable() {
-        static const VTable vtable = {
-            [](void* fn, Args... args) {
-                return reinterpret_cast<R(*)(Args...)>(fn)(Forward(args)...);
-            },
-            [](void*) { /* Nothing to clean up for function pointers. */ }
-        };
-        return vtable;
-    }
-
-    // Used when fFunction is a pointer to a functor of type Fn on the heap (we own it).
     template <typename Fn>
-    static const VTable& GetOutlineVTable() {
+    class LambdaImpl final : public Interface {
-        static const VTable vtable = {
+    public:
-            [](void* fn, Args... args) { return (*static_cast<Fn*>(fn))(Forward(args)...); },
+        LambdaImpl(const Fn& fn) : fFn(fn) {}
-            [](void* fn) { SkDELETE(static_cast<Fn*>(fn)); },
-        };
-        return vtable;
-    }
 
-    // Used when fFunction _is_ a functor of type Fn, not a pointer to the functor.
+        R call(Args... args) const override { return fFn(Forward(args)...); }
-    template <typename Fn>
+        Interface* clone() const { return SkNEW_ARGS(LambdaImpl<Fn>, (fFn)); }
-    static const VTable& GetInlineVTable() {
+    private:
-        static const VTable vtable = {
+        Fn fFn;
-            [](void* fn, Args... args) {
+    };
-                union { void** p; Fn* f; } pun = { &fn };
-                return (*pun.f)(Forward(args)...);
-            },
-            [](void* fn) {
-                union { void** p; Fn* f; } pun = { &fn };
-                (*pun.f).~Fn();
-                (void)(pun.f);   // Otherwise, when ~Fn() is trivial, MSVC complains pun is unused.
-            }
-        };
-        return vtable;
-    }
 
+    class FnPtrImpl final : public Interface {
+    public:
+        FnPtrImpl(R (*fn)(Args...)) : fFn(fn) {}
 
-    void* fFunction;        // A function pointer, a pointer to a functor, or an inlined functor.
+        R call(Args... args) const override { return fFn(Forward(args)...); }
-    const VTable& fVTable;  // How to call, delete (and one day copy, move) fFunction.
+        Interface* clone() const { return SkNEW_ARGS(FnPtrImpl, (fFn)); }
+    private:
+        R (*fFn)(Args...);
+    };
+
+    SkAutoTDelete<Interface> fFunction;
 };
 
-// TODO:
-//   - is it worth moving fCall out of the VTable into SkFunction itself to avoid the indirection?
-//   - make SkFunction copyable
-
 #endif//SkFunction_DEFINED

tests/FunctionTest.cpp

@@ -8,29 +8,31 @@
 #include "SkFunction.h"
 #include "Test.h"
 
-static void test_add_five(skiatest::Reporter* r, SkFunction<int(int)>&& f) {
+static void test_add_five(skiatest::Reporter* r, SkFunction<int(int)>& f) {
     REPORTER_ASSERT(r, f(3) == 8);
     REPORTER_ASSERT(r, f(4) == 9);
 }
 
+static void test_add_five(skiatest::Reporter* r, SkFunction<int(int)>&& f) { test_add_five(r, f); }
+
 static int add_five(int x) { return x + 5; }
 
 struct AddFive {
-    int operator()(int x) { return x + 5; };
+    int operator()(int x) const { return x + 5; };
 };
 
-class MoveOnlyAdd5 : SkNoncopyable {
+class MoveOnlyThree : SkNoncopyable {
 public:
-    MoveOnlyAdd5() {}
+    MoveOnlyThree() {}
-    MoveOnlyAdd5(MoveOnlyAdd5&&) {}
+    MoveOnlyThree(MoveOnlyThree&&) {}
-    MoveOnlyAdd5& operator=(MoveOnlyAdd5&&) { return *this; }
+    MoveOnlyThree& operator=(MoveOnlyThree&&) { return *this; }
 
-    int operator()(int x) { return x + 5; }
+    int val() { return 3; }
 };
 
 DEF_TEST(Function, r) {
-    // We should be able to turn a static function, an explicit functor, or a lambda
+    // We should be able to turn a function pointer, an explicit functor, or a
-    // all into an SkFunction equally well.
+    // lambda into an SkFunction all equally well.
     test_add_five(r, &add_five);
     test_add_five(r, AddFive());
     test_add_five(r, [](int x) { return x + 5; });
@@ -40,12 +42,29 @@ DEF_TEST(Function, r) {
     int a = 1, b = 1, c = 1, d = 1, e = 1;
     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, MoveOnlyAdd5());
-
     // Makes sure we forward arguments when calling SkFunction.
-    SkFunction<int(int, MoveOnlyAdd5&&, int)> f([](int x, MoveOnlyAdd5&& addFive, int y) {
+    SkFunction<int(int, MoveOnlyThree&&, int)> f([](int x, MoveOnlyThree&& three, int y) {
-            return x * addFive(y);
+            return x * three.val() + y;
     });
-    REPORTER_ASSERT(r, f(2, MoveOnlyAdd5(), 4) == 18);
+    REPORTER_ASSERT(r, f(2, MoveOnlyThree(), 4) == 10);
+
+    // SkFunctions can go in containers.
+    SkTArray<SkFunction<int(int)>> add_fivers;
+    add_fivers.push_back(&add_five);
+    add_fivers.push_back(AddFive());
+    add_fivers.push_back([](int x) { return x + 5; });
+    add_fivers.push_back([&](int x) { return x + a + b + c + d + e; });
+    for (auto& f : add_fivers) {
+        test_add_five(r, f);
+    }
+
+    // SkFunctions are assignable.
+    SkFunction<int(int)> empty;
+    empty = [](int x) { return x + 5; };
+    test_add_five(r, empty);
+
+    // This all is silly acrobatics, but it should at least work correctly.
+    SkFunction<int(int)> emptyA, emptyB(emptyA);
+    emptyA = emptyB;
+    emptyA = emptyA;
 }