Revision: 24933
Author:   a...@chromium.org
Date:     Tue Oct 28 12:23:26 2014 UTC
Log:      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=ma...@chromium.org

Review URL: https://codereview.chromium.org/677953004
https://code.google.com/p/v8/source/detail?r=24933

Modified:
 /branches/bleeding_edge/src/messages.js
 /branches/bleeding_edge/src/preparser.h
 /branches/bleeding_edge/test/cctest/test-parsing.cc

=======================================
--- /branches/bleeding_edge/src/messages.js     Tue Oct 21 17:21:32 2014 UTC
+++ /branches/bleeding_edge/src/messages.js     Tue Oct 28 12:23:26 2014 UTC
@@ -176,7 +176,8 @@
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"]
 };


=======================================
--- /branches/bleeding_edge/src/preparser.h     Thu Oct 23 12:30:20 2014 UTC
+++ /branches/bleeding_edge/src/preparser.h     Tue Oct 28 12:23:26 2014 UTC
@@ -476,6 +476,7 @@
   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);
@@ -1182,11 +1183,8 @@
   static bool IsArrayIndex(PreParserIdentifier string, uint32_t* index) {
     return false;
   }
-
- bool IsConstructorProperty(PreParserExpression property) { return false; }

static PreParserExpression GetPropertyValue(PreParserExpression property) {
-    UNREACHABLE();
     return PreParserExpression::Default();
   }

@@ -1925,7 +1923,9 @@
 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 @@

   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 @@
         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 @@

   } 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 @@
       *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 @@

     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 @@
   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());
=======================================
--- /branches/bleeding_edge/test/cctest/test-parsing.cc Mon Oct 27 16:25:11 2014 UTC +++ /branches/bleeding_edge/test/cctest/test-parsing.cc Tue Oct 28 12:23:26 2014 UTC
@@ -4041,9 +4041,6 @@


 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 @@
 }


-// 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 @@
     "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[] = {

--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to