Move object literal checking into checker 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 143 matching lines @@
     kAllowEvalOrArguments,
     kDontAllowEvalOrArguments
   };
 
   enum Mode {
     PARSE_LAZILY,
     PARSE_EAGERLY
   };
 
   class Checkpoint;
-  class ObjectLiteralChecker;
+  class ObjectLiteralCheckerBase;
 
   // ---------------------------------------------------------------------------
   // FunctionState and BlockState together implement the parser's scope stack.
   // The parser's current scope is in scope_. BlockState and FunctionState
   // constructors push on the scope stack and the destructors pop. They are also
   // used to hold the parser's per-function and per-block state.
   class BlockState BASE_EMBEDDED {
    public:
     BlockState(typename Traits::Type::Scope** scope_stack,
                typename Traits::Type::Scope* scope)
@@ skipping 330 matching lines @@
 
   ExpressionT ParseRegExpLiteral(bool seen_equal, bool* ok);
 
   ExpressionT ParsePrimaryExpression(bool* ok);
   ExpressionT ParseExpression(bool accept_IN, bool* ok);
   ExpressionT ParseArrayLiteral(bool* ok);
   ExpressionT ParsePropertyName(IdentifierT* name, bool* is_get, bool* is_set,
                                 bool* is_static, bool* is_computed_name,
                                 bool* ok);
   ExpressionT ParseObjectLiteral(bool* ok);
-  ObjectLiteralPropertyT ParsePropertyDefinition(ObjectLiteralChecker* checker,
-                                                 bool in_class, bool is_static,
-                                                 bool* is_computed_name,
-                                                 bool* has_seen_constructor,
-                                                 bool* ok);
+  ObjectLiteralPropertyT ParsePropertyDefinition(
+      ObjectLiteralCheckerBase* checker, bool in_class, bool is_static,
+      bool* is_computed_name, 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);
   ExpressionT ParseMemberExpression(bool* ok);
   ExpressionT ParseMemberExpressionContinuation(ExpressionT expression,
                                                 bool* ok);
   ExpressionT ParseArrowFunctionLiteral(int start_pos, ExpressionT params_ast,
                                         bool* ok);
   ExpressionT ParseTemplateLiteral(ExpressionT tag, int start, bool* ok);
   void AddTemplateExpression(ExpressionT);
 
   // Checks if the expression is a valid reference expression (e.g., on the
   // left-hand side of assignments). Although ruled out by ECMA as early errors,
   // we allow calls for web compatibility and rewrite them to a runtime throw.
   ExpressionT CheckAndRewriteReferenceExpression(
       ExpressionT expression,
       Scanner::Location location, const char* message, bool* ok);
 
-  // Used to detect duplicates in object literals. Each of the values
-  // kGetterProperty, kSetterProperty and kValueProperty represents
-  // a type of object literal property. When parsing a property, its
-  // type value is stored in the DuplicateFinder for the property name.
-  // Values are chosen so that having intersection bits means the there is
-  // an incompatibility.
-  // I.e., you can add a getter to a property that already has a setter, since
-  // kGetterProperty and kSetterProperty doesn't intersect, but not if it
-  // already has a getter or a value. Adding the getter to an existing
-  // setter will store the value (kGetterProperty | kSetterProperty), which
-  // is incompatible with adding any further properties.
+  // Used to validate property names in object literals and class literals
   enum PropertyKind {
-    kNone = 0,
-    // Bit patterns representing different object literal property types.
-    kGetterProperty = 1,
-    kSetterProperty = 2,
-    kValueProperty = 7,
-    // Helper constants.
-    kValueFlag = 4
+    kAccessorProperty = 1,
+    kValueProperty = 2,
+    kMethodProperty = 3

[adamk, 2015/01/27 20:05:48]

Given that these are no longer bit patterns, you should be able to leave off the
values and let the compiler assign them.

[arv (Not doing code reviews), 2015/01/28 02:37:15]

Done.

   };
 
-  // Validation per ECMA 262 - 11.1.5 "Object Initializer".
-  class ObjectLiteralChecker {
+  class ObjectLiteralCheckerBase {
    public:
-    ObjectLiteralChecker(ParserBase* parser, StrictMode strict_mode)
-        : parser_(parser),
-          finder_(scanner()->unicode_cache()),
-          strict_mode_(strict_mode) {}
+    explicit ObjectLiteralCheckerBase(ParserBase* parser) : parser_(parser) {}
 
-    void CheckProperty(Token::Value property, PropertyKind type, bool* ok);
+    virtual void CheckProperty(Token::Value property, PropertyKind type,
+                               bool is_static, bool is_generator, bool* ok) = 0;
 
-   private:
+   protected:
     ParserBase* parser() const { return parser_; }
     Scanner* scanner() const { return parser_->scanner(); }
 
-    // Checks the type of conflict based on values coming from PropertyType.
-    bool HasConflict(PropertyKind type1, PropertyKind type2) {
-      return (type1 & type2) != 0;
+   private:
+    ParserBase* parser_;
+  };
+
+  // Validation per ES6 object literals.
+  class ObjectLiteralChecker : public ObjectLiteralCheckerBase {
+   public:
+    explicit ObjectLiteralChecker(ParserBase* parser)
+        : ObjectLiteralCheckerBase(parser), has_seen_proto_(false) {}
+
+    void CheckProperty(Token::Value property, PropertyKind type, bool is_static,
+                       bool is_generator, bool* ok) OVERRIDE;
+
+   private:
+    bool IsProto() { return this->scanner()->LiteralMatches("__proto__", 9); }
+
+    bool has_seen_proto_;
+  };
+
+  // Validation per ES6 class literals.
+  class ClassLiteralChecker : public ObjectLiteralCheckerBase {
+   public:
+    explicit ClassLiteralChecker(ParserBase* parser)
+        : ObjectLiteralCheckerBase(parser), has_seen_constructor_(false) {}
+
+    void CheckProperty(Token::Value property, PropertyKind type, bool is_static,
+                       bool is_generator, bool* ok) OVERRIDE;
+
+   private:
+    bool IsConstructor() {
+      return this->scanner()->LiteralMatches("constructor", 11);
     }
-    bool IsDataDataConflict(PropertyKind type1, PropertyKind type2) {
-      return ((type1 & type2) & kValueFlag) != 0;
-    }
-    bool IsDataAccessorConflict(PropertyKind type1, PropertyKind type2) {
-      return ((type1 ^ type2) & kValueFlag) != 0;
-    }
-    bool IsAccessorAccessorConflict(PropertyKind type1, PropertyKind type2) {
-      return ((type1 | type2) & kValueFlag) == 0;
+    bool IsPrototype() {
+      return this->scanner()->LiteralMatches("prototype", 9);
     }
 
-    ParserBase* parser_;
-    DuplicateFinder finder_;
-    StrictMode strict_mode_;
+    bool has_seen_constructor_;
   };
 
+
   // If true, the next (and immediately following) function literal is
   // preceded by a parenthesis.
   // Heuristically that means that the function will be called immediately,
   // so never lazily compile it.
   bool parenthesized_function_;
 
   typename Traits::Type::Scope* scope_;  // Scope stack.
   FunctionState* function_state_;  // Function state stack.
   v8::Extension* extension_;
   FuncNameInferrer* fni_;
@@ skipping 1455 matching lines @@
 
   uint32_t index;
   return this->IsArrayIndex(*name, &index)
              ? factory()->NewNumberLiteral(index, pos)
              : factory()->NewStringLiteral(*name, pos);
 }
 
 
 template <class Traits>
 typename ParserBase<Traits>::ObjectLiteralPropertyT
-ParserBase<Traits>::ParsePropertyDefinition(ObjectLiteralChecker* checker,
+ParserBase<Traits>::ParsePropertyDefinition(ObjectLiteralCheckerBase* checker,
                                             bool in_class, bool is_static,
                                             bool* is_computed_name,
                                             bool* has_seen_constructor,
                                             bool* ok) {
-  DCHECK(!in_class || is_static || has_seen_constructor != NULL);
+  DCHECK(!in_class || is_static || has_seen_constructor != nullptr);
   ExpressionT value = this->EmptyExpression();
   IdentifierT name = this->EmptyIdentifier();
   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();
   ExpressionT name_expression = ParsePropertyName(
       &name, &is_get, &is_set, &name_is_static, is_computed_name,
       CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
 
-  if (fni_ != NULL && !*is_computed_name) {
+  if (fni_ != nullptr && !*is_computed_name) {
     this->PushLiteralName(fni_, name);
   }
 
   if (!in_class && !is_generator && peek() == Token::COLON) {
     // PropertyDefinition : PropertyName ':' AssignmentExpression
-    if (!*is_computed_name && checker != NULL) {
-      checker->CheckProperty(name_token, kValueProperty,
+    if (!*is_computed_name) {
+      checker->CheckProperty(name_token, kValueProperty, is_static,
+                             is_generator,
                              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();
+    if (!*is_computed_name) {
+      checker->CheckProperty(name_token, kMethodProperty, is_static,
+                             is_generator,
+                             CHECK_OK_CUSTOM(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) {
-        ReportMessageAt(scanner()->location(), "duplicate_constructor");
-        *ok = false;
-        return this->EmptyObjectLiteralProperty();
-      }
-
       *has_seen_constructor = true;
       kind = FunctionKind::kNormalFunction;
     }
 
-    if (!*is_computed_name && 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));
 
     return factory()->NewObjectLiteralProperty(name_expression, value,
                                                ObjectLiteralProperty::COMPUTED,
                                                is_static, *is_computed_name);
 
   } else if (in_class && name_is_static && !is_static) {
     // static MethodDefinition
-    return ParsePropertyDefinition(checker, true, true, is_computed_name, NULL,
-                                   ok);
+    return ParsePropertyDefinition(checker, true, true, is_computed_name,
+                                   nullptr, ok);
 
   } else if (is_get || is_set) {
     // Accessor
     name = this->EmptyIdentifier();
     bool dont_care = false;
     name_token = peek();
 
     name_expression = ParsePropertyName(
         &name, &dont_care, &dont_care, &dont_care, is_computed_name,
         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)) {
-      ReportMessageAt(scanner()->location(), "constructor_special_method");
-      *ok = false;
-      return this->EmptyObjectLiteralProperty();
-    }
-    if (!*is_computed_name && checker != NULL) {
-      checker->CheckProperty(name_token,
-                             is_get ? kGetterProperty : kSetterProperty,
+    if (!*is_computed_name) {
+      checker->CheckProperty(name_token, kAccessorProperty, is_static,
+                             is_generator,
                              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));
@@ skipping 37 matching lines @@
     bool* ok) {
   // ObjectLiteral ::
   // '{' (PropertyDefinition (',' PropertyDefinition)* ','? )? '}'
 
   int pos = peek_position();
   typename Traits::Type::PropertyList properties =
       this->NewPropertyList(4, zone_);
   int number_of_boilerplate_properties = 0;
   bool has_function = false;
   bool has_computed_names = false;
-
-  ObjectLiteralChecker checker(this, strict_mode());
+  ObjectLiteralChecker checker(this);
 
   Expect(Token::LBRACE, CHECK_OK);
 
   while (peek() != Token::RBRACE) {
-    if (fni_ != NULL) fni_->Enter();
+    if (fni_ != nullptr) fni_->Enter();
 
     const bool in_class = false;
     const bool is_static = false;
     bool is_computed_name = false;
     ObjectLiteralPropertyT property = this->ParsePropertyDefinition(
-        &checker, in_class, is_static, &is_computed_name, NULL, CHECK_OK);
+        &checker, in_class, is_static, &is_computed_name, nullptr, CHECK_OK);
 
     if (is_computed_name) {
       has_computed_names = true;
     }
 
     // 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 (!has_computed_names && this->IsBoilerplateProperty(property)) {
       number_of_boilerplate_properties++;
     }
     properties->Add(property, zone());
 
     if (peek() != Token::RBRACE) {
       // Need {} because of the CHECK_OK macro.
       Expect(Token::COMMA, CHECK_OK);
     }
 
-    if (fni_ != NULL) {
+    if (fni_ != nullptr) {
       fni_->Infer();
       fni_->Leave();
     }
   }
   Expect(Token::RBRACE, CHECK_OK);
 
   // Computation of literal_index must happen before pre parse bailout.
   int literal_index = function_state_->NextMaterializedLiteralIndex();
 
   return factory()->NewObjectLiteral(properties,
@@ skipping 751 matching lines @@
   }
 }
 
 
 #undef CHECK_OK
 #undef CHECK_OK_CUSTOM
 
 
 template <typename Traits>
 void ParserBase<Traits>::ObjectLiteralChecker::CheckProperty(
-    Token::Value property, PropertyKind type, bool* ok) {
-  int old;
-  if (property == Token::NUMBER) {
-    old = scanner()->FindNumber(&finder_, type);
-  } else {
-    old = scanner()->FindSymbol(&finder_, type);
+    Token::Value property, PropertyKind type, bool is_static, bool is_generator,
+    bool* ok) {
+  DCHECK(!is_static);
+  DCHECK(!is_generator || type == kMethodProperty);
+
+  if (property == Token::NUMBER) return;
+
+  if (type == kValueProperty && IsProto()) {
+    if (has_seen_proto_) {
+      this->parser()->ReportMessage("duplicate_proto");
+      *ok = false;
+      return;
+    }
+    has_seen_proto_ = true;
+    return;
   }
-  PropertyKind old_type = static_cast<PropertyKind>(old);
-  if (HasConflict(old_type, type)) {
-    if (IsDataDataConflict(old_type, type)) {
-      // Both are data properties.
-      if (strict_mode_ == SLOPPY) return;
-      parser()->ReportMessage("strict_duplicate_property");

[adamk, 2015/01/27 20:05:48]

If this message is now unused, you ought to be able to remove it (same goes
below).

[arv (Not doing code reviews), 2015/01/28 02:37:15]

Done.

-    } else if (IsDataAccessorConflict(old_type, type)) {
-      // Both a data and an accessor property with the same name.
-      parser()->ReportMessage("accessor_data_property");
-    } else {
-      DCHECK(IsAccessorAccessorConflict(old_type, type));
-      // Both accessors of the same type.
-      parser()->ReportMessage("accessor_get_set");
+}
+
+
+template <typename Traits>
+void ParserBase<Traits>::ClassLiteralChecker::CheckProperty(
+    Token::Value property, PropertyKind type, bool is_static, bool is_generator,
+    bool* ok) {
+  DCHECK(type == kMethodProperty);
+
+  if (property == Token::NUMBER) return;
+
+  if (is_static) {
+    if (IsPrototype()) {
+      this->parser()->ReportMessage("static_prototype");
+      *ok = false;
+      return;
     }
-    *ok = false;
+  } else if (IsConstructor()) {
+    if (is_generator || type == kAccessorProperty) {
+      this->parser()->ReportMessage("constructor_special_method");
+      *ok = false;
+      return;
+    }
+    if (has_seen_constructor_) {
+      this->parser()->ReportMessage("duplicate_constructor");
+      *ok = false;
+      return;
+    }
+    has_seen_constructor_ = true;
+    return;
   }
 }
 } }  // v8::internal
 
 #endif  // V8_PREPARSER_H