[cleanup] refactor ParsePropertyDefinition for clarity

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/bailout-reason.h"
 #include "src/expression-classifier.h"
 #include "src/func-name-inferrer.h"
@@ skipping 2593 matching lines @@
   int next_beg_pos = scanner()->peek_location().beg_pos;
   int next_end_pos = scanner()->peek_location().end_pos;
   ExpressionT name_expression = ParsePropertyName(
       &name, &is_get, &is_set, &name_is_static, is_computed_name, classifier,
       CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
 
   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->CheckProperty(name_token, kValueProperty, is_static,
-                             is_generator,
-                             CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
+  if (!in_class && !is_generator) {
+    DCHECK(!is_static);
+
+    if (peek() == Token::COLON) {
+      // PropertyDefinition
+      //    PropertyName ':' AssignmentExpression
+      if (!*is_computed_name) {
+        checker->CheckProperty(name_token, kValueProperty, false, false,
+                               CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
+      }
+      Consume(Token::COLON);
+      value = this->ParseAssignmentExpression(
+          true, classifier, CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
+      return factory()->NewObjectLiteralProperty(name_expression, value, false,
+                                                 *is_computed_name);
     }
-    Consume(Token::COLON);
-    value = this->ParseAssignmentExpression(
-        true, classifier, CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
 
-  } else if (is_generator || peek() == Token::LPAREN) {
-    // Concise Method
+    if (Token::IsIdentifier(name_token, language_mode(),
+                            this->is_generator()) &&
+        (peek() == Token::COMMA || peek() == Token::RBRACE ||
+         peek() == Token::ASSIGN)) {
+      // PropertyDefinition
+      //    IdentifierReference
+      //    CoverInitializedName
+      //
+      // CoverInitializedName
+      //    IdentifierReference Initializer(opt)
+      if (classifier->duplicate_finder() != nullptr &&
+          scanner()->FindSymbol(classifier->duplicate_finder(), 1) != 0) {
+        classifier->RecordDuplicateFormalParameterError(scanner()->location());
+      }
+
+      ExpressionT lhs = this->ExpressionFromIdentifier(
+          name, next_beg_pos, next_end_pos, scope_, factory());
+
+      if (peek() == Token::ASSIGN) {
+        this->ExpressionUnexpectedToken(classifier);
+        Consume(Token::ASSIGN);
+        ExpressionClassifier rhs_classifier;
+        ExpressionT rhs = this->ParseAssignmentExpression(
+            true, &rhs_classifier, CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
+        classifier->Accumulate(rhs_classifier,
+                               ExpressionClassifier::ExpressionProductions);
+        value = factory()->NewAssignment(Token::ASSIGN, lhs, rhs,
+                                         RelocInfo::kNoPosition);
+      } else {
+        value = lhs;
+      }
+
+      return factory()->NewObjectLiteralProperty(
+          name_expression, value, ObjectLiteralProperty::COMPUTED, false,
+          false);
+    }
+  }
+
+
+  if (is_generator || peek() == Token::LPAREN) {
+    // MethodDefinition
+    //    PropertyName '(' StrictFormalParameters ')' '{' FunctionBody '}'
+    //    '*' PropertyName '(' StrictFormalParameters ')' '{' FunctionBody '}'
     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)) {
       *has_seen_constructor = true;
       kind = has_extends ? FunctionKind::kSubclassConstructor
                          : FunctionKind::kBaseConstructor;
     }
 
     if (!in_class) kind = WithObjectLiteralBit(kind);
 
     value = this->ParseFunctionLiteral(
         name, scanner()->location(), kSkipFunctionNameCheck, kind,
         RelocInfo::kNoPosition, FunctionLiteral::ANONYMOUS_EXPRESSION,
         FunctionLiteral::NORMAL_ARITY, language_mode(),
         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
+  if (in_class && name_is_static && !is_static) {
+    // ClassElement (static)
+    //    static MethodDefinition

[adamk, 2015/09/16 20:58:00]

'static' should be quoted.

[caitp (gmail), 2015/09/16 21:01:09]

Done.

     return ParsePropertyDefinition(checker, true, has_extends, true,
                                    is_computed_name, nullptr, classifier, ok);
-  } else if ((is_get || is_set) &&
-             (in_class || (peek() != Token::RBRACE && peek() != Token::COMMA &&
-                           peek() != Token::ASSIGN))) {
-    // Accessor
+  }
+
+  if (is_get || is_set) {
+    // MethodDefinition (Accessors)
+    //    get PropertyName '(' ')' '{' FunctionBody '}'
+    //    set PropertyName '(' PropertySetParameterList ')' '{' FunctionBody '}'

[caitp (gmail), 2015/09/16 20:10:25]

The get/set should probably be quoted, but there isn't a lot of room left on the
line to add another 2 characters :(

[adamk, 2015/09/16 20:58:00]

Yuck, 80 char limit is unfortunate here. Probably more readable all on one line
than quoted and split into two, so fine to leave it.

[caitp (gmail), 2015/09/16 21:01:09]

Acknowledged.

     name = this->EmptyIdentifier();
     bool dont_care = false;
     name_token = peek();
 
     name_expression = ParsePropertyName(
         &name, &dont_care, &dont_care, &dont_care, is_computed_name, classifier,
         CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
 
     if (!*is_computed_name) {
       checker->CheckProperty(name_token, kAccessorProperty, is_static,
@@ skipping 14 matching lines @@
     // statically we can skip the extra runtime check.
     if (!*is_computed_name) {
       name_expression =
           factory()->NewStringLiteral(name, name_expression->position());
     }
 
     return factory()->NewObjectLiteralProperty(
         name_expression, value,
         is_get ? ObjectLiteralProperty::GETTER : ObjectLiteralProperty::SETTER,
         is_static, *is_computed_name);
-
-  } else if (!in_class && Token::IsIdentifier(name_token, language_mode(),
-                                              this->is_generator())) {
-    DCHECK(!*is_computed_name);
-    DCHECK(!is_static);
-
-    if (classifier->duplicate_finder() != nullptr &&
-        scanner()->FindSymbol(classifier->duplicate_finder(), 1) != 0) {
-      classifier->RecordDuplicateFormalParameterError(scanner()->location());
-    }
-
-    ExpressionT lhs = this->ExpressionFromIdentifier(
-        name, next_beg_pos, next_end_pos, scope_, factory());
-    if (peek() == Token::ASSIGN) {
-      this->ExpressionUnexpectedToken(classifier);
-      Consume(Token::ASSIGN);
-      ExpressionClassifier rhs_classifier;
-      ExpressionT rhs = this->ParseAssignmentExpression(
-          true, &rhs_classifier, CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
-      classifier->Accumulate(rhs_classifier,
-                             ExpressionClassifier::ExpressionProductions);
-      value = factory()->NewAssignment(Token::ASSIGN, lhs, rhs,
-                                       RelocInfo::kNoPosition);
-    } else {
-      value = lhs;
-    }
-    return factory()->NewObjectLiteralProperty(
-        name_expression, value, ObjectLiteralProperty::COMPUTED, false, false);
-
-  } else {
-    Token::Value next = Next();
-    ReportUnexpectedToken(next);
-    *ok = false;
-    return this->EmptyObjectLiteralProperty();
   }
 
-  return factory()->NewObjectLiteralProperty(name_expression, value, is_static,
-                                             *is_computed_name);
+  Token::Value next = Next();
+  ReportUnexpectedToken(next);
+  *ok = false;
+  return this->EmptyObjectLiteralProperty();
 }
 
 
 template <class Traits>
 typename ParserBase<Traits>::ExpressionT ParserBase<Traits>::ParseObjectLiteral(
     ExpressionClassifier* classifier, bool* ok) {
   // ObjectLiteral ::
   // '{' (PropertyDefinition (',' PropertyDefinition)* ','? )? '}'
 
   int pos = peek_position();
@@ skipping 1415 matching lines @@
       *ok = false;
       return;
     }
     has_seen_constructor_ = true;
     return;
   }
 }
 } }  // v8::internal
 
 #endif  // V8_PREPARSER_H