feedback

feedback from include review

Docs-Preview: https://skia.org/?cl=49260
Bug: skia: 6898
Change-Id: Ic60980dfe53d14fd8c6973ef2f4dabb6b650cdd8
Reviewed-on: https://skia-review.googlesource.com/49260
Commit-Queue: Cary Clark <caryclark@skia.org>
Reviewed-by: Cary Clark <caryclark@skia.org>

3 files changed, 168 insertions, 76 deletions

diff --git a/tools/bookmaker/bookmaker.cpp b/tools/bookmaker/bookmaker.cpp
index ac1a4a9939..2ec987b333 100644
--- a/tools/bookmaker/bookmaker.cpp
+++ b/tools/bookmaker/bookmaker.cpp
@@ -1119,8 +1119,10 @@ bool BmhParser::addDefinition(const char* defStart, bool hasEnd, MarkType markTy
         case MarkType::kTime:
         case MarkType::kVolatile:
         case MarkType::kWidth:
-            if (hasEnd) {
+            if (hasEnd && MarkType::kAnchor != markType) {
                 return this->reportError<bool>("one liners omit end element");
+            } else if (!hasEnd && MarkType::kAnchor == markType) {
+                return this->reportError<bool>("anchor line must have end element last");
             }
             fMarkup.emplace_front(markType, defStart, fLineCount, fParent);
             definition = &fMarkup.front();
diff --git a/tools/bookmaker/bookmaker.h b/tools/bookmaker/bookmaker.h
index 4e93f88faf..4cb9fea1c7 100644
--- a/tools/bookmaker/bookmaker.h
+++ b/tools/bookmaker/bookmaker.h
@@ -1841,12 +1841,13 @@ public:
         return 0 == size;
     }
 
+    void descriptionOut(const Definition* def);
     void enumHeaderOut(const RootDefinition* root, const Definition& child);
     void enumMembersOut(const RootDefinition* root, Definition& child);
     void enumSizeItems(const Definition& child);
     int lookupMethod(const PunctuationState punctuation, const Word word,
             const int start, const int run, int lastWrite, 
-            const char* data);
+            const char* data, bool hasIndirection);
     int lookupReference(const PunctuationState punctuation, const Word word,
             const int start, const int run, int lastWrite, const char last, 
             const char* data);
diff --git a/tools/bookmaker/includeWriter.cpp b/tools/bookmaker/includeWriter.cpp
index 60b1b9288a..e2215a6e1c 100644
--- a/tools/bookmaker/includeWriter.cpp
+++ b/tools/bookmaker/includeWriter.cpp
@@ -7,6 +7,108 @@
 
 #include "bookmaker.h"
 
+void IncludeWriter::descriptionOut(const Definition* def) {
+    const char* commentStart = def->fContentStart;
+    int commentLen = (int) (def->fContentEnd - commentStart);
+    bool breakOut = false;
+    for (auto prop : def->fChildren) {
+        switch (prop->fMarkType) {
+            case MarkType::kDefinedBy:
+                commentStart = prop->fTerminator;
+                break;
+            case MarkType::kDeprecated: 
+            case MarkType::kPrivate:
+                commentLen = (int) (prop->fStart - commentStart);
+                if (commentLen > 0) {
+                    SkASSERT(commentLen < 1000);
+                    if (Wrote::kNone != this->rewriteBlock(commentLen, commentStart, Phrase::kNo)) {
+                        this->lfcr();
+                    }
+                }
+                commentStart = prop->fContentStart;
+                commentLen = (int) (prop->fContentEnd - commentStart);
+                if (commentLen > 0) {
+                    if (Wrote::kNone != this->rewriteBlock(commentLen, commentStart, Phrase::kNo)) {
+                        this->lfcr();
+                    }
+                }
+                commentStart = prop->fTerminator;
+                commentLen = (int) (def->fContentEnd - commentStart);
+                break;
+            case MarkType::kExperimental:
+                this->writeString("EXPERIMENTAL:");
+                this->writeSpace();
+                commentStart = prop->fContentStart;
+                commentLen = (int) (prop->fContentEnd - commentStart);
+                if (commentLen > 0) {
+                    if (Wrote::kNone != this->rewriteBlock(commentLen, commentStart, Phrase::kNo)) {
+                        this->lfcr();
+                    }
+                }
+                commentStart = prop->fTerminator;
+                commentLen = (int) (def->fContentEnd - commentStart);
+                break;
+            case MarkType::kFormula:
+                commentLen = prop->fStart - commentStart;
+                if (commentLen > 0) {
+                    if (Wrote::kNone != this->rewriteBlock(commentLen, commentStart, Phrase::kNo)) {
+                        this->lfcr();
+                    }
+                }
+                this->writeBlock(prop->length(), prop->fContentStart);
+                commentStart = prop->fTerminator;
+                commentLen = (int) (def->fContentEnd - commentStart);
+                if ('\n' == commentStart[0] && '\n' == commentStart[1]) {
+                    this->lf(2);
+                }
+                break;
+            case MarkType::kToDo:
+                commentLen = (int) (prop->fStart - commentStart);
+                if (commentLen > 0) {
+                    SkASSERT(commentLen < 1000);
+                    if (Wrote::kNone != this->rewriteBlock(commentLen, commentStart, Phrase::kNo)) {
+                        this->lfcr();
+                    }
+                }
+                commentStart = prop->fTerminator;
+                commentLen = (int) (def->fContentEnd - commentStart);
+                break;
+            case MarkType::kList:
+                commentLen = prop->fStart - commentStart;
+                if (commentLen > 0) {
+                    if (Wrote::kNone != this->rewriteBlock(commentLen, commentStart,
+                            Phrase::kNo)) {
+                        this->lfcr();
+                    }
+                }
+                for (auto row : prop->fChildren) {
+                    SkASSERT(MarkType::kRow == row->fMarkType);
+                    for (auto column : row->fChildren) {
+                        SkASSERT(MarkType::kColumn == column->fMarkType);
+                        this->writeString("-");
+                        this->writeSpace();
+                        this->descriptionOut(column);
+                        this->lf(1);
+                    }
+                }
+                commentStart = prop->fTerminator;
+                commentLen = (int) (def->fContentEnd - commentStart);
+                if ('\n' == commentStart[0] && '\n' == commentStart[1]) {
+                    this->lf(2);
+                }
+                break;
+            default:
+                commentLen = (int) (prop->fStart - commentStart);
+                breakOut = true;
+        }
+        if (breakOut) {
+            break;
+        }
+    }
+    SkASSERT(commentLen > 0 && commentLen < 1500);
+    this->rewriteBlock(commentLen, commentStart, Phrase::kNo);
+}
+
 void IncludeWriter::enumHeaderOut(const RootDefinition* root,
         const Definition& child) {
     const Definition* enumDef = nullptr;
@@ -55,6 +157,7 @@ void IncludeWriter::enumHeaderOut(const RootDefinition* root,
     Definition* codeBlock = nullptr;
     const char* commentStart = nullptr;
     bool wroteHeader = false;
+    bool lastAnchor = false;
     SkDEBUGCODE(bool foundConst = false);
     for (auto test : enumDef->fChildren) {
         if (MarkType::kCode == test->fMarkType) {
@@ -79,13 +182,22 @@ void IncludeWriter::enumHeaderOut(const RootDefinition* root,
             this->lfcr();
             wroteHeader = true;
         }
+        if (lastAnchor) {
+            if (commentEnd - commentStart > 1) {
+                SkASSERT('\n' == commentStart[0]);
+                if (' ' == commentStart[1]) {
+                    this->writeSpace();
+                }
+            }
+            lastAnchor = false;
+        }
         this->rewriteBlock((int) (commentEnd - commentStart), commentStart, Phrase::kNo);
         if (MarkType::kAnchor == test->fMarkType) {
             commentStart = test->fContentStart;
             commentEnd = test->fChildren[0]->fStart;
             this->writeSpace();
             this->rewriteBlock((int) (commentEnd - commentStart), commentStart, Phrase::kNo);
-            this->writeSpace();
+            lastAnchor = true;   // this->writeSpace();
         }
         commentStart = test->fTerminator;
         if (MarkType::kConst == test->fMarkType) {
@@ -415,67 +527,7 @@ void IncludeWriter::methodOut(const Definition* method, const Definition& child)
     }
     this->writeCommentHeader();
     fIndent += 4;
-    const char* commentStart = method->fContentStart;
-    int commentLen = (int) (method->fContentEnd - commentStart);
-    bool breakOut = false;
-    for (auto methodProp : method->fChildren) {
-        switch (methodProp->fMarkType) {
-            case MarkType::kDefinedBy:
-                commentStart = methodProp->fTerminator;
-                break;
-            case MarkType::kDeprecated: 
-            case MarkType::kPrivate:
-                commentLen = (int) (methodProp->fStart - commentStart);
-                if (commentLen > 0) {
-                    SkASSERT(commentLen < 1000);
-                    if (Wrote::kNone != this->rewriteBlock(commentLen, commentStart, Phrase::kNo)) {
-                        this->lfcr();
-                    }
-                }
-                commentStart = methodProp->fContentStart;
-                commentLen = (int) (methodProp->fContentEnd - commentStart);
-                if (commentLen > 0) {
-                    if (Wrote::kNone != this->rewriteBlock(commentLen, commentStart, Phrase::kNo)) {
-                        this->lfcr();
-                    }
-                }
-                commentStart = methodProp->fTerminator;
-                commentLen = (int) (method->fContentEnd - commentStart);
-                break;
-            case MarkType::kExperimental:
-                this->writeString("EXPERIMENTAL:");
-                this->writeSpace();
-                commentStart = methodProp->fContentStart;
-                commentLen = (int) (methodProp->fContentEnd - commentStart);
-                if (commentLen > 0) {
-                    if (Wrote::kNone != this->rewriteBlock(commentLen, commentStart, Phrase::kNo)) {
-                        this->lfcr();
-                    }
-                }
-                commentStart = methodProp->fTerminator;
-                commentLen = (int) (method->fContentEnd - commentStart);
-                break;
-            case MarkType::kToDo:
-                commentLen = (int) (methodProp->fStart - commentStart);
-                if (commentLen > 0) {
-                    SkASSERT(commentLen < 1000);
-                    if (Wrote::kNone != this->rewriteBlock(commentLen, commentStart, Phrase::kNo)) {
-                        this->lfcr();
-                    }
-                }
-                commentStart = methodProp->fTerminator;
-                commentLen = (int) (method->fContentEnd - commentStart);
-                break;
-            default:
-                commentLen = (int) (methodProp->fStart - commentStart);
-                breakOut = true;
-        }
-        if (breakOut) {
-            break;
-        }
-    }
-    SkASSERT(commentLen > 0 && commentLen < 1500);
-    this->rewriteBlock(commentLen, commentStart, Phrase::kNo);
+    this->descriptionOut(method);
     // compute indention column
     size_t column = 0;
     bool hasParmReturn = false;
@@ -595,7 +647,7 @@ Definition* IncludeWriter::structMemberOut(const Definition* memberStart, const
             memberStart->fContentStart);
     this->indentToColumn(fStructMemberTab);
     this->writeString(name.c_str());
-    auto tokenIter = child.fParent->fTokens.begin();

+    auto tokenIter = child.fParent->fTokens.begin();
     std::advance(tokenIter, child.fParentIndex + 1);
     Definition* valueStart = &*tokenIter;
     while (Definition::Type::kPunctuation != tokenIter->fType) {
@@ -828,7 +880,8 @@ bool IncludeWriter::populate(Definition* def, ParentPair* prevPair, RootDefiniti
             }
             if (Definition::Type::kPunctuation == child.fType &&
                     (Punctuation::kSemicolon == child.fPunctuation ||
-                    Punctuation::kLeftBrace == child.fPunctuation)) {
+                    Punctuation::kLeftBrace == child.fPunctuation ||
+                    (Punctuation::kColon == child.fPunctuation && inConstructor))) {
                 SkASSERT(fContinuation[0] == '(');
                 const char* continueEnd = child.fContentStart;
                 while (continueEnd > fContinuation && isspace(continueEnd[-1])) {
@@ -838,7 +891,6 @@ bool IncludeWriter::populate(Definition* def, ParentPair* prevPair, RootDefiniti
                 method = root->find(methodName, RootDefinition::AllowParens::kNo);
                 if (!method) {
                     fLineCount = child.fLineCount;
-                    fclose(fOut);  // so we can see what we've written so far
                     return this->reportError<bool>("method not found");
                 }
                 this->methodOut(method, child);
@@ -852,17 +904,19 @@ bool IncludeWriter::populate(Definition* def, ParentPair* prevPair, RootDefiniti
             if (inConstructor) {
                 continue;
             }
-            methodName += "()";
-            method = root->find(methodName, RootDefinition::AllowParens::kNo);
+            method = root->find(methodName + "()", RootDefinition::AllowParens::kNo);
             if (method && MarkType::kDefinedBy == method->fMarkType) {
                 method = method->fParent;
             }
             if (method) {
+                if (method->fCloned) {
+                    clonedMethod = method;
+                    continue;
+                }
                 this->methodOut(method, child);
                 continue;
             }
             fLineCount = child.fLineCount;
-            fclose(fOut);  // so we can see what we've written so far
             return this->reportError<bool>("method not found");
         }
         if (Bracket::kSlashSlash == child.fBracket || Bracket::kSlashStar == child.fBracket) {
@@ -1196,6 +1250,16 @@ string IncludeWriter::resolveMethod(const char* start, const char* end, bool fir
         report.reportError("method should not include references to itself");
         return "";
     }
+    if (fBmhMethod) {
+        for (auto child : fBmhMethod->fChildren) {
+            if (MarkType::kParam != child->fMarkType) {
+                continue;
+            }
+            if (methodname == child->fName) {
+                return "";
+            }
+        }
+    }
     return substitute;
 }
 
@@ -1292,14 +1356,27 @@ string IncludeWriter::resolveRef(const char* start, const char* end, bool first,
 }
 
 int IncludeWriter::lookupMethod(const PunctuationState punctuation, const Word word,
-        const int lastSpace, const int run, int lastWrite, const char* data) {
+        const int lastSpace, const int run, int lastWrite, const char* data,
+        bool hasIndirection) {
     int wordStart = lastSpace;
     while (' ' >= data[wordStart]) {
         ++wordStart;
     }
     const int wordEnd = PunctuationState::kDelimiter == punctuation ||
             PunctuationState::kPeriod == punctuation ? run - 1 : run;
-    string temp = this->resolveMethod(&data[wordStart], &data[wordEnd], Word::kFirst == word);
+    string temp;
+    if (hasIndirection && '(' != data[wordEnd - 1] && ')' != data[wordEnd - 1]) {
+        // FIXME: hard-coded to assume a.b or a->b is a.b() or a->b().
+        // need to check class a for member b to see if this is so
+        TextParser parser(fFileName, &data[wordStart], &data[wordEnd], fLineCount);
+        const char* indirection = parser.anyOf(".>");
+        if (&data[wordEnd] <= &indirection[2] || 'f' != indirection[1] ||
+                !isupper(indirection[2])) {
+            temp = string(&data[wordStart], wordEnd - wordStart) + "()";
+        }
+    } else {
+        temp = this->resolveMethod(&data[wordStart], &data[wordEnd], Word::kFirst == word);
+    }
     if (temp.length()) {
         if (wordStart > lastWrite) {
             SkASSERT(data[wordStart - 1] >= ' ');
@@ -1372,9 +1449,11 @@ IncludeWriter::Wrote IncludeWriter::rewriteBlock(int size, const char* data, Phr
     int lastSpace = -1;
     char c = 0;
     char last;
+    bool embeddedIndirection = false;
     bool embeddedSymbol = false;
     bool hasLower = false;
     bool hasUpper = false;
+    bool hasIndirection = false;
     bool hasSymbol = false;
     while (run < size) {
         last = c;
@@ -1417,7 +1496,7 @@ IncludeWriter::Wrote IncludeWriter::rewriteBlock(int size, const char* data, Phr
                     case Word::kMixed:
                         if (hasUpper && hasLower && !hasSymbol && lastSpace > 0) {
                             lastWrite = this->lookupMethod(punctuation, word, lastSpace, run,
-                                    lastWrite, data);
+                                    lastWrite, data, hasIndirection && !hasSymbol);
                         }
                         break;
                     default:
@@ -1427,9 +1506,11 @@ IncludeWriter::Wrote IncludeWriter::rewriteBlock(int size, const char* data, Phr
                         (PunctuationState::kStart == punctuation && ' ' >= last) ? 
                         PunctuationState::kStart : PunctuationState::kSpace;
                 word = Word::kStart;
+                embeddedIndirection = false;
                 embeddedSymbol = false;
                 hasLower = false;
                 hasUpper = false;
+                hasIndirection = false;
                 hasSymbol = false;
                 lastSpace = run;
                 break;
@@ -1450,7 +1531,7 @@ IncludeWriter::Wrote IncludeWriter::rewriteBlock(int size, const char* data, Phr
                     default:
                         SkASSERT(0);
                 }
-                embeddedSymbol = true;
+                embeddedIndirection = true;
                 break;
             case ',': case ';': case ':':
                 switch (word) {
@@ -1471,11 +1552,16 @@ IncludeWriter::Wrote IncludeWriter::rewriteBlock(int size, const char* data, Phr
                 }
                 embeddedSymbol = true;
                 break;
+            case '>':
+                if ('-' == last) {
+                    embeddedIndirection = true;
+                    break;
+                }
             case '\'': // possessive apostrophe isn't treated as delimiting punctation
             case '\"': // quote is passed straight through
             case '=':
             case '!':  // assumed not to be punctuation, but a programming symbol
-            case '&': case '>': case '<': case '{': case '}': case '/': case '*': case '[': case ']':
+            case '&': case '<': case '{': case '}': case '/': case '*': case '[': case ']':
                 word = Word::kMixed;
                 embeddedSymbol = true;
                 break;
@@ -1545,6 +1631,7 @@ IncludeWriter::Wrote IncludeWriter::rewriteBlock(int size, const char* data, Phr
                         PunctuationState::kDelimiter == punctuation) {
                     word = Word::kMixed;
                 }
+                hasIndirection |= embeddedIndirection;
                 hasSymbol |= embeddedSymbol;
                 break;
             case 'a': case 'b': case 'c': case 'd': case 'e':
@@ -1570,6 +1657,7 @@ IncludeWriter::Wrote IncludeWriter::rewriteBlock(int size, const char* data, Phr
                 }
                 hasLower = true;
                 punctuation = PunctuationState::kStart;
+                hasIndirection |= embeddedIndirection;
                 hasSymbol |= embeddedSymbol;
                 break;
             default:
@@ -1580,7 +1668,8 @@ IncludeWriter::Wrote IncludeWriter::rewriteBlock(int size, const char* data, Phr
     if ((word == Word::kCap || word == Word::kFirst || word == Word::kUnderline) && hasLower) {
         lastWrite = this->lookupReference(punctuation, word, start, run, lastWrite, last, data);
     } else if (word == Word::kMixed && hasUpper && hasLower && !hasSymbol && lastSpace > 0) {
-        lastWrite = this->lookupMethod(punctuation, word, lastSpace, run, lastWrite, data);
+        lastWrite = this->lookupMethod(punctuation, word, lastSpace, run, lastWrite, data,
+                hasIndirection && !hasSymbol);
     }
     if (run > lastWrite) {
         if (' ' == data[lastWrite]) {