[parser] Lift template literal invalid escape restriction

src/parsing/scanner.cc

Index: src/parsing/scanner.cc
diff --git a/src/parsing/scanner.cc b/src/parsing/scanner.cc
index 86aadcff9fae2e74e7e6d11172ab2aac91d4f62a..e1eed27b473fb1da313dafd5b865963d31f98fd3 100644
--- a/src/parsing/scanner.cc
+++ b/src/parsing/scanner.cc
@@ -948,16 +948,12 @@ bool Scanner::ScanEscape() {
       break;
   }
 
-  // According to ECMA-262, section 7.8.4, characters not covered by the
-  // above cases should be illegal, but they are commonly handled as
-  // non-escaped characters by JS VMs.
+  // Other escaped characters are interpreted as their non-escaped version.
   AddLiteralChar(c);
   return true;
 }
 
 
-// Octal escapes of the forms '\0xx' and '\xxx' are not a part of
-// ECMA-262. Other JS VMs support them.
 template <bool capture_raw>
 uc32 Scanner::ScanOctalEscape(uc32 c, int length) {
   uc32 x = c - '0';
@@ -1039,6 +1035,16 @@ Token::Value Scanner::ScanTemplateSpan() {
   // TEMPLATE_TAIL terminates a TemplateLiteral and does not need to be
   // followed by an Expression.
 
+  MessageTemplate::Template old_scanner_error = scanner_error_;
+  Location old_scanner_error_location = scanner_error_location_;
+  scanner_error_ = MessageTemplate::kNone;
+  scanner_error_location_ = Location::invalid();

[vogelheim, 2017/02/20 11:23:20]

This logic (here & below) is a bit obtuse. I don't see much away around the
logic itself, but maybe we can reduce the boilerplate a bit. Maybe:

Have a (local) helper class MessageSaveScope that
- saved the message template + location in the constructor
- reconstructs it in the destructor,
- has a bool has_message() const; and a void Reset(); method.

You'd then have two instances of this, for scanner_error + .._location and
octal_message_ + ..._pos_. That should reduce the boilerplate a bit.

[Please decide as you see fit.]

[bakkot1, 2017/02/21 21:54:15]

Added a ErrorState scoped helper which does this. Instead of has_message
and Reset I just have MoveErrorTo, which is what's actually used.

IIRC scoped helpers - at least the BlockState helper - don't usually expose much
of an interface, so this approach seemed more in line with that. Also less code
duplication.

I was tempted to have the constructor take the destination as a parameter too,
and have the destructor do the copy to the destination, but I'm not sure if
that's good style. Also, it would end up reporting the last invalid escape in
the template, instead of the first. Thoughts?

[vogelheim, 2017/02/22 10:03:02]

Thanks.
I think both are fine. I think I like it a little better the way you've now done
it. I'll be happy to let you pick, in case you prefer one or the other.

+
+  MessageTemplate::Template old_octal_message = octal_message_;
+  Location old_octal_pos = octal_pos_;
+  octal_message_ = MessageTemplate::kNone;
+  octal_pos_ = Location::invalid();
+
   Token::Value result = Token::TEMPLATE_SPAN;
   LiteralScope literal(this);
   StartRawLiteral();
@@ -1069,8 +1075,27 @@ Token::Value Scanner::ScanTemplateSpan() {
             AddRawLiteralChar('\n');
           }
         }
-      } else if (!ScanEscape<capture_raw, in_template_literal>()) {
-        return Token::ILLEGAL;
+      } else {
+        ScanEscape<capture_raw, in_template_literal>();
+        // For templates, invalid escape sequence checking is handled in the
+        // parser.
+        if (has_error()) {
+          if (!has_invalid_template_escape()) {
+            invalid_template_escape_message_ = scanner_error_;
+            invalid_template_escape_location_ = scanner_error_location_;
+          }
+          scanner_error_ = MessageTemplate::kNone;
+          scanner_error_location_ = Location::invalid();
+        }
+        if (octal_message_ != MessageTemplate::kNone) {
+          if (!has_invalid_template_escape()) {
+            invalid_template_escape_message_ =
+                MessageTemplate::kTemplateOctalLiteral;
+            invalid_template_escape_location_ = octal_pos_;
+          }
+          octal_message_ = MessageTemplate::kNone;
+          octal_pos_ = Location::invalid();
+        }
       }
     } else if (c < 0) {
       // Unterminated template literal
@@ -1095,6 +1120,11 @@ Token::Value Scanner::ScanTemplateSpan() {
   literal.Complete();
   next_.location.end_pos = source_pos();
   next_.token = result;
+
+  scanner_error_ = old_scanner_error;
+  scanner_error_location_ = old_scanner_error_location;
+  octal_message_ = old_octal_message;
+  octal_pos_ = old_octal_pos;
   return result;
 }