ParserBase should accept ESCAPED_STRICT_RESERVED_WORD as an identifier

test/cctest/test-parsing.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 1696 matching lines @@
     "++arguments;",
     "eval++;",
     "arguments++;",
     NULL
   };
 
   RunParserSyncTest(context_data, statement_data, kError);
 }
 
 
 TEST(NoErrorsEvalAndArgumentsSloppy) {

[adamk, 2017/02/15 00:29:06]

This is an example of the kind of test I had in mind. It tests parsing behavior
only, while testing both the Parser and the PreParser to make sure they have the
same behavior.

As for the fact that "function l\u0065t() { }" still fails with your patch, that
may be ok (it'd be good to know why, though). Hopefully you'll at least be able
to see it fail easily by using this style of test.

   // Tests that both preparsing and parsing accept "eval" and "arguments" as
   // identifiers when needed.
   const char* context_data[][2] = {
     { "", "" },
     { "function test_func() {", "}"},
     { NULL, NULL }
   };
 
   const char* statement_data[] = {
     "var eval;",
@@ skipping 7714 matching lines @@
       DCHECK_NULL(scope->sibling());
       DCHECK(scope->is_function_scope());
       const i::AstRawString* var_name =
           info.ast_value_factory()->GetOneByteString("my_var");
       i::Variable* var = scope->Lookup(var_name);
       CHECK_EQ(inners[i].ctxt_allocate,
                i::ScopeTestHelper::MustAllocateInContext(var));
     }
   }
 }
+
+TEST(EscapedStrictReservedWord) {
+  // Test that identifiers which are both escaped and only reserved in the
+  // strict mode are accepted in non-strict mode.
+  v8::V8::Initialize();
+  v8::HandleScope scope(CcTest::isolate());
+  v8::Context::Scope context_scope(v8::Context::New(CcTest::isolate()));
+  const char* scripts[] = {
+      "if (true) l\u0065t: ;",           "function l\u0065t() { }",
+      "(function l\u0065t() { })",       "async function l\u0065t() { }",
+      "(async function l\u0065t() { })", "async l\u0065t => 42"};
+  for (const char* script : scripts) {
+    v8::TryCatch try_catch(CcTest::isolate());
+    v8_compile(script);

[vabr (Chromium), 2017/02/14 23:58:53]

I'm not at all sure that I'm doing the right thing here.

If I place some obvious failure to |scripts|, e.g., "var var -", the test fails
by reaching the CHECK below and printing the message.

However, it passes for all |scripts| above, despite d8 showing errors:
d8> function l\u0065t() { }
(d8):1: SyntaxError: Keyword must not contain escaped characters
function l\u0065t() { }
         ^^^^^^^^
SyntaxError: Keyword must not contain escaped characters


So while my fix is obviously not complete yet, I'm not even sure how to test it
properly. Any advice?

+    if (try_catch.HasCaught()) {
+      v8::String::Utf8Value exception(try_catch.Exception());
+      v8::base::OS::Print("Compiling %s failed with %s.", script, *exception);
+      CHECK(false);
+    }
+  }
+}