Simplify Scanner bookmarking.

src/parsing/scanner.cc

Index: src/parsing/scanner.cc
diff --git a/src/parsing/scanner.cc b/src/parsing/scanner.cc
index be4100a0971688dea6165b9bf77cabe8a190185b..dbdab3929d3b20fab976855533a8b054e081f057 100644
--- a/src/parsing/scanner.cc
+++ b/src/parsing/scanner.cc
@@ -19,9 +19,6 @@
 namespace v8 {
 namespace internal {
 
-const size_t Utf16CharacterStream::kNoBookmark =
-    std::numeric_limits<size_t>::max();
-
 Handle<String> Scanner::LiteralBuffer::Internalize(Isolate* isolate) const {
   if (is_one_byte()) {
     return isolate->factory()->InternalizeOneByteString(one_byte_literal());
@@ -29,21 +26,42 @@ Handle<String> Scanner::LiteralBuffer::Internalize(Isolate* isolate) const {
   return isolate->factory()->InternalizeTwoByteString(two_byte_literal());
 }
 
+// ----------------------------------------------------------------------------
+// Scanner::BookmarkScope
+
+const size_t Scanner::BookmarkScope::kNoBookmark =
+    std::numeric_limits<size_t>::max() - 1;
+const size_t Scanner::BookmarkScope::kBookmarkWasApplied =
+    std::numeric_limits<size_t>::max();
+
+void Scanner::BookmarkScope::Set() {
+  DCHECK_EQ(bookmark_, kNoBookmark);
+  DCHECK_EQ(scanner_->next_next_.token, Token::UNINITIALIZED);
+  bookmark_ = scanner_->location().beg_pos;
+}
+
+void Scanner::BookmarkScope::Reset() {

[marja, 2016/09/19 07:50:22]

IMO "Reset" is not the correct name for this function. "Reset" means more like
"clear the current value", whereas this actually takes the bookmark into use!
How about Apply?

[vogelheim, 2016/09/20 11:52:44]

Done.

+  DCHECK(HasBeenSet());  // Caller hasn't called SetBookmark.
+  scanner_->Seek(bookmark_);
+  bookmark_ = kBookmarkWasApplied;
+}
+
+bool Scanner::BookmarkScope::HasBeenSet() {
+  return bookmark_ != kNoBookmark && bookmark_ != kBookmarkWasApplied;
+}
 
+bool Scanner::BookmarkScope::HasBeenReset() {
+  return bookmark_ == kBookmarkWasApplied;

[marja, 2016/09/19 07:50:22]

... here you already use the term "apply" :)

[vogelheim, 2016/09/20 11:52:44]

Done.

+}
 
 // ----------------------------------------------------------------------------
 // Scanner
 
 Scanner::Scanner(UnicodeCache* unicode_cache)
     : unicode_cache_(unicode_cache),
-      bookmark_c0_(kNoBookmark),
       octal_pos_(Location::invalid()),
       decimal_with_leading_zero_pos_(Location::invalid()),
       found_html_comment_(false) {
-  bookmark_current_.literal_chars = &bookmark_current_literal_;
-  bookmark_current_.raw_literal_chars = &bookmark_current_raw_literal_;
-  bookmark_next_.literal_chars = &bookmark_next_literal_;
-  bookmark_next_.raw_literal_chars = &bookmark_next_raw_literal_;
 }
 
 
@@ -1584,59 +1602,21 @@ int Scanner::FindSymbol(DuplicateFinder* finder, int value) {
   return finder->AddTwoByteSymbol(literal_two_byte_string(), value);
 }
 
-
-bool Scanner::SetBookmark() {
-  if (c0_ != kNoBookmark && bookmark_c0_ == kNoBookmark &&
-      next_next_.token == Token::UNINITIALIZED && source_->SetBookmark()) {
-    bookmark_c0_ = c0_;
-    CopyTokenDesc(&bookmark_current_, &current_);
-    CopyTokenDesc(&bookmark_next_, &next_);
-    return true;
-  }
-  return false;
-}
-
-
-void Scanner::ResetToBookmark() {
-  DCHECK(BookmarkHasBeenSet());  // Caller hasn't called SetBookmark.
-
-  source_->ResetToBookmark();
-  c0_ = bookmark_c0_;
-  CopyToNextTokenDesc(&bookmark_current_);
-  current_ = next_;
-  CopyToNextTokenDesc(&bookmark_next_);
-  bookmark_c0_ = kBookmarkWasApplied;
-}
-
-
-bool Scanner::BookmarkHasBeenSet() { return bookmark_c0_ >= 0; }
-
-
-bool Scanner::BookmarkHasBeenReset() {
-  return bookmark_c0_ == kBookmarkWasApplied;
-}
-
-
-void Scanner::DropBookmark() { bookmark_c0_ = kNoBookmark; }
-
-void Scanner::CopyToNextTokenDesc(TokenDesc* from) {
-  StartLiteral();
-  StartRawLiteral();
-  CopyTokenDesc(&next_, from);
-  if (next_.literal_chars->length() == 0) next_.literal_chars = nullptr;
-  if (next_.raw_literal_chars->length() == 0) next_.raw_literal_chars = nullptr;
-}
-
-void Scanner::CopyTokenDesc(TokenDesc* to, TokenDesc* from) {
-  DCHECK_NOT_NULL(to);
-  DCHECK_NOT_NULL(from);
-  to->token = from->token;
-  to->location = from->location;
-  to->literal_chars->CopyFrom(from->literal_chars);
-  to->raw_literal_chars->CopyFrom(from->raw_literal_chars);
+void Scanner::Seek(size_t position) {
+  // Use with care: This cleanly resets most, but not all scanner state.
+  // TODO(vogelheim): Fix this, or at least DCHECK the relevant conditions.
+
+  // To re-scan from a given character position, we need to:
+  // 1, Reset the next_ and next_next_ tokens (since they'll be used by Next(),
+  next_.token = Token::UNINITIALIZED;
+  next_next_.token = Token::UNINITIALIZED;
+  // 2, reset the source to the desired position,
+  source_->Seek(position);
+  // 3, re-scan, by scanning the look-ahead char + 2 tokens (current_ + next_).
+  c0_ = source_->Advance();
+  Scan();
+  Scan();
 }
 
-
-
 }  // namespace internal
 }  // namespace v8