Allow duplicate property names in classes

src/preparser.h

 // Copyright 2012 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.
 
 #ifndef V8_PREPARSER_H
 #define V8_PREPARSER_H
 
 #include "src/v8.h"
 
 #include "src/bailout-reason.h"
@@ skipping 458 matching lines @@
   ExpressionT ParseRegExpLiteral(bool seen_equal, bool* ok);
 
   ExpressionT ParsePrimaryExpression(bool* ok);
   ExpressionT ParseExpression(bool accept_IN, bool* ok);
   ExpressionT ParseArrayLiteral(bool* ok);
   IdentifierT ParsePropertyName(bool* is_get, bool* is_set, bool* is_static,
                                 bool* ok);
   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);
   ExpressionT ParseYieldExpression(bool* ok);
   ExpressionT ParseConditionalExpression(bool accept_IN, bool* ok);
   ExpressionT ParseBinaryExpression(int prec, bool accept_IN, bool* ok);
   ExpressionT ParseUnaryExpression(bool* ok);
   ExpressionT ParsePostfixExpression(bool* ok);
   ExpressionT ParseLeftHandSideExpression(bool* ok);
   ExpressionT ParseMemberWithNewPrefixesExpression(bool* ok);
@@ skipping 687 matching lines @@
 
   static bool IsBoilerplateProperty(PreParserExpression property) {
     // PreParser doesn't count boilerplate properties.
     return false;
   }
 
   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();
   }
 
   // Functions for encapsulating the differences between parsing and preparsing;
   // operations interleaved with the recursive descent.
   static void PushLiteralName(FuncNameInferrer* fni, PreParserIdentifier id) {
     // PreParser should not use FuncNameInferrer.
     UNREACHABLE();
@@ skipping 720 matching lines @@
       return ParseIdentifierNameOrGetOrSet(is_get, is_set, ok);
   }
   UNREACHABLE();
   return this->EmptyIdentifier();
 }
 
 
 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) {
   ExpressionT value = this->EmptyExpression();
   bool is_get = false;
   bool is_set = false;
   bool name_is_static = false;
   bool is_generator = allow_harmony_object_literals_ && Check(Token::MUL);
 
   Token::Value name_token = peek();
   int next_pos = peek_position();
   IdentifierT name =
       ParsePropertyName(&is_get, &is_set, &name_is_static,
                         CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
 
   if (fni_ != NULL) this->PushLiteralName(fni_, name);
 
   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));
 
   } else if (is_generator ||
              (allow_harmony_object_literals_ && peek() == Token::LPAREN)) {
     // Concise Method
 
     if (is_static && this->IsPrototype(name)) {
       ReportMessageAt(scanner()->location(), "static_prototype");
       *ok = false;
       return this->EmptyObjectLiteralProperty();
     }
 
     FunctionKind kind = is_generator ? FunctionKind::kConciseGeneratorMethod
                                      : FunctionKind::kConciseMethod;
 
     if (in_class && !is_static && this->IsConstructor(name)) {
       if (is_generator) {
         ReportMessageAt(scanner()->location(), "constructor_special_method");
         *ok = false;
         return this->EmptyObjectLiteralProperty();
       }
 
+      if (*has_seen_constructor) {

[marja, 2014/10/28 08:30:22]

It's not trivial to see that has_seen_constructor cannot be NULL here... so the
caller must always pass non-NULL if in_class and !is_static, but otherwise it's
ok to pass NULL? This is quite subtle. CHECK that this is followed? Document it
better? Or make this if (has_seen_constructor && *has_seen_constructor)? Always
pass a dummy non-NULL ptr? Idk what would be the best solution. Wdyt?

[arv (Not doing code reviews), 2014/10/28 09:25:55]

I agree that this is ugly. The least I can do is to add an assert.

+        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(),
         false,  // reserved words are allowed here
         kind, RelocInfo::kNoPosition, FunctionLiteral::ANONYMOUS_EXPRESSION,
         FunctionLiteral::NORMAL_ARITY,
         CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
 
   } 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
     bool dont_care = false;
     name_token = peek();
     name = ParsePropertyName(&dont_care, &dont_care, &dont_care,
                              CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
 
     // Validate the property.
     if (is_static && this->IsPrototype(name)) {
       ReportMessageAt(scanner()->location(), "static_prototype");
       *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(),
         false,  // reserved words are allowed here
         FunctionKind::kNormalFunction, RelocInfo::kNoPosition,
         FunctionLiteral::ANONYMOUS_EXPRESSION,
         is_get ? FunctionLiteral::GETTER_ARITY : FunctionLiteral::SETTER_ARITY,
         CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
     return factory()->NewObjectLiteralProperty(is_get, value, next_pos,
                                                is_static);
@@ skipping 33 matching lines @@
 
   ObjectLiteralChecker checker(this, strict_mode());
 
   Expect(Token::LBRACE, CHECK_OK);
 
   while (peek() != Token::RBRACE) {
     if (fni_ != NULL) fni_->Enter();
 
     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
     // property. (Parser only.)
     this->CheckFunctionLiteralInsideTopLevelObjectLiteral(scope_, property,
                                                           &has_function);
 
     // Count CONSTANT or COMPUTED properties to maintain the enumeration order.
     if (this->IsBoilerplateProperty(property)) {
       number_of_boilerplate_properties++;
@@ skipping 661 matching lines @@
     scope_->SetStrictMode(STRICT);
     extends = this->ParseLeftHandSideExpression(CHECK_OK);
   }
 
   // TODO(arv): Implement scopes and name binding in class body only.
   typename Traits::Type::ScopePtr scope = this->NewScope(scope_, BLOCK_SCOPE);
   BlockState block_state(&scope_, Traits::Type::ptr_to_scope(scope));
   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());
     }
 
     if (fni_ != NULL) {
       fni_->Infer();
       fni_->Leave();
     }
   }
@@ skipping 58 matching lines @@
       DCHECK(IsAccessorAccessorConflict(old_type, type));
       // Both accessors of the same type.
       parser()->ReportMessage("accessor_get_set");
     }
     *ok = false;
   }
 }
 } }  // v8::internal
 
 #endif  // V8_PREPARSER_H