Allow duplicate property names in classes

ES6 no longer makes duplicate properties an error. However, we
continue to treat duplicate properties in strict mode object
literals as errors. With this change we allow duplicate properties
in class bodies. We continue to flag duplicate constructors as an
error as required by ES6.

BUG=v8:3570
LOG=Y
R=marja@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#24933}
git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@24933 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/messages.js b/src/messages.js
index 6578e8d..513ffdc 100644 (file)
--- a/src/messages.js
+++ b/src/messages.js
@@ -176,7 +176,8 @@ var kMessages = {
   module_export_undefined:       ["Export '", "%0", "' is not defined in module"],
   unexpected_super:              ["'super' keyword unexpected here"],
   extends_value_not_a_function:  ["Class extends value ", "%0", " is not a function or null"],
-  prototype_parent_not_an_object: ["Class extends value does not have valid prototype property ", "%0"]
+  prototype_parent_not_an_object: ["Class extends value does not have valid prototype property ", "%0"],
+  duplicate_constructor:         ["A class may only have one constructor"]
 };
 
 

diff --git a/src/preparser.h b/src/preparser.h
index a53abfe..2db88e7 100644 (file)
--- a/src/preparser.h
+++ b/src/preparser.h
@@ -476,6 +476,7 @@ class ParserBase : public Traits {
   ExpressionT ParseObjectLiteral(bool* ok);
   ObjectLiteralPropertyT ParsePropertyDefinition(ObjectLiteralChecker* checker,
                                                  bool in_class, bool is_static,
+                                                 bool* has_seen_constructor,
                                                  bool* ok);
   typename Traits::Type::ExpressionList ParseArguments(bool* ok);
   ExpressionT ParseAssignmentExpression(bool accept_IN, bool* ok);
@@ -1183,10 +1184,7 @@ class PreParserTraits {
     return false;
   }
 
-  bool IsConstructorProperty(PreParserExpression property) { return false; }
-
   static PreParserExpression GetPropertyValue(PreParserExpression property) {
-    UNREACHABLE();
     return PreParserExpression::Default();
   }
 
@@ -1925,7 +1923,9 @@ typename ParserBase<Traits>::IdentifierT ParserBase<Traits>::ParsePropertyName(
 template <class Traits>
 typename ParserBase<Traits>::ObjectLiteralPropertyT ParserBase<
     Traits>::ParsePropertyDefinition(ObjectLiteralChecker* checker,
-                                     bool in_class, bool is_static, bool* ok) {
+                                     bool in_class, bool is_static,
+                                     bool* has_seen_constructor, bool* ok) {
+  DCHECK(!in_class || is_static || has_seen_constructor != NULL);
   ExpressionT value = this->EmptyExpression();
   bool is_get = false;
   bool is_set = false;
@@ -1942,8 +1942,10 @@ typename ParserBase<Traits>::ObjectLiteralPropertyT ParserBase<
 
   if (!in_class && !is_generator && peek() == Token::COLON) {
     // PropertyDefinition : PropertyName ':' AssignmentExpression
-    checker->CheckProperty(name_token, kValueProperty,
-                           CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
+    if (checker != NULL) {
+      checker->CheckProperty(name_token, kValueProperty,
+                             CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
+    }
     Consume(Token::COLON);
     value = this->ParseAssignmentExpression(
         true, CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
@@ -1968,11 +1970,20 @@ typename ParserBase<Traits>::ObjectLiteralPropertyT ParserBase<
         return this->EmptyObjectLiteralProperty();
       }
 
+      if (*has_seen_constructor) {
+        ReportMessageAt(scanner()->location(), "duplicate_constructor");
+        *ok = false;
+        return this->EmptyObjectLiteralProperty();
+      }
+
+      *has_seen_constructor = true;
       kind = FunctionKind::kNormalFunction;
     }
 
-    checker->CheckProperty(name_token, kValueProperty,
-                           CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
+    if (checker != NULL) {
+      checker->CheckProperty(name_token, kValueProperty,
+                             CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
+    }
 
     value = this->ParseFunctionLiteral(
         name, scanner()->location(),
@@ -1983,7 +1994,7 @@ typename ParserBase<Traits>::ObjectLiteralPropertyT ParserBase<
 
   } else if (in_class && name_is_static && !is_static) {
     // static MethodDefinition
-    return ParsePropertyDefinition(checker, true, true, ok);
+    return ParsePropertyDefinition(checker, true, true, NULL, ok);
 
   } else if (is_get || is_set) {
     // Accessor
@@ -1998,16 +2009,15 @@ typename ParserBase<Traits>::ObjectLiteralPropertyT ParserBase<
       *ok = false;
       return this->EmptyObjectLiteralProperty();
     } else if (in_class && !is_static && this->IsConstructor(name)) {
-      // ES6, spec draft rev 27, treats static get constructor as an error too.
-      // https://bugs.ecmascript.org/show_bug.cgi?id=3223
-      // TODO(arv): Update when bug is resolved.
       ReportMessageAt(scanner()->location(), "constructor_special_method");
       *ok = false;
       return this->EmptyObjectLiteralProperty();
     }
-    checker->CheckProperty(name_token,
-                           is_get ? kGetterProperty : kSetterProperty,
-                           CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
+    if (checker != NULL) {
+      checker->CheckProperty(name_token,
+                             is_get ? kGetterProperty : kSetterProperty,
+                             CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
+    }
 
     typename Traits::Type::FunctionLiteral value = this->ParseFunctionLiteral(
         name, scanner()->location(),
@@ -2061,8 +2071,8 @@ typename ParserBase<Traits>::ExpressionT ParserBase<Traits>::ParseObjectLiteral(
 
     const bool in_class = false;
     const bool is_static = false;
-    ObjectLiteralPropertyT property =
-        this->ParsePropertyDefinition(&checker, in_class, is_static, CHECK_OK);
+    ObjectLiteralPropertyT property = this->ParsePropertyDefinition(
+        &checker, in_class, is_static, NULL, CHECK_OK);
 
     // Mark top-level object literals that contain function literals and
     // pretenure the literal so it can be added as a constant function
@@ -2744,22 +2754,22 @@ typename ParserBase<Traits>::ExpressionT ParserBase<Traits>::ParseClassLiteral(
   scope_->SetStrictMode(STRICT);
   scope_->SetScopeName(name);
 
-  ObjectLiteralChecker checker(this, STRICT);
   typename Traits::Type::PropertyList properties =
       this->NewPropertyList(4, zone_);
   ExpressionT constructor = this->EmptyExpression();
+  bool has_seen_constructor = false;
 
   Expect(Token::LBRACE, CHECK_OK);
   while (peek() != Token::RBRACE) {
     if (Check(Token::SEMICOLON)) continue;
     if (fni_ != NULL) fni_->Enter();
-
     const bool in_class = true;
     const bool is_static = false;
-    ObjectLiteralPropertyT property =
-        this->ParsePropertyDefinition(&checker, in_class, is_static, CHECK_OK);
+    bool old_has_seen_constructor = has_seen_constructor;
+    ObjectLiteralPropertyT property = this->ParsePropertyDefinition(
+        NULL, in_class, is_static, &has_seen_constructor, CHECK_OK);
 
-    if (this->IsConstructorProperty(property)) {
+    if (has_seen_constructor != old_has_seen_constructor) {
       constructor = this->GetPropertyValue(property);
     } else {
       properties->Add(property, zone());

diff --git a/test/cctest/test-parsing.cc b/test/cctest/test-parsing.cc
index 4028167..55c5b6c 100644 (file)
--- a/test/cctest/test-parsing.cc
+++ b/test/cctest/test-parsing.cc
@@ -4041,9 +4041,6 @@ TEST(ClassConstructorNoErrors) {
 
 
 TEST(ClassMultipleConstructorErrors) {
-  // We currently do not allow any duplicate properties in class bodies. This
-  // test ensures that when we change that we still throw on duplicate
-  // constructors.
   const char* context_data[][2] = {{"class C {", "}"},
                                    {"(class {", "});"},
                                    {NULL, NULL}};
@@ -4061,9 +4058,7 @@ TEST(ClassMultipleConstructorErrors) {
 }
 
 
-// TODO(arv): We should allow duplicate property names.
-// https://code.google.com/p/v8/issues/detail?id=3570
-DISABLED_TEST(ClassMultiplePropertyNamesNoErrors) {
+TEST(ClassMultiplePropertyNamesNoErrors) {
   const char* context_data[][2] = {{"class C {", "}"},
                                    {"(class {", "});"},
                                    {NULL, NULL}};
@@ -4072,6 +4067,8 @@ DISABLED_TEST(ClassMultiplePropertyNamesNoErrors) {
     "constructor() {}; static constructor() {}",
     "m() {}; static m() {}",
     "m() {}; m() {}",
+    "static m() {}; static m() {}",
+    "get m() {}; set m(_) {}; get m() {}; set m(_) {};",
     NULL};
 
   static const ParserFlag always_flags[] = {