ES6: Add support for method shorthand in object literals

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/func-name-inferrer.h"
@@ skipping 67 matching lines @@
         fni_(NULL),
         log_(log),
         mode_(PARSE_EAGERLY),  // Lazy mode must be set explicitly.
         scanner_(scanner),
         stack_limit_(stack_limit),
         stack_overflow_(false),
         allow_lazy_(false),
         allow_natives_syntax_(false),
         allow_generators_(false),
         allow_arrow_functions_(false),
+        allow_harmony_object_literals_(false),
         zone_(zone),
         ast_node_id_gen_(ast_node_id_gen) {}
 
   // Getters that indicate whether certain syntactical constructs are
   // allowed to be parsed by this instance of the parser.
   bool allow_lazy() const { return allow_lazy_; }
   bool allow_natives_syntax() const { return allow_natives_syntax_; }
   bool allow_generators() const { return allow_generators_; }
   bool allow_arrow_functions() const { return allow_arrow_functions_; }
   bool allow_modules() const { return scanner()->HarmonyModules(); }
   bool allow_harmony_scoping() const { return scanner()->HarmonyScoping(); }
   bool allow_harmony_numeric_literals() const {
     return scanner()->HarmonyNumericLiterals();
   }
   bool allow_classes() const { return scanner()->HarmonyClasses(); }
+  bool allow_harmony_object_literals() const {
+    return allow_harmony_object_literals_;
+  }
 
   // Setters that determine whether certain syntactical constructs are
   // allowed to be parsed by this instance of the parser.
   void set_allow_lazy(bool allow) { allow_lazy_ = allow; }
   void set_allow_natives_syntax(bool allow) { allow_natives_syntax_ = allow; }
   void set_allow_generators(bool allow) { allow_generators_ = allow; }
   void set_allow_arrow_functions(bool allow) { allow_arrow_functions_ = allow; }
   void set_allow_modules(bool allow) { scanner()->SetHarmonyModules(allow); }
   void set_allow_harmony_scoping(bool allow) {
     scanner()->SetHarmonyScoping(allow);
   }
   void set_allow_harmony_numeric_literals(bool allow) {
     scanner()->SetHarmonyNumericLiterals(allow);
   }
-  void set_allow_classes(bool allow) {
-    scanner()->SetHarmonyClasses(allow);
+  void set_allow_classes(bool allow) { scanner()->SetHarmonyClasses(allow); }
+  void set_allow_harmony_object_literals(bool allow) {
+    allow_harmony_object_literals_ = allow;
   }
 
  protected:
   friend class Traits::Checkpoint;
 
   enum AllowEvalOrArgumentsAsIdentifier {
     kAllowEvalOrArguments,
     kDontAllowEvalOrArguments
   };
 
@@ skipping 345 matching lines @@
                                             bool* ok);
 
   ExpressionT ParseRegExpLiteral(bool seen_equal, bool* ok);
 
   ExpressionT ParsePrimaryExpression(bool* ok);
   ExpressionT ParseExpression(bool accept_IN, bool* ok);
   ExpressionT ParseArrayLiteral(bool* ok);
   ExpressionT ParseObjectLiteral(bool* ok);
   ObjectLiteralPropertyT ParsePropertyDefinition(ObjectLiteralChecker* checker,
                                                  bool* ok);
+  IdentifierT ParsePropertyName(bool* is_getter, bool* is_setter, bool* ok);
+  IdentifierT ParsePropertyName(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);
@@ skipping 78 matching lines @@
 
  private:
   Scanner* scanner_;
   uintptr_t stack_limit_;
   bool stack_overflow_;
 
   bool allow_lazy_;
   bool allow_natives_syntax_;
   bool allow_generators_;
   bool allow_arrow_functions_;
+  bool allow_harmony_object_literals_;
 
   typename Traits::Type::Zone* zone_;  // Only used by Parser.
   AstNode::IdGen* ast_node_id_gen_;
 };
 
 
 class PreParserIdentifier {
  public:
   PreParserIdentifier() : type_(kUnknownIdentifier) {}
   static PreParserIdentifier Default() {
@@ skipping 455 matching lines @@
     return PreParserStatement::Default();
   }
   PreParserExpression NewFunctionLiteral(
       PreParserIdentifier name, AstValueFactory* ast_value_factory,
       const PreParserScope& scope, PreParserStatementList body,
       int materialized_literal_count, int expected_property_count,
       int handler_count, int parameter_count,
       FunctionLiteral::ParameterFlag has_duplicate_parameters,
       FunctionLiteral::FunctionType function_type,
       FunctionLiteral::IsFunctionFlag is_function,
-      FunctionLiteral::IsParenthesizedFlag is_parenthesized,
-      FunctionLiteral::KindFlag kind, int position) {
+      FunctionLiteral::IsParenthesizedFlag is_parenthesized, FunctionKind kind,
+      int position) {
     return PreParserExpression::Default();
   }
 
   // Return the object itself as AstVisitor and implement the needed
   // dummy method right in this class.
   PreParserFactory* visitor() { return this; }
   BailoutReason dont_optimize_reason() { return kNoReason; }
   int* ast_properties() {
     static int dummy = 42;
     return &dummy;
@@ skipping 255 matching lines @@
     return 0;
   }
 
   static AstValueFactory* ast_value_factory() { return NULL; }
 
   void CheckConflictingVarDeclarations(PreParserScope scope, bool* ok) {}
 
   // Temporary glue; these functions will move to ParserBase.
   PreParserExpression ParseV8Intrinsic(bool* ok);
   PreParserExpression ParseFunctionLiteral(
-      PreParserIdentifier name,
-      Scanner::Location function_name_location,
-      bool name_is_strict_reserved,
-      bool is_generator,
-      int function_token_position,
-      FunctionLiteral::FunctionType type,
-      FunctionLiteral::ArityRestriction arity_restriction,
-      bool* ok);
+      PreParserIdentifier name, Scanner::Location function_name_location,
+      bool name_is_strict_reserved, FunctionKind kind,
+      int function_token_position, FunctionLiteral::FunctionType type,
+      FunctionLiteral::ArityRestriction arity_restriction, bool* ok);
 
  private:
   PreParser* pre_parser_;
 };
 
 
 // Preparsing checks a JavaScript program and emits preparse-data that helps
 // a later parsing to be faster.
 // See preparse-data-format.h for the data format.
 
@@ skipping 110 matching lines @@
 
   V8_INLINE void SkipLazyFunctionBody(PreParserIdentifier function_name,
                                       int* materialized_literal_count,
                                       int* expected_property_count, bool* ok);
   V8_INLINE PreParserStatementList
       ParseEagerFunctionBody(PreParserIdentifier function_name, int pos,
                              Variable* fvar, Token::Value fvar_init_op,
                              bool is_generator, bool* ok);
 
   Expression ParseFunctionLiteral(
-      Identifier name,
-      Scanner::Location function_name_location,
-      bool name_is_strict_reserved,
-      bool is_generator,
-      int function_token_pos,
+      Identifier name, Scanner::Location function_name_location,
+      bool name_is_strict_reserved, FunctionKind kind, int function_token_pos,
       FunctionLiteral::FunctionType function_type,
-      FunctionLiteral::ArityRestriction arity_restriction,
-      bool* ok);
+      FunctionLiteral::ArityRestriction arity_restriction, bool* ok);
   void ParseLazyFunctionLiteralBody(bool* ok);
 
   bool CheckInOrOf(bool accept_OF);
 };
 
 
 PreParserStatementList PreParser::ParseEagerFunctionBody(
     PreParserIdentifier function_name, int pos, Variable* fvar,
     Token::Value fvar_init_op, bool is_generator, bool* ok) {
   ParsingModeScope parsing_mode(this, PARSE_EAGERLY);
@@ skipping 355 matching lines @@
   Expect(Token::RBRACK, CHECK_OK);
 
   // Update the scope information before the pre-parsing bailout.
   int literal_index = function_state_->NextMaterializedLiteralIndex();
 
   return factory()->NewArrayLiteral(values, literal_index, pos);
 }
 
 
 template <class Traits>
+typename ParserBase<Traits>::IdentifierT ParserBase<Traits>::ParsePropertyName(
+    bool* is_getter, bool* is_setter, bool* ok) {
+  Token::Value next = peek();
+  switch (next) {
+    case Token::STRING:
+      Consume(Token::STRING);
+      return this->GetSymbol(scanner_);
+    case Token::NUMBER:
+      Consume(Token::NUMBER);
+      return this->GetNumberAsSymbol(scanner_);
+    default:
+      return ParseIdentifierNameOrGetOrSet(is_getter, is_setter,
+                                           CHECK_OK_CUSTOM(EmptyIdentifier));
+  }
+}
+
+
+template <class Traits>
+typename ParserBase<Traits>::IdentifierT ParserBase<Traits>::ParsePropertyName(

[rossberg, 2014/08/25 09:39:30]

Nit: Given that it's called only in one place, I don't think this wrapper is
necessary.

[arv (Not doing code reviews), 2014/09/02 14:42:48]

This was done done for readability. The out params are don't care and since it
was being called from a context where we have is_getter and is_setter it got
confusing.

[arv (Not doing code reviews), 2014/09/09 22:24:27]

Removed.

+    bool* ok) {
+  bool is_getter = false;
+  bool is_setter = false;
+  return ParsePropertyName(&is_getter, &is_setter,
+                           CHECK_OK_CUSTOM(EmptyIdentifier));
+}
+
+
+template <class Traits>
 typename ParserBase<Traits>::ObjectLiteralPropertyT ParserBase<
     Traits>::ParsePropertyDefinition(ObjectLiteralChecker* checker, bool* ok) {
-  LiteralT key = this->EmptyLiteral();
+  // TODO(arv): Add support for concise generator methods.
+  ExpressionT value = this->EmptyExpression();
+  bool is_getter = false;
+  bool is_setter = false;
   Token::Value next = peek();
   int next_pos = peek_position();
+  IdentifierT name = ParsePropertyName(
+      &is_getter, &is_setter, CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
+  if (fni_ != NULL) this->PushLiteralName(fni_, name);
 
-  switch (next) {
-    case Token::STRING: {
-      Consume(Token::STRING);
-      IdentifierT string = this->GetSymbol(scanner_);
-      if (fni_ != NULL) this->PushLiteralName(fni_, string);
-      uint32_t index;
-      if (this->IsArrayIndex(string, &index)) {
-        key = factory()->NewNumberLiteral(index, next_pos);
-        break;
-      }
-      key = factory()->NewStringLiteral(string, next_pos);
-      break;
-    }
-    case Token::NUMBER: {
-      Consume(Token::NUMBER);
-      key = this->ExpressionFromLiteral(Token::NUMBER, next_pos, scanner_,
-                                        factory());
-      break;
-    }
-    default: {
-      bool is_getter = false;
-      bool is_setter = false;
-      IdentifierT id = ParseIdentifierNameOrGetOrSet(
-          &is_getter, &is_setter, CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
-      if (fni_ != NULL) this->PushLiteralName(fni_, id);
+  // PropertyDefinition : PropertyName ':' AssignmentExpression
+  if (peek() == Token::COLON) {
+    checker->CheckProperty(next, kValueProperty,

[rossberg, 2014/08/25 09:39:30]

Can this check be factored out and moved after the if-chain, to avoid its
duplication?

[arv (Not doing code reviews), 2014/09/02 14:42:48]

I had that at first but the call to CheckProperty depends on the current state
of the parser/scanner. I can try to refactor further.

[arv (Not doing code reviews), 2014/09/09 22:24:27]

Decided not to change this.

1. The duplicate check will go away in ES6.
2. We want to check the name before we parse the expression/function body
3. The ObjectLiteralChecker depends on the current position of the scanner.

+                           CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
+    Consume(Token::COLON);
+    value = this->ParseAssignmentExpression(
+        true, CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
 
-      if ((is_getter || is_setter) && peek() != Token::COLON) {
-        // Special handling of getter and setter syntax:
-        // { ... , get foo() { ... }, ... , set foo(v) { ... v ... } , ... }
-        // We have already read the "get" or "set" keyword.
-        IdentifierT name = this->EmptyIdentifier();
-        switch (peek()) {
-          case Token::STRING:
-            Consume(Token::STRING);
-            name = this->GetSymbol(scanner_);
-            break;
-          case Token::NUMBER:
-            Consume(Token::NUMBER);
-            name = this->GetNumberAsSymbol(scanner_);
-            break;
-          default:
-            name = ParseIdentifierName(
-                CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
-        }
-        // Validate the property.
-        PropertyKind type = is_getter ? kGetterProperty : kSetterProperty;
-        checker->CheckProperty(next, type,
-                               CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
-        typename Traits::Type::FunctionLiteral value =
-            this->ParseFunctionLiteral(
-                name, scanner()->location(),
-                false,  // reserved words are allowed here
-                false,  // not a generator
-                RelocInfo::kNoPosition, FunctionLiteral::ANONYMOUS_EXPRESSION,
-                is_getter ? FunctionLiteral::GETTER_ARITY
-                          : FunctionLiteral::SETTER_ARITY,
-                CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
-        return factory()->NewObjectLiteralProperty(is_getter, value, next_pos);
-      }
-      // Failed to parse as get/set property, so it's just a normal property
-      // (which might be called "get" or "set" or something else).
-      key = factory()->NewStringLiteral(id, next_pos);
-    }
+    // Concise Method

[rossberg, 2014/08/25 09:39:30]

Nit: V8 typically uses a style where a comment like this is placed inside the
respective branch.

[arv (Not doing code reviews), 2014/09/09 22:24:27]

Done.

+  } else if (allow_harmony_object_literals_ && peek() == Token::LPAREN) {
+    checker->CheckProperty(next, kValueProperty,
+                           CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
+    value = this->ParseFunctionLiteral(
+        name, scanner()->location(),
+        false,  // reserved words are allowed here
+        FunctionKind::kConciseMethod, RelocInfo::kNoPosition,
+        FunctionLiteral::ANONYMOUS_EXPRESSION, FunctionLiteral::NORMAL_ARITY,
+        CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
+
+    // Accessor
+  } else if (is_getter || is_setter) {
+    name = ParsePropertyName(CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
+    // Validate the property.
+    PropertyKind type = is_getter ? kGetterProperty : kSetterProperty;
+    checker->CheckProperty(next, type,
+                           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_getter ? FunctionLiteral::GETTER_ARITY
+                  : FunctionLiteral::SETTER_ARITY,
+        CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
+    return factory()->NewObjectLiteralProperty(is_getter, value, next_pos);
+  } else {
+    Token::Value next = Next();
+    ReportUnexpectedToken(next);
+    *ok = false;
+    return this->EmptyObjectLiteralProperty();
   }
 
-  // Validate the property
-  checker->CheckProperty(next, kValueProperty,
-                         CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
-
-  Expect(Token::COLON, CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
-  ExpressionT value = this->ParseAssignmentExpression(
-      true, CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
+  LiteralT key = this->EmptyLiteral();

[rossberg, 2014/08/25 09:39:30]

Nit: use ?: instead of `if` to avoid the redundant init.

[arv (Not doing code reviews), 2014/09/09 22:24:27]

Done.

+  uint32_t index;
+  if (this->IsArrayIndex(name, &index)) {
+    key = factory()->NewNumberLiteral(index, next_pos);
+  } else {
+    key = factory()->NewStringLiteral(name, next_pos);
+  }
 
   return factory()->NewObjectLiteralProperty(key, value);
 }
 
 
 template <class Traits>
 typename ParserBase<Traits>::ExpressionT ParserBase<Traits>::ParseObjectLiteral(
     bool* ok) {
   // ObjectLiteral ::
   // '{' (PropertyDefinition (',' PropertyDefinition)* ','? )? '}'
@@ skipping 478 matching lines @@
     bool is_strict_reserved_name = false;
     Scanner::Location function_name_location = Scanner::Location::invalid();
     FunctionLiteral::FunctionType function_type =
         FunctionLiteral::ANONYMOUS_EXPRESSION;
     if (peek_any_identifier()) {
       name = ParseIdentifierOrStrictReservedWord(&is_strict_reserved_name,
                                                  CHECK_OK);
       function_name_location = scanner()->location();
       function_type = FunctionLiteral::NAMED_EXPRESSION;
     }
-    result = this->ParseFunctionLiteral(name,
-                                        function_name_location,
-                                        is_strict_reserved_name,
-                                        is_generator,
-                                        function_token_position,
-                                        function_type,
-                                        FunctionLiteral::NORMAL_ARITY,
-                                        CHECK_OK);
+    result = this->ParseFunctionLiteral(
+        name, function_name_location, is_strict_reserved_name,
+        is_generator ? FunctionKind::kGeneratorFunction
+                     : FunctionKind::kNormalFunction,
+        function_token_position, function_type, FunctionLiteral::NORMAL_ARITY,
+        CHECK_OK);
   } else if (peek() == Token::SUPER) {
     int beg_pos = position();
     Consume(Token::SUPER);
     Token::Value next = peek();
     if (next == Token::PERIOD || next == Token::LBRACK ||
         next == Token::LPAREN) {
       result = this->SuperReference(scope_, factory());
     } else {
       ReportMessageAt(Scanner::Location(beg_pos, position()),
                       "unexpected_super");
@@ skipping 140 matching lines @@
 
     ast_properties = *factory()->visitor()->ast_properties();
     dont_optimize_reason = factory()->visitor()->dont_optimize_reason();
   }
 
   FunctionLiteralT function_literal = factory()->NewFunctionLiteral(
       this->EmptyIdentifierString(), this->ast_value_factory(), scope, body,
       materialized_literal_count, expected_property_count, handler_count,
       num_parameters, FunctionLiteral::kNoDuplicateParameters,
       FunctionLiteral::ANONYMOUS_EXPRESSION, FunctionLiteral::kIsFunction,
-      FunctionLiteral::kNotParenthesized, FunctionLiteral::kArrowFunction,
+      FunctionLiteral::kNotParenthesized, FunctionKind::kArrowFunction,
       start_pos);
 
   function_literal->set_function_token_position(start_pos);
   function_literal->set_ast_properties(&ast_properties);
   function_literal->set_dont_optimize_reason(dont_optimize_reason);
 
   if (fni_ != NULL) this->InferFunctionName(fni_, function_literal);
 
   return function_literal;
 }
@@ skipping 51 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