Convert all SkRecordPattern matchers into SkRecord mutators.

  - Allow any return type from SkRecord mutators and visitors;
  - update existing calls to mutate and visit;
  - convert match to operator() in SkRecordPattern;
  - tidy up a few inelegant bits of old code in tests.

The net result is that the generated code is much clearer.  All the mutate() calls
inline as you'd hope, and you can now actually follow along with the disassembly.

BUG=skia:2378
R=fmalita@chromium.org, bungeman@google.com, mtklein@google.com

Author: mtklein@chromium.org

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

git-svn-id: http://skia.googlecode.com/svn/trunk@14631 2bbb7eff-a529-9590-31e7-b0007b416f81

4 files changed, 33 insertions, 38 deletions

diff --git a/src/record/SkRecord.h b/src/record/SkRecord.h
index 068155e47f..6c5177eefb 100644
--- a/src/record/SkRecord.h
+++ b/src/record/SkRecord.h
@@ -33,7 +33,7 @@ public:
     ~SkRecord() {
         Destroyer destroyer;
         for (unsigned i = 0; i < this->count(); i++) {
-            this->mutate(i, destroyer);
+            this->mutate<void>(i, destroyer);
         }
     }
 
@@ -42,23 +42,24 @@ public:
 
     // Visit the i-th canvas command with a functor matching this interface:
     //   template <typename T>
-    //   void operator()(const T& record) { ... }
+    //   R operator()(const T& record) { ... }
     // This operator() must be defined for at least all SkRecords::*.
-    template <typename F>
-    void visit(unsigned i, F& f) const {
+    template <typename R, typename F>
+    R visit(unsigned i, F& f) const {
         SkASSERT(i < this->count());
-        fRecords[i].visit(fTypes[i], f);
+        return fRecords[i].visit<R>(fTypes[i], f);
     }
 
     // Mutate the i-th canvas command with a functor matching this interface:
     //   template <typename T>
-    //   void operator()(T* record) { ... }
+    //   R operator()(T* record) { ... }
     // This operator() must be defined for at least all SkRecords::*.
-    template <typename F>
-    void mutate(unsigned i, F& f) {
+    template <typename R, typename F>
+    R mutate(unsigned i, F& f) {
         SkASSERT(i < this->count());
-        fRecords[i].mutate(fTypes[i], f);
+        return fRecords[i].mutate<R>(fTypes[i], f);
     }
+    // TODO: It'd be nice to infer R from F for visit and mutate if we ever get std::result_of.
 
     // 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.
@@ -89,7 +90,7 @@ public:
         SkASSERT(i < this->count());
 
         Destroyer destroyer;
-        this->mutate(i, destroyer);
+        this->mutate<void>(i, destroyer);
 
         fTypes[i] = T::kType;
         return fRecords[i].set(this->allocCommand<T>());
@@ -191,20 +192,24 @@ private:
 
         // Visit this record with functor F (see public API above) assuming the record we're
         // pointing to has this type.
-        template <typename F>
-        void visit(Type8 type, F& f) const {
+        template <typename R, typename F>
+        R visit(Type8 type, F& f) const {
         #define CASE(T) case SkRecords::T##_Type: return f(*this->ptr<SkRecords::T>());
             switch(type) { SK_RECORD_TYPES(CASE) }
         #undef CASE
+            SkDEBUGFAIL("Unreachable");
+            return R();
         }
 
         // Mutate this record with functor F (see public API above) assuming the record we're
         // pointing to has this type.
-        template <typename F>
-        void mutate(Type8 type, F& f) {
+        template <typename R, typename F>
+        R mutate(Type8 type, F& f) {
         #define CASE(T) case SkRecords::T##_Type: return f(this->ptr<SkRecords::T>());
             switch(type) { SK_RECORD_TYPES(CASE) }
         #undef CASE
+            SkDEBUGFAIL("Unreachable");
+            return R();
         }
 
     private:
diff --git a/src/record/SkRecordDraw.cpp b/src/record/SkRecordDraw.cpp
index 4878b3f4f6..666cfc9000 100644
--- a/src/record/SkRecordDraw.cpp
+++ b/src/record/SkRecordDraw.cpp
@@ -94,6 +94,6 @@ template <> void Draw::draw(const SkRecords::BoundedDrawPosTextH& r) { this->dra
 
 void SkRecordDraw(const SkRecord& record, SkCanvas* canvas) {
     for (Draw draw(canvas); draw.index() < record.count(); draw.next()) {
-        record.visit(draw.index(), draw);
+        record.visit<void>(draw.index(), draw);
     }
 }
diff --git a/src/record/SkRecordOpts.cpp b/src/record/SkRecordOpts.cpp
index d855124917..c58a034fa7 100644
--- a/src/record/SkRecordOpts.cpp
+++ b/src/record/SkRecordOpts.cpp
@@ -264,7 +264,7 @@ public:
 
     void apply(SkRecord* record) {
         for (fRecord = record, fIndex = 0; fIndex < record->count(); fIndex++) {
-            fRecord->mutate(fIndex, *this);
+            fRecord->mutate<void>(fIndex, *this);
         }
     }
 
diff --git a/src/record/SkRecordPattern.h b/src/record/SkRecordPattern.h
index c5d87f2a20..b1334a87c6 100644
--- a/src/record/SkRecordPattern.h
+++ b/src/record/SkRecordPattern.h
@@ -17,13 +17,13 @@ public:
     typedef T type;
     type* get() { return fPtr; }
 
-    bool match(T* ptr) {
+    bool operator()(T* ptr) {
         fPtr = ptr;
         return true;
     }
 
     template <typename U>
-    bool match(U*) {
+    bool operator()(U*) {
         fPtr = NULL;
         return false;
     }
@@ -42,19 +42,19 @@ public:
     type* get() { return fPaint; }
 
     template <typename T>
-    SK_WHEN(HasMember_paint<T>, bool) match(T* draw) {
+    SK_WHEN(HasMember_paint<T>, bool) operator()(T* draw) {
         fPaint = AsPtr(draw->paint);
         return true;
     }
 
     template <typename T>
-    SK_WHEN(!HasMember_paint<T>, bool) match(T*) {
+    SK_WHEN(!HasMember_paint<T>, bool) operator()(T*) {
         fPaint = NULL;
         return false;
     }
 
     // SaveLayer has an SkPaint named paint, but it's not a draw.
-    bool match(SaveLayer*) {
+    bool operator()(SaveLayer*) {
         fPaint = NULL;
         return false;
     }
@@ -71,14 +71,14 @@ private:
 template <typename Matcher>
 struct Not {
     template <typename T>
-    bool match(T* ptr) { return !Matcher().match(ptr); }
+    bool operator()(T* ptr) { return !Matcher()(ptr); }
 };
 
 // Matches if either of A or B does.  Stores nothing.
 template <typename A, typename B>
 struct Or {
     template <typename T>
-    bool match(T* ptr) { return A().match(ptr) || B().match(ptr); }
+    bool operator()(T* ptr) { return A()(ptr) || B()(ptr); }
 };
 
 // Matches if any of A, B or C does.  Stores nothing.
@@ -96,7 +96,7 @@ public:
     void reset() {}
 
     template <typename T>
-    bool match(T* ptr) { return Matcher().match(ptr); }
+    bool operator()(T* ptr) { return Matcher()(ptr); }
 };
 
 // This version stores a list of matches.  It's enabled if Matcher stores something.
@@ -109,9 +109,9 @@ public:
     void reset() { fMatches.rewind(); }
 
     template <typename T>
-    bool match(T* ptr) {
+    bool operator()(T* ptr) {
         Matcher matcher;
-        if (matcher.match(ptr)) {
+        if (matcher(ptr)) {
             fMatches.push(matcher.get());
             return true;
         }
@@ -161,16 +161,11 @@ public:
     template <typename T> T* third()  { return fTail.fTail.fHead.get(); }
 
 private:
-    template <typename T>
-    void operator()(T* r) { fHeadMatched = fHead.match(r); }
-
     // If head isn't a Star, try to match at i once.
     template <typename T>
     unsigned matchHead(T*, SkRecord* record, unsigned i) {
         if (i < record->count()) {
-            fHeadMatched = false;
-            record->mutate(i, *this);
-            if (fHeadMatched) {
+            if (record->mutate<bool>(i, fHead)) {
                 return i+1;
             }
         }
@@ -182,9 +177,7 @@ private:
     unsigned matchHead(Star<T>*, SkRecord* record, unsigned i) {
         fHead.reset();
         while (i < record->count()) {
-            fHeadMatched = false;
-            record->mutate(i, *this);
-            if (!fHeadMatched) {
+            if (!record->mutate<bool>(i, fHead)) {
                 return i;
             }
             i++;
@@ -194,9 +187,6 @@ private:
 
     Matcher fHead;
     Pattern fTail;
-    bool fHeadMatched;
-
-    friend class ::SkRecord;  // So operator() can otherwise stay private.
 
     // All Cons are friends with each other.  This lets first, second, and third work.
     template <typename, typename> friend class Cons;