Allow eval and arguments as property names

We incorrectly disallowed eval and arguments in accessor and method
names. This was because we checked the name inside the
ParseFunctionLiteral. We now flag accessors so that lazy parsing of
these functions are treated correctly.

BUG=v8:1984
R=adamk, dslomov@chromium.org
LOG=N

Review URL: https://codereview.chromium.org/899363002

Cr-Commit-Position: refs/heads/master@{#26497}

src/parser.cc

@@ -3676,9 +3676,9 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
     // We don't yet know if the function will be strict, so we cannot yet
     // produce errors for parameter names or duplicates. However, we remember
     // the locations of these errors if they occur and produce the errors later.
-    Scanner::Location eval_args_error_log = Scanner::Location::invalid();
+    Scanner::Location eval_args_error_loc = Scanner::Location::invalid();
     Scanner::Location dupe_error_loc = Scanner::Location::invalid();
-    Scanner::Location reserved_loc = Scanner::Location::invalid();
+    Scanner::Location reserved_error_loc = Scanner::Location::invalid();
 
     bool is_rest = false;
     bool done = arity_restriction == FunctionLiteral::GETTER_ARITY ||
@@ -3695,11 +3695,11 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
           ParseIdentifierOrStrictReservedWord(&is_strict_reserved, CHECK_OK);
 
       // Store locations for possible future error reports.
-      if (!eval_args_error_log.IsValid() && IsEvalOrArguments(param_name)) {
-        eval_args_error_log = scanner()->location();
+      if (!eval_args_error_loc.IsValid() && IsEvalOrArguments(param_name)) {
+        eval_args_error_loc = scanner()->location();
       }
-      if (!reserved_loc.IsValid() && is_strict_reserved) {
-        reserved_loc = scanner()->location();
+      if (!reserved_error_loc.IsValid() && is_strict_reserved) {
+        reserved_error_loc = scanner()->location();
       }
       if (!dupe_error_loc.IsValid() && scope_->IsDeclared(param_name)) {
         duplicate_parameters = FunctionLiteral::kHasDuplicateParameters;
@@ -3810,19 +3810,16 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
       handler_count = function_state.handler_count();
     }
 
-    // Validate strict mode.
-    // Concise methods use StrictFormalParameters.
-    // Functions for which IsSimpleParameterList() returns false use
-    // StrictFormalParameters.
-    if (is_strict(language_mode()) || IsConciseMethod(kind) || is_rest) {
-      CheckStrictFunctionNameAndParameters(function_name,
-                                           name_is_strict_reserved,
-                                           function_name_location,
-                                           eval_args_error_log,
-                                           dupe_error_loc,
-                                           reserved_loc,
-                                           CHECK_OK);
-    }
+    // Validate name and parameter names. We can do this only after parsing the
+    // function, since the function can declare itself strict.
+    CheckFunctionName(language_mode(), kind, function_name,
+                      name_is_strict_reserved, function_name_location,
+                      CHECK_OK);
+    const bool use_strict_params = is_rest || IsConciseMethod(kind);
+    CheckFunctionParameterNames(language_mode(), use_strict_params,
+                                eval_args_error_loc, dupe_error_loc,
+                                reserved_error_loc, CHECK_OK);
+
     if (is_strict(language_mode())) {
       CheckStrictOctalLiteral(scope->start_position(), scope->end_position(),
                               CHECK_OK);

src/preparser.cc

@@ -936,36 +936,16 @@ PreParser::Expression PreParser::ParseFunctionLiteral(
   }
   Expect(Token::RBRACE, CHECK_OK);
 
-  // Validate strict mode. We can do this only after parsing the function,
-  // since the function can declare itself strict.
-  // Concise methods use StrictFormalParameters.
-  if (is_strict(language_mode()) || IsConciseMethod(kind) || is_rest) {
-    if (function_name.IsEvalOrArguments()) {
-      ReportMessageAt(function_name_location, "strict_eval_arguments");
-      *ok = false;
-      return Expression::Default();
-    }
-    if (name_is_strict_reserved) {
-      ReportMessageAt(function_name_location, "unexpected_strict_reserved");
-      *ok = false;
-      return Expression::Default();
-    }
-    if (eval_args_error_loc.IsValid()) {
-      ReportMessageAt(eval_args_error_loc, "strict_eval_arguments");
-      *ok = false;
-      return Expression::Default();
-    }
-    if (dupe_error_loc.IsValid()) {
-      ReportMessageAt(dupe_error_loc, "strict_param_dupe");
-      *ok = false;
-      return Expression::Default();
-    }
-    if (reserved_error_loc.IsValid()) {
-      ReportMessageAt(reserved_error_loc, "unexpected_strict_reserved");
-      *ok = false;
-      return Expression::Default();
-    }
+  // Validate name and parameter names. We can do this only after parsing the
+  // function, since the function can declare itself strict.
+  CheckFunctionName(language_mode(), kind, function_name,
+                    name_is_strict_reserved, function_name_location, CHECK_OK);
+  const bool use_strict_params = is_rest || IsConciseMethod(kind);
+  CheckFunctionParameterNames(language_mode(), use_strict_params,
+                              eval_args_error_loc, dupe_error_loc,
+                              reserved_error_loc, CHECK_OK);
 
+  if (is_strict(language_mode())) {
     int end_position = scanner()->location().end_pos;
     CheckStrictOctalLiteral(start_position, end_position, CHECK_OK);
   }

src/preparser.h

@@ -426,17 +426,18 @@ class ParserBase : public Traits {
     CheckOctalLiteral(beg_pos, end_pos, "template_octal_literal", ok);
   }
 
-  // Validates strict mode for function parameter lists. This has to be
-  // done after parsing the function, since the function can declare
-  // itself strict.
-  void CheckStrictFunctionNameAndParameters(
-      IdentifierT function_name,
-      bool function_name_is_strict_reserved,
-      const Scanner::Location& function_name_loc,
-      const Scanner::Location& eval_args_error_loc,
-      const Scanner::Location& dupe_error_loc,
-      const Scanner::Location& reserved_loc,
-      bool* ok) {
+  // Checking the name of a function literal. This has to be done after parsing
+  // the function, since the function can declare itself strict.
+  void CheckFunctionName(LanguageMode language_mode, FunctionKind kind,
+                         IdentifierT function_name,
+                         bool function_name_is_strict_reserved,
+                         const Scanner::Location& function_name_loc,
+                         bool* ok) {
+    // Property names are never checked.
+    if (IsConciseMethod(kind) || IsAccessorFunction(kind)) return;
+    // The function name needs to be checked in strict mode.
+    if (is_sloppy(language_mode)) return;
+
     if (this->IsEvalOrArguments(function_name)) {
       Traits::ReportMessageAt(function_name_loc, "strict_eval_arguments");
       *ok = false;
@@ -447,11 +448,25 @@ class ParserBase : public Traits {
       *ok = false;
       return;
     }
+  }
+
+  // Checking the parameter names of a function literal. This has to be done
+  // after parsing the function, since the function can declare itself strict.
+  void CheckFunctionParameterNames(LanguageMode language_mode,
+                                   bool strict_params,
+                                   const Scanner::Location& eval_args_error_loc,
+                                   const Scanner::Location& dupe_error_loc,
+                                   const Scanner::Location& reserved_loc,
+                                   bool* ok) {
+    if (is_sloppy(language_mode) && !strict_params) return;
+
     if (eval_args_error_loc.IsValid()) {
       Traits::ReportMessageAt(eval_args_error_loc, "strict_eval_arguments");
       *ok = false;
       return;
     }
+    // TODO(arv): When we add support for destructuring in setters we also need
+    // to check for duplicate names.
     if (dupe_error_loc.IsValid()) {
       Traits::ReportMessageAt(dupe_error_loc, "strict_param_dupe");
       *ok = false;
@@ -2838,6 +2853,7 @@ ParserBase<Traits>::ParseArrowFunctionLiteral(int start_pos,
                                  Traits::Type::ptr_to_scope(scope),
                                  kArrowFunction, &function_factory);
     Scanner::Location dupe_error_loc = Scanner::Location::invalid();
+    // TODO(arv): Pass in eval_args_error_loc and reserved_loc here.
     num_parameters = Traits::DeclareArrowParametersFromExpression(
         params_ast, scope_, &dupe_error_loc, ok);
     if (!*ok) {
@@ -2891,14 +2907,13 @@ ParserBase<Traits>::ParseArrowFunctionLiteral(int start_pos,
     scope->set_end_position(scanner()->location().end_pos);
 
     // Arrow function *parameter lists* are always checked as in strict mode.
-    bool function_name_is_strict_reserved = false;
-    Scanner::Location function_name_loc = Scanner::Location::invalid();
+    // TODO(arv): eval_args_error_loc and reserved_loc needs to be set by
+    // DeclareArrowParametersFromExpression.
     Scanner::Location eval_args_error_loc = Scanner::Location::invalid();
     Scanner::Location reserved_loc = Scanner::Location::invalid();
-    this->CheckStrictFunctionNameAndParameters(
-        this->EmptyIdentifier(), function_name_is_strict_reserved,
-        function_name_loc, eval_args_error_loc, dupe_error_loc, reserved_loc,
-        CHECK_OK);
+    const bool use_strict_params = true;
+    this->CheckFunctionParameterNames(language_mode(), use_strict_params,
+        eval_args_error_loc, dupe_error_loc, reserved_loc, CHECK_OK);
 
     // Validate strict mode.
     if (is_strict(language_mode())) {

test/cctest/test-parsing.cc

@@ -5104,3 +5104,48 @@ TEST(LanguageModeDirectives) {
   TestLanguageMode("\"use some future directive\"; \"use strict\";", i::STRICT);
   TestLanguageMode("\"use some future directive\"; \"use strong\";", i::STRONG);
 }
+
+
+TEST(PropertyNameEvalArguments) {
+  const char* context_data[][2] = {{"'use strict';", ""},
+                                   {"'use strong';", ""},
+                                   {NULL, NULL}};
+
+  const char* statement_data[] = {
+      "({eval: 1})",
+      "({arguments: 1})",
+      "({eval() {}})",
+      "({arguments() {}})",
+      "({*eval() {}})",
+      "({*arguments() {}})",
+      "({get eval() {}})",
+      "({get arguments() {}})",
+      "({set eval(_) {}})",
+      "({set arguments(_) {}})",
+
+      "class C {eval() {}}",
+      "class C {arguments() {}}",
+      "class C {*eval() {}}",
+      "class C {*arguments() {}}",
+      "class C {get eval() {}}",
+      "class C {get arguments() {}}",
+      "class C {set eval(_) {}}",
+      "class C {set arguments(_) {}}",
+
+      "class C {static eval() {}}",
+      "class C {static arguments() {}}",
+      "class C {static *eval() {}}",
+      "class C {static *arguments() {}}",
+      "class C {static get eval() {}}",
+      "class C {static get arguments() {}}",
+      "class C {static set eval(_) {}}",
+      "class C {static set arguments(_) {}}",
+
+      NULL};
+
+  static const ParserFlag always_flags[] = {
+      kAllowHarmonyClasses, kAllowHarmonyObjectLiterals, kAllowHarmonyScoping,
+      kAllowStrongMode};
+  RunParserSyncTest(context_data, statement_data, kSuccess, NULL, 0,
+                    always_flags, arraysize(always_flags));
+}

test/mjsunit/harmony/class-property-name-eval-arguments.js

+// Copyright 2015 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.
+
+// Flags: --harmony-classes --harmony-sloppy
+
+
+(function Method() {
+  class C {
+    eval() {
+      return 1;
+    }
+    arguments() {
+      return 2;
+    }
+    static eval() {
+      return 3;
+    }
+    static arguments() {
+      return 4;
+    }
+  };
+
+  assertEquals(1, new C().eval());
+  assertEquals(2, new C().arguments());
+  assertEquals(3, C.eval());
+  assertEquals(4, C.arguments());
+})();
+
+
+(function Getters() {
+  class C {
+    get eval() {
+      return 1;
+    }
+    get arguments() {
+      return 2;
+    }
+    static get eval() {
+      return 3;
+    }
+    static get arguments() {
+      return 4;
+    }
+  };
+
+  assertEquals(1, new C().eval);
+  assertEquals(2, new C().arguments);
+  assertEquals(3, C.eval);
+  assertEquals(4, C.arguments);
+})();
+
+
+(function Setters() {
+  var x = 0;
+  class C {
+    set eval(v) {
+      x = v;
+    }
+    set arguments(v) {
+      x = v;
+    }
+    static set eval(v) {
+      x = v;
+    }
+    static set arguments(v) {
+      x = v;
+    }
+  };
+
+  new C().eval = 1;
+  assertEquals(1, x);
+  new C().arguments = 2;
+  assertEquals(2, x);
+  C.eval = 3;
+  assertEquals(3, x);
+  C.arguments = 4;
+  assertEquals(4, x);
+})();

test/mjsunit/harmony/method-name-eval-arguments.js

+// Copyright 2015 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.
+
+// Flags: --harmony-object-literals
+
+(function TestSloppyMode() {
+  var o = {
+    eval() {
+      return 1;
+    },
+    arguments() {
+      return 2;
+    },
+  };
+
+  assertEquals(1, o.eval());
+  assertEquals(2, o.arguments());
+})();
+
+(function TestStrictMode() {
+  'use strict';
+
+  var o = {
+    eval() {
+      return 1;
+    },
+    arguments() {
+      return 2;
+    },
+  };
+
+  assertEquals(1, o.eval());
+  assertEquals(2, o.arguments());
+})();

test/mjsunit/property-name-eval-arguments.js

+// Copyright 2015 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.
+
+
+(function TestSloppyMode() {
+  var e = 1, a = 2;
+  var o = {
+    get eval() {
+      return e;
+    },
+    set eval(v) {
+      e = v;
+    },
+    get arguments() {
+      return a;
+    },
+    set arguments(v) {
+      a = v;
+    },
+  };
+
+  assertEquals(1, o.eval);
+  o.eval = 3;
+  assertEquals(3, e);
+
+  assertEquals(2, o.arguments);
+  o.arguments = 4;
+  assertEquals(4, a);
+})();
+
+
+(function TestStrictMode() {
+  'use strict';
+
+  var e = 1, a = 2;
+  var o = {
+    get eval() {
+      return e;
+    },
+    set eval(v) {
+      e = v;
+    },
+    get arguments() {
+      return a;
+    },
+    set arguments(v) {
+      a = v;
+    },
+  };
+
+  assertEquals(1, o.eval);
+  o.eval = 3;
+  assertEquals(3, e);
+
+  assertEquals(2, o.arguments);
+  o.arguments = 4;
+  assertEquals(4, a);
+})();