SkRecord: infer return type for visit() and mutate().

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

11 files changed, 40 insertions, 44 deletions

diff --git a/dm/DMSrcSink.cpp b/dm/DMSrcSink.cpp
index 3f4667931b..6fd46c80ed 100644
--- a/dm/DMSrcSink.cpp
+++ b/dm/DMSrcSink.cpp
@@ -842,15 +842,15 @@ Error ImageGenSrc::draw(SkCanvas* canvas) const {
         canvas->drawImage(image, 0, 0);
         return "";
     }
-    
+
     // Test various color and alpha types on CPU
     SkImageInfo decodeInfo = gen->getInfo().makeAlphaType(fDstAlphaType);
-    
+
     if (kGray_8_SkColorType == decodeInfo.colorType() &&
             kOpaque_SkAlphaType != decodeInfo.alphaType()) {
         return Error::Nonfatal("Avoid requesting non-opaque kGray8 decodes.");
     }
-    
+
     SkAutoTUnref<SkColorTable> colorTable(nullptr);
     SkPMColor* colorPtr = nullptr;
     int* colorCountPtr = nullptr;
@@ -867,7 +867,7 @@ Error ImageGenSrc::draw(SkCanvas* canvas) const {
         return SkStringPrintf("Image(%s) is too large (%d x %d)", fPath.c_str(),
                               decodeInfo.width(), decodeInfo.height());
     }
-    
+
     if (!gen->getPixels(decodeInfo, bitmap.getPixels(), bitmap.rowBytes(), colorPtr,
                         colorCountPtr))
     {
@@ -1508,7 +1508,7 @@ Error ViaSingletonPictures::draw(
             drawables ? *drawables : empty,
         };
         for (int i = 0; i < skr.count(); i++) {
-            skr.visit<void>(i, drawsAsSingletonPictures);
+            skr.visit(i, drawsAsSingletonPictures);
         }
         sk_sp<SkPicture> macroPic(macroRec.finishRecordingAsPicture());
 
diff --git a/src/core/SkBigPicture.cpp b/src/core/SkBigPicture.cpp
index 61f3a0ef1c..70f68db337 100644
--- a/src/core/SkBigPicture.cpp
+++ b/src/core/SkBigPicture.cpp
@@ -88,9 +88,9 @@ SkBigPicture::Analysis::Analysis(const SkRecord& record) {
 
     bool hasText = false, hasBitmap = false;
     for (int i = 0; i < record.count(); i++) {
-        hasText   = hasText   || record.visit<bool>(i,   text);
-        hasBitmap = hasBitmap || record.visit<bool>(i, bitmap);
-        record.visit<void>(i, path);
+        hasText   = hasText   || record.visit(i,   text);
+        hasBitmap = hasBitmap || record.visit(i, bitmap);
+        record.visit(i, path);
     }
 
     fHasText                    = hasText;
diff --git a/src/core/SkRecord.cpp b/src/core/SkRecord.cpp
index 2e03e993b8..3685b2d16d 100644
--- a/src/core/SkRecord.cpp
+++ b/src/core/SkRecord.cpp
@@ -11,7 +11,7 @@
 SkRecord::~SkRecord() {
     Destroyer destroyer;
     for (int i = 0; i < this->count(); i++) {
-        this->mutate<void>(i, destroyer);
+        this->mutate(i, destroyer);
     }
 }
 
diff --git a/src/core/SkRecord.h b/src/core/SkRecord.h
index 866d1837ad..79fe5232fb 100644
--- a/src/core/SkRecord.h
+++ b/src/core/SkRecord.h
@@ -46,24 +46,20 @@ public:
     //   template <typename T>
     //   R operator()(const T& record) { ... }
     // This operator() must be defined for at least all SkRecords::*.
-    template <typename R, typename F>
-    R visit(int i, F& f) const {
-        SkASSERT(i < this->count());
-        return fRecords[i].visit<R>(f);
+    template <typename F>
+    auto visit(int i, F&& f) const -> decltype(f(SkRecords::NoOp())) {
+        return fRecords[i].visit(f);
     }
 
     // Mutate the i-th canvas command with a functor matching this interface:
     //   template <typename T>
     //   R operator()(T* record) { ... }
     // This operator() must be defined for at least all SkRecords::*.
-    template <typename R, typename F>
-    R mutate(int i, F& f) {
-        SkASSERT(i < this->count());
-        return fRecords[i].mutate<R>(f);
+    template <typename F>
+    auto mutate(int i, F&& f) -> decltype(f((SkRecords::NoOp*)nullptr)) {
+        return fRecords[i].mutate(f);
     }
 
-    // TODO: It'd be nice to infer R from F for visit and mutate.
-
     // Allocate contiguous space for count Ts, to be freed when the SkRecord is destroyed.
     // Here T can be any class, not just those from SkRecords.  Throws on failure.
     template <typename T>
@@ -89,7 +85,7 @@ public:
         SkASSERT(i < this->count());
 
         Destroyer destroyer;
-        this->mutate<void>(i, destroyer);
+        this->mutate(i, destroyer);
 
         return fRecords[i].set(this->allocCommand<T>());
     }
@@ -168,23 +164,23 @@ private:
         void* ptr() const { return (void*)(fTypeAndPtr & ((1ull<<kTypeShift)-1)); }
 
         // Visit this record with functor F (see public API above).
-        template <typename R, typename F>
-        R visit(F& f) const {
+        template <typename F>
+        auto visit(F&& f) const -> decltype(f(SkRecords::NoOp())) {
         #define CASE(T) case SkRecords::T##_Type: return f(*(const SkRecords::T*)this->ptr());
             switch(this->type()) { SK_RECORD_TYPES(CASE) }
         #undef CASE
             SkDEBUGFAIL("Unreachable");
-            return R();
+            return f(SkRecords::NoOp());
         }
 
         // Mutate this record with functor F (see public API above).
-        template <typename R, typename F>
-        R mutate(F& f) {
+        template <typename F>
+        auto mutate(F&& f) -> decltype(f((SkRecords::NoOp*)nullptr)) {
         #define CASE(T) case SkRecords::T##_Type: return f((SkRecords::T*)this->ptr());
             switch(this->type()) { SK_RECORD_TYPES(CASE) }
         #undef CASE
             SkDEBUGFAIL("Unreachable");
-            return R();
+            return f((SkRecords::NoOp*)nullptr);
         }
     };
 
diff --git a/src/core/SkRecordDraw.cpp b/src/core/SkRecordDraw.cpp
index 0e4b5af580..394cc72026 100644
--- a/src/core/SkRecordDraw.cpp
+++ b/src/core/SkRecordDraw.cpp
@@ -40,7 +40,7 @@ void SkRecordDraw(const SkRecord& record,
             // This visit call uses the SkRecords::Draw::operator() to call
             // methods on the |canvas|, wrapped by methods defined with the
             // DRAW() macro.
-            record.visit<void>(ops[i], draw);
+            record.visit(ops[i], draw);
         }
     } else {
         // Draw all ops.
@@ -52,7 +52,7 @@ void SkRecordDraw(const SkRecord& record,
             // This visit call uses the SkRecords::Draw::operator() to call
             // methods on the |canvas|, wrapped by methods defined with the
             // DRAW() macro.
-            record.visit<void>(i, draw);
+            record.visit(i, draw);
         }
     }
 }
@@ -66,7 +66,7 @@ void SkRecordPartialDraw(const SkRecord& record, SkCanvas* canvas,
     stop = SkTMin(stop, record.count());
     SkRecords::Draw draw(canvas, drawablePicts, nullptr, drawableCount, &initialCTM);
     for (int i = start; i < stop; i++) {
-        record.visit<void>(i, draw);
+        record.visit(i, draw);
     }
 }
 
@@ -521,7 +521,7 @@ private:
     Bounds bounds(const DrawAnnotation& op) const {
         return this->adjustAndMap(op.rect, nullptr);
     }
-    
+
     static void AdjustTextForFontMetrics(SkRect* rect, const SkPaint& paint) {
 #ifdef SK_DEBUG
         SkRect correct = *rect;
@@ -797,7 +797,7 @@ void SkRecordFillBounds(const SkRect& cullRect, const SkRecord& record, SkRect b
     SkRecords::FillBounds visitor(cullRect, record, bounds);
     for (int curOp = 0; curOp < record.count(); curOp++) {
         visitor.setCurrentOp(curOp);
-        record.visit<void>(curOp, visitor);
+        record.visit(curOp, visitor);
     }
     visitor.cleanUp();
 }
@@ -807,7 +807,7 @@ void SkRecordComputeLayers(const SkRect& cullRect, const SkRecord& record, SkRec
     SkRecords::CollectLayers visitor(cullRect, record, bounds, pictList, data);
     for (int curOp = 0; curOp < record.count(); curOp++) {
         visitor.setCurrentOp(curOp);
-        record.visit<void>(curOp, visitor);
+        record.visit(curOp, visitor);
     }
     visitor.cleanUp();
 }
diff --git a/src/core/SkRecordPattern.h b/src/core/SkRecordPattern.h
index 9493bbfaf7..45f45724c1 100644
--- a/src/core/SkRecordPattern.h
+++ b/src/core/SkRecordPattern.h
@@ -147,7 +147,7 @@ private:
     template <typename T>
     int matchFirst(T* first, SkRecord* record, int i) {
         if (i < record->count()) {
-            if (record->mutate<bool>(i, *first)) {
+            if (record->mutate(i, *first)) {
                 return i+1;
             }
         }
@@ -158,7 +158,7 @@ private:
     template <typename T>
     int matchFirst(Greedy<T>* first, SkRecord* record, int i) {
         while (i < record->count()) {
-            if (!record->mutate<bool>(i, *first)) {
+            if (!record->mutate(i, *first)) {
                 return i;
             }
             i++;
diff --git a/src/gpu/GrRecordReplaceDraw.cpp b/src/gpu/GrRecordReplaceDraw.cpp
index 1fe6040789..4d793904f4 100644
--- a/src/gpu/GrRecordReplaceDraw.cpp
+++ b/src/gpu/GrRecordReplaceDraw.cpp
@@ -99,7 +99,7 @@ public:
                     return fNumReplaced;
                 }
 
-                record->visit<void>(fOps[fIndex], *this);
+                record->visit(fOps[fIndex], *this);
             }
 
         } else {
@@ -108,7 +108,7 @@ public:
                     return fNumReplaced;
                 }
 
-                record->visit<void>(fIndex, *this);
+                record->visit(fIndex, *this);
             }
         }
 
diff --git a/tests/RecordTest.cpp b/tests/RecordTest.cpp
index e613b425fd..1b4077cce8 100644
--- a/tests/RecordTest.cpp
+++ b/tests/RecordTest.cpp
@@ -29,7 +29,7 @@ public:
 
     void apply(const SkRecord& record) {
         for (int i = 0; i < record.count(); i++) {
-            record.visit<void>(i, *this);
+            record.visit(i, *this);
         }
     }
 
@@ -47,7 +47,7 @@ struct Stretch {
 
     void apply(SkRecord* record) {
         for (int i = 0; i < record->count(); i++) {
-            record->mutate<void>(i, *this);
+            record->mutate(i, *this);
         }
     }
 };
diff --git a/tests/RecordTestUtils.h b/tests/RecordTestUtils.h
index a281566652..853ee21908 100644
--- a/tests/RecordTestUtils.h
+++ b/tests/RecordTestUtils.h
@@ -30,7 +30,7 @@ struct ReadAs {
 template <typename T>
 static const T* assert_type(skiatest::Reporter* r, const SkRecord& record, int index) {
     ReadAs<T> reader;
-    record.visit<void>(index, reader);
+    record.visit(index, reader);
     REPORTER_ASSERT(r, T::kType == reader.type);
     REPORTER_ASSERT(r, SkToBool(reader.ptr));
     return reader.ptr;
@@ -45,7 +45,7 @@ template <typename DrawT> int count_instances_of_type(const SkRecord& record) {
     MatchType<DrawT> matcher;
     int counter = 0;
     for (int i = 0; i < record.count(); i++) {
-        counter += record.visit<int>(i, matcher);
+        counter += record.visit(i, matcher);
     }
     return counter;
 }
@@ -53,7 +53,7 @@ template <typename DrawT> int count_instances_of_type(const SkRecord& record) {
 template <typename DrawT> int find_first_instances_of_type(const SkRecord& record) {
     MatchType<DrawT> matcher;
     for (int i = 0; i < record.count(); i++) {
-        if (record.visit<int>(i, matcher)) {
+        if (record.visit(i, matcher)) {
             return i;
         }
     }
diff --git a/tests/RecorderTest.cpp b/tests/RecorderTest.cpp
index b1af1388fb..59d5a8e6cd 100644
--- a/tests/RecorderTest.cpp
+++ b/tests/RecorderTest.cpp
@@ -31,7 +31,7 @@ public:
 
     void apply(const SkRecord& record) {
         for (int i = 0; i < record.count(); i++) {
-            record.visit<void>(i, *this);
+            record.visit(i, *this);
         }
     }
 
diff --git a/tools/DumpRecord.cpp b/tools/DumpRecord.cpp
index e7d76f0515..efaf0eb58e 100644
--- a/tools/DumpRecord.cpp
+++ b/tools/DumpRecord.cpp
@@ -68,7 +68,7 @@ public:
 
             const SkRecord& record = *bp->record();
             for (int i = 0; i < record.count(); i++) {
-                record.visit<void>(i, *this);
+                record.visit(i, *this);
             }
 
             --fIndent;
@@ -119,6 +119,6 @@ void DumpRecord(const SkRecord& record,
                   bool timeWithCommand) {
     Dumper dumper(canvas, record.count(), timeWithCommand);
     for (int i = 0; i < record.count(); i++) {
-        record.visit<void>(i, dumper);
+        record.visit(i, dumper);
     }
 }