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

src/parsing/scanner.cc

Index: src/parsing/scanner.cc
diff --git a/src/parsing/scanner.cc b/src/parsing/scanner.cc
index c1580bbeae7dd23d69d0e02c13b0090474cb1773..c961ff758e77f96ff76e9ff8e96e5c2d1d3f4d94 100644
--- a/src/parsing/scanner.cc
+++ b/src/parsing/scanner.cc
@@ -397,6 +397,7 @@ Token::Value Scanner::Next() {
       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;
     }
@@ -609,6 +610,7 @@ Token::Value Scanner::ScanHtmlComment() {
 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
@@ -889,6 +891,8 @@ void Scanner::SanityCheckTokenDesc(const TokenDesc& token) const {
   // - 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:
@@ -909,10 +913,12 @@ void Scanner::SanityCheckTokenDesc(const TokenDesc& token) const {
     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;
   }
 }
@@ -1117,10 +1123,11 @@ Token::Value Scanner::ScanTemplateSpan() {
         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
@@ -1736,7 +1743,9 @@ void Scanner::SeekNext(size_t position) {
   // 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,