[parser] Correctly handle invalid escapes in adjacent template tokens.

src/parsing/scanner.cc

 // Copyright 2011 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Features shared by parsing and pre-parsing scanners.
 
 #include "src/parsing/scanner.h"
 
 #include <stdint.h>
 
@@ skipping 379 matching lines @@
   has_multiline_comment_before_next_ = false;
   if (static_cast<unsigned>(c0_) <= 0x7f) {
     Token::Value token = static_cast<Token::Value>(one_char_tokens[c0_]);
     if (token != Token::ILLEGAL) {
       int pos = source_pos();
       next_.token = token;
       next_.location.beg_pos = pos;
       next_.location.end_pos = pos + 1;
       next_.literal_chars = nullptr;
       next_.raw_literal_chars = nullptr;
+      next_.invalid_template_escape_message_ = MessageTemplate::kNone;
       Advance();
       return current_.token;
     }
   }
   Scan();
   return current_.token;
 }
 
 
 Token::Value Scanner::PeekAhead() {
@@ skipping 192 matching lines @@
     return Token::LT;
   }
 
   found_html_comment_ = true;
   return SkipSingleLineComment();
 }
 
 void Scanner::Scan() {
   next_.literal_chars = NULL;
   next_.raw_literal_chars = NULL;
+  next_.invalid_template_escape_message_ = MessageTemplate::kNone;
   Token::Value token;
   do {
     // Remember the position of the next token
     next_.location.beg_pos = source_pos();
 
     switch (c0_) {
       case ' ':
       case '\t':
         Advance();
         token = Token::WHITESPACE;
@@ skipping 260 matching lines @@
 }
 
 #ifdef DEBUG
 void Scanner::SanityCheckTokenDesc(const TokenDesc& token) const {
   // Most tokens should not have literal_chars or even raw_literal chars.
   // The rules are:
   // - UNINITIALIZED: we don't care.
   // - TEMPLATE_*: need both literal + raw literal chars.
   // - IDENTIFIERS, STRINGS, etc.: need a literal, but no raw literal.
   // - all others: should have neither.
+  // Furthermore, only TEMPLATE_* tokens can have a
+  // invalid_template_escape_message_.
 
   switch (token.token) {
     case Token::UNINITIALIZED:
       // token.literal_chars & other members might be garbage. That's ok.
       break;
     case Token::TEMPLATE_SPAN:
     case Token::TEMPLATE_TAIL:
       DCHECK_NOT_NULL(token.raw_literal_chars);
       DCHECK_NOT_NULL(token.literal_chars);
       break;
     case Token::ESCAPED_KEYWORD:
     case Token::ESCAPED_STRICT_RESERVED_WORD:
     case Token::FUTURE_STRICT_RESERVED_WORD:
     case Token::IDENTIFIER:
     case Token::NUMBER:
     case Token::REGEXP_LITERAL:
     case Token::SMI:
     case Token::STRING:
       DCHECK_NOT_NULL(token.literal_chars);
       DCHECK_NULL(token.raw_literal_chars);
+      DCHECK_EQ(token.invalid_template_escape_message_, MessageTemplate::kNone);
       break;
     default:
       DCHECK_NULL(token.literal_chars);
       DCHECK_NULL(token.raw_literal_chars);
+      DCHECK_EQ(token.invalid_template_escape_message_, MessageTemplate::kNone);
       break;
   }
 }
 #endif  // DEBUG
 
 void Scanner::SeekForward(int pos) {
   // After this call, we will have the token at the given position as
   // the "next" token. The "current" token will be invalid.
   if (pos == next_.location.beg_pos) return;
   int current_pos = source_pos();
@@ skipping 184 matching lines @@
           } else {
             AddRawLiteralChar('\n');
           }
         }
       } else {
         bool success = ScanEscape<capture_raw, in_template_literal>();
         USE(success);
         DCHECK_EQ(!success, has_error());
         // For templates, invalid escape sequence checking is handled in the
         // parser.
-        scanner_error_state.MoveErrorTo(&invalid_template_escape_message_,
-                                        &invalid_template_escape_location_);
-        octal_error_state.MoveErrorTo(&invalid_template_escape_message_,
-                                      &invalid_template_escape_location_);
+        scanner_error_state.MoveErrorTo(

[vogelheim, 2017/03/03 10:40:08]

nitpick: With this change, it might be slightly nicer to change the signature of
MoveErrorTo to MoveErrorTo(TokenDesc&), since now there's a 'thing' that
contains both of the variables we're trying to modify here. 

[bakkot1, 2017/03/03 20:50:17]

TokenDesc is private in Scanner. I could make it public, I guess.

On the other hand, at that point it could just be "MoveErrorToNextTokenDesc"
and take no parameters at all. Thoughts?

[vogelheim, 2017/03/03 21:26:19]

Oops. Hrmm, yeah, I think I asked you to take ErrorState out of the header...
:-/


Alternatives:
- Leave things as they are.
- Make TokenDesc public. I'm not fond of it.
- Make ErrorState a friend of Scanner.
- Forward declare Scanner::ErrorState.

I think I'd prefer last or second-to-last, but am happy to let you have the last
word.

(What I mean with the last one is:
Declare: class ErrorState; in the private:-section of Scanner, and in Scanner.cc
implement it as Scanner::ErrorState. When I try it locally (without the rest of
your patch), this seems to work and gives Scanner::ErrorState full access to
Scanner, without polluting the header.)

+            &next_.invalid_template_escape_message_,
+            &next_.invalid_template_escape_location_);
+        octal_error_state.MoveErrorTo(&next_.invalid_template_escape_message_,
+                                      &next_.invalid_template_escape_location_);
       }
     } else if (c < 0) {
       // Unterminated template literal
       PushBack(c);
       break;
     } else {
       // The TRV of LineTerminatorSequence :: <CR> is the CV 0x000A.
       // The TRV of LineTerminatorSequence :: <CR><LF> is the sequence
       // consisting of the CV 0x000A.
       if (c == '\r') {
@@ skipping 595 matching lines @@
 }
 
 void Scanner::SeekNext(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 current_, next_ and next_next_ tokens
   //    (next_ + next_next_ will be overwrittem by Next(),
   //     current_ will remain unchanged, so overwrite it fully.)
-  current_ = {{0, 0}, nullptr, nullptr, 0, Token::UNINITIALIZED};
+  current_ = {
+      {0, 0}, nullptr, nullptr, 0, Token::UNINITIALIZED, MessageTemplate::kNone,
+      {0, 0}};
   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 + 1 token (next_).
   c0_ = source_->Advance();
   Next();
   DCHECK_EQ(next_.location.beg_pos, static_cast<int>(position));
 }
 
 }  // namespace internal
 }  // namespace v8