ES6: Add support for method shorthand in object literals

src/preparser.h

Index: src/preparser.h
diff --git a/src/preparser.h b/src/preparser.h
index b5bcf529a8caa0bbce36ebf9d2dceaa0ce2fdf0d..30b6e4f534639c415183c8549753887e9e79bafe 100644
--- a/src/preparser.h
+++ b/src/preparser.h
@@ -85,6 +85,7 @@ class ParserBase : public Traits {
         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) {}
 
@@ -100,6 +101,9 @@ class ParserBase : public Traits {
     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.
@@ -114,8 +118,9 @@ class ParserBase : public Traits {
   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:
@@ -481,6 +486,8 @@ class ParserBase : public Traits {
   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);
@@ -579,6 +586,7 @@ class ParserBase : public Traits {
   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_;
@@ -1054,8 +1062,8 @@ class PreParserFactory {
       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();
   }
 
@@ -1331,14 +1339,10 @@ class PreParserTraits {
   // 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_;
@@ -1469,14 +1473,10 @@ class PreParser : public ParserBase<PreParserTraits> {
                              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);
@@ -1852,84 +1852,95 @@ typename ParserBase<Traits>::ExpressionT ParserBase<Traits>::ParseArrayLiteral(
 
 
 template <class Traits>
-typename ParserBase<Traits>::ObjectLiteralPropertyT ParserBase<
-    Traits>::ParsePropertyDefinition(ObjectLiteralChecker* checker, bool* ok) {
-  LiteralT key = this->EmptyLiteral();
+typename ParserBase<Traits>::IdentifierT ParserBase<Traits>::ParsePropertyName(
+    bool* is_getter, bool* is_setter, bool* ok) {
   Token::Value next = peek();
-  int next_pos = peek_position();
-
   switch (next) {
-    case Token::STRING: {
+    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: {
+      return this->GetSymbol(scanner_);
+    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);
-
-      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);
-    }
+      return this->GetNumberAsSymbol(scanner_);
+    default:
+      return ParseIdentifierNameOrGetOrSet(is_getter, is_setter,
+                                           CHECK_OK_CUSTOM(EmptyIdentifier));
   }
+}
+
 
-  // Validate the property
-  checker->CheckProperty(next, kValueProperty,
-                         CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
+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));
+}
 
-  Expect(Token::COLON, CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
-  ExpressionT value = this->ParseAssignmentExpression(
-      true, CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
+
+template <class Traits>
+typename ParserBase<Traits>::ObjectLiteralPropertyT ParserBase<
+    Traits>::ParsePropertyDefinition(ObjectLiteralChecker* checker, bool* ok) {
+  // 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);
+
+  // 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));
+
+    // 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();
+  }
+
+  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);
 }
@@ -2428,14 +2439,12 @@ ParserBase<Traits>::ParseMemberExpression(bool* 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);
@@ -2596,7 +2605,7 @@ typename ParserBase<Traits>::ExpressionT ParserBase<
       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);