[parser] Use Back2() where appropriate.

R=marja@chromium.org
BUG=v8:4947

Review-Url: https://codereview.chromium.org/2366573002
Cr-Commit-Position: refs/heads/master@{#39662}

src/parsing/scanner.cc

@@ -340,14 +340,14 @@ static inline bool IsLittleEndianByteOrderMark(uc32 c) {
   return c == 0xFFFE;
 }
 
-
 bool Scanner::SkipWhiteSpace() {
   int start_position = source_pos();
 
   while (true) {
     while (true) {
-      // The unicode cache accepts unsigned inputs.
+      // Don't skip behind the end of input.
       if (c0_ == kEndOfInput) break;
+
       // Advance as long as character is a WhiteSpace or LineTerminator.
       // Remember if the latter is the case.
       if (unicode_cache_->IsLineTerminator(c0_)) {
@@ -363,25 +363,27 @@ bool Scanner::SkipWhiteSpace() {
     // line (with only whitespace in front of it), we treat the rest
     // of the line as a comment. This is in line with the way
     // SpiderMonkey handles it.
-    if (c0_ == '-' && has_line_terminator_before_next_) {
-      Advance();
-      if (c0_ == '-') {
-        Advance();
-        if (c0_ == '>') {
-          // Treat the rest of the line as a comment.
-          SkipSingleLineComment();
-          // Continue skipping white space after the comment.
-          continue;
-        }
-        PushBack('-');  // undo Advance()
-      }
+    if (c0_ != '-' || !has_line_terminator_before_next_) break;
+
+    Advance();
+    if (c0_ != '-') {
       PushBack('-');  // undo Advance()
+      break;
+    }
+
+    Advance();
+    if (c0_ != '>') {
+      PushBack2('-', '-');  // undo 2x Advance();
+      break;
     }
-    // Return whether or not we skipped any characters.
-    return source_pos() != start_position;
+
+    // Treat the rest of the line as a comment.
+    SkipSingleLineComment();
   }
-}
 
+  // Return whether or not we skipped any characters.
+  return source_pos() != start_position;
+}
 
 Token::Value Scanner::SkipSingleLineComment() {
   Advance();
@@ -485,24 +487,24 @@ Token::Value Scanner::SkipMultiLineComment() {
   return Token::ILLEGAL;
 }
 
-
 Token::Value Scanner::ScanHtmlComment() {
   // Check for <!-- comments.
   DCHECK(c0_ == '!');
   Advance();
-  if (c0_ == '-') {
-    Advance();
-    if (c0_ == '-') {
-      found_html_comment_ = true;
-      return SkipSingleLineComment();
-    }
-    PushBack('-');  // undo Advance()
+  if (c0_ != '-') {
+    PushBack('!');  // undo Advance()
+    return Token::LT;
+  }
+
+  Advance();
+  if (c0_ != '-') {
+    PushBack2('-', '!');  // undo 2x Advance()
+    return Token::LT;
   }
-  PushBack('!');  // undo Advance()
-  DCHECK(c0_ == '!');
-  return Token::LT;
-}
 
+  found_html_comment_ = true;
+  return SkipSingleLineComment();
+}
 
 void Scanner::Scan() {
   next_.literal_chars = NULL;

src/parsing/scanner.h

@@ -617,6 +617,15 @@ class Scanner {
     c0_ = ch;
   }
 
+  // Same as PushBack(ch1); PushBack(ch2).
+  // - Potentially more efficient as it uses Back2() on the stream.
+  // - Uses char as parameters, since we're only calling it with ASCII chars in
+  //   practice. This way, we can avoid a few edge cases.
+  void PushBack2(char ch1, char ch2) {
+    source_->Back2();
+    c0_ = ch2;
+  }
+
   inline Token::Value Select(Token::Value tok) {
     Advance();
     return tok;

test/cctest/parsing/test-scanner.cc

@@ -25,6 +25,10 @@ std::unique_ptr<Scanner> make_scanner(const char* src) {
 
 }  // anonymous namespace
 
+// DCHECK_TOK checks token equality, but by checking for equality of the token
+// names. That should have the same result, but has much nicer error messaages.
+#define DCHECK_TOK(a, b) DCHECK_EQ(Token::Name(a), Token::Name(b))
+
 TEST(Bookmarks) {
   // Scan through the given source and record the tokens for use as reference
   // below.
@@ -51,12 +55,33 @@ TEST(Bookmarks) {
       if (i == bookmark_pos) {
         bookmark.Set();
       }
-      DCHECK_EQ(tokens[i], scanner->Next());
+      DCHECK_TOK(tokens[i], scanner->Next());
     }
 
     bookmark.Apply();
     for (size_t i = bookmark_pos; i < tokens.size(); i++) {
-      DCHECK_EQ(tokens[i], scanner->Next());
+      DCHECK_TOK(tokens[i], scanner->Next());
+    }
+  }
+}
+
+TEST(AllThePushbacks) {
+  const struct {
+    const char* src;
+    const Token::Value tokens[5];  // Large enough for any of the test cases.
+  } test_cases[] = {
+      {"<-x", {Token::LT, Token::SUB, Token::IDENTIFIER, Token::EOS}},
+      {"<!x", {Token::LT, Token::NOT, Token::IDENTIFIER, Token::EOS}},
+      {"<!-x",
+       {Token::LT, Token::NOT, Token::SUB, Token::IDENTIFIER, Token::EOS}},
+      {"<!-- xx -->\nx", {Token::IDENTIFIER, Token::EOS}},
+  };
+
+  for (const auto& test_case : test_cases) {
+    auto scanner = make_scanner(test_case.src);
+    for (size_t i = 0; test_case.tokens[i] != Token::EOS; i++) {
+      DCHECK_TOK(test_case.tokens[i], scanner->Next());
     }
+    DCHECK_TOK(Token::EOS, scanner->Next());
   }
 }