Split the AST representation of class properties from object properties

src/parsing/parser-base.h

Index: src/parsing/parser-base.h
diff --git a/src/parsing/parser-base.h b/src/parsing/parser-base.h
index 96741fb4364a97f0639dd52f303f2a7651d6e5e5..806a53d7bab7b3a8e068a14e8d29d05975b29946 100644
--- a/src/parsing/parser-base.h
+++ b/src/parsing/parser-base.h
@@ -183,6 +183,7 @@ struct FormalParametersBase {
 //   typedef Expression;
 //   typedef FunctionLiteral;
 //   typedef ObjectLiteralProperty;
+//   typedef ClassLiteralProperty;
 //   typedef ExpressionList;
 //   typedef PropertyList;
 //   typedef FormalParameters;
@@ -204,6 +205,7 @@ class ParserBase {
   typedef typename Types::Expression ExpressionT;
   typedef typename Types::FunctionLiteral FunctionLiteralT;
   typedef typename Types::ObjectLiteralProperty ObjectLiteralPropertyT;
+  typedef typename Types::ClassLiteralProperty ClassLiteralPropertyT;
   typedef typename Types::ExpressionList ExpressionListT;
   typedef typename Types::PropertyList PropertyListT;
   typedef typename Types::FormalParameters FormalParametersT;
@@ -294,7 +296,8 @@ class ParserBase {
   // clang-format on
 
   class Checkpoint;
-  class ObjectLiteralCheckerBase;
+  class ClassLiteralChecker;
+  class ObjectLiteralChecker;
 
   // ---------------------------------------------------------------------------
   // ScopeState and its subclasses implement the parser's scope stack.
@@ -1132,14 +1135,15 @@ class ParserBase {
 
   bool SetPropertyKindFromToken(Token::Value token, PropertyKind* kind);
   ExpressionT ParsePropertyName(IdentifierT* name, PropertyKind* kind,
-                                bool in_class, bool* is_generator, bool* is_get,
-                                bool* is_set, bool* is_async, bool* is_static,
-                                bool* is_computed_name, bool* ok);
-
+                                bool* is_generator, bool* is_get, bool* is_set,
+                                bool* is_async, bool* is_computed_name,
+                                bool* ok);
   ExpressionT ParseObjectLiteral(bool* ok);
-  ObjectLiteralPropertyT ParsePropertyDefinition(
-      ObjectLiteralCheckerBase* checker, bool in_class, bool has_extends,
-      bool* is_computed_name, bool* has_seen_constructor, IdentifierT* name,
+  ClassLiteralPropertyT ParseClassPropertyDefinition(
+      ClassLiteralChecker* checker, bool has_extends, bool* is_computed_name,
+      bool* has_seen_constructor, IdentifierT* name, bool* ok);
+  ObjectLiteralPropertyT ParseObjectPropertyDefinition(
+      ObjectLiteralChecker* checker, bool* is_computed_name, IdentifierT* name,
       bool* ok);
   ExpressionListT ParseArguments(Scanner::Location* first_spread_pos,
                                  bool maybe_arrow, bool* ok);
@@ -1225,46 +1229,32 @@ class ParserBase {
     return Call::NOT_EVAL;
   }
 
-  class ObjectLiteralCheckerBase {
-   public:
-    explicit ObjectLiteralCheckerBase(ParserBase* parser) : parser_(parser) {}
-
-    virtual void CheckProperty(Token::Value property, PropertyKind type,
-                               MethodKind method_type, bool* ok) = 0;
-
-    virtual ~ObjectLiteralCheckerBase() {}
-
-   protected:
-    ParserBase* parser() const { return parser_; }
-    Scanner* scanner() const { return parser_->scanner(); }
-
-   private:
-    ParserBase* parser_;
-  };
-
   // Validation per ES6 object literals.
-  class ObjectLiteralChecker : public ObjectLiteralCheckerBase {
+  class ObjectLiteralChecker {
    public:
     explicit ObjectLiteralChecker(ParserBase* parser)
-        : ObjectLiteralCheckerBase(parser), has_seen_proto_(false) {}
+        : parser_(parser), has_seen_proto_(false) {}
 
-    void CheckProperty(Token::Value property, PropertyKind type,
-                       MethodKind method_type, bool* ok) override;
+    void CheckDuplicateProto(Token::Value property);
 
    private:
     bool IsProto() { return this->scanner()->LiteralMatches("__proto__", 9); }
 
+    ParserBase* parser() const { return parser_; }
+    Scanner* scanner() const { return parser_->scanner(); }
+
+    ParserBase* parser_;
     bool has_seen_proto_;
   };
 
   // Validation per ES6 class literals.
-  class ClassLiteralChecker : public ObjectLiteralCheckerBase {
+  class ClassLiteralChecker {
    public:
     explicit ClassLiteralChecker(ParserBase* parser)
-        : ObjectLiteralCheckerBase(parser), has_seen_constructor_(false) {}
+        : parser_(parser), has_seen_constructor_(false) {}
 
-    void CheckProperty(Token::Value property, PropertyKind type,
-                       MethodKind method_type, bool* ok) override;
+    void CheckClassMethodName(Token::Value property, PropertyKind type,
+                              MethodKind method_type, bool* ok);
 
    private:
     bool IsConstructor() {
@@ -1274,6 +1264,10 @@ class ParserBase {
       return this->scanner()->LiteralMatches("prototype", 9);
     }
 
+    ParserBase* parser() const { return parser_; }
+    Scanner* scanner() const { return parser_->scanner(); }
+
+    ParserBase* parser_;
     bool has_seen_constructor_;
   };
 
@@ -1916,15 +1910,16 @@ bool ParserBase<Impl>::SetPropertyKindFromToken(Token::Value token,
 
 template <class Impl>
 typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParsePropertyName(
-    IdentifierT* name, PropertyKind* kind, bool in_class, bool* is_generator,
-    bool* is_get, bool* is_set, bool* is_async, bool* is_static,
-    bool* is_computed_name, bool* ok) {
-  DCHECK(*kind == PropertyKind::kNotSet);
+    IdentifierT* name, PropertyKind* kind, bool* is_generator, bool* is_get,
+    bool* is_set, bool* is_async, bool* is_computed_name, bool* ok) {
+  DCHECK(*kind == PropertyKind::kNotSet ||
+         *kind == PropertyKind::kMethodProperty);  // The second case occurs
+                                                   // when parsing static fields
+                                                   // of classes.
   DCHECK(!*is_generator);
   DCHECK(!*is_get);
   DCHECK(!*is_set);
   DCHECK(!*is_async);
-  DCHECK(!*is_static);
   DCHECK(!*is_computed_name);
 
   *is_generator = Check(Token::MUL);
@@ -1935,19 +1930,6 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParsePropertyName(
   Token::Value token = peek();
   int pos = peek_position();
 
-  if (in_class && !*is_generator && token == Token::STATIC) {
-    Consume(Token::STATIC);
-    *kind = PropertyKind::kMethodProperty;
-    if (peek() == Token::LPAREN) {
-      *name = impl()->GetSymbol();  // TODO(bakkot) specialize on 'static'
-      return factory()->NewStringLiteral(*name, pos);
-    }
-    *is_generator = Check(Token::MUL);
-    *is_static = true;
-    token = peek();
-    pos = peek_position();
-  }
-
   if (allow_harmony_async_await() && !*is_generator && token == Token::ASYNC &&
       !scanner()->HasAnyLineTerminatorAfterNext()) {
     Consume(Token::ASYNC);
@@ -1965,25 +1947,17 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParsePropertyName(
   }
 
   if (token == Token::IDENTIFIER && !*is_generator && !*is_async) {
-    // This is checking for 'get' and 'set' in particular. Any other identifier
-    // must be the property name, and so SetPropertyKindFromToken will return
-    // true (unless there's a syntax error).
+    // This is checking for 'get' and 'set' in particular.
     Consume(Token::IDENTIFIER);
     token = peek();
-    if (SetPropertyKindFromToken(token, kind)) {
+    if (SetPropertyKindFromToken(token, kind) ||
+        !scanner()->IsGetOrSet(is_get, is_set)) {
       *name = impl()->GetSymbol();
       if (fni_ != nullptr) {
         impl()->PushLiteralName(fni_, *name);
       }
       return factory()->NewStringLiteral(*name, pos);
     }
-    scanner()->IsGetOrSet(is_get, is_set);
-    if (!*is_get && !*is_set) {  // TODO(bakkot) have IsGetOrSet return a bool
-      Token::Value next = Next();
-      ReportUnexpectedToken(next);
-      *ok = false;
-      return impl()->EmptyExpression();
-    }
     *kind = PropertyKind::kAccessorProperty;
     pos = peek_position();
   }
@@ -2032,17 +2006,8 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParsePropertyName(
   }
 
   if (*kind == PropertyKind::kNotSet) {
-    if (!SetPropertyKindFromToken(peek(), kind) ||
-        (*kind == PropertyKind::kShorthandProperty &&
-         !Token::IsIdentifier(token, language_mode(), this->is_generator(),
-                              parsing_module_ || is_async_function()))) {
-      Token::Value next = Next();
-      ReportUnexpectedToken(next);
-      *ok = false;
-      return impl()->EmptyExpression();
-    }
+    SetPropertyKindFromToken(peek(), kind);
   }
-  DCHECK(*kind != PropertyKind::kNotSet);
 
   if (*is_computed_name) {
     return expression;
@@ -2059,13 +2024,13 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParsePropertyName(
 }
 
 template <typename Impl>
-typename ParserBase<Impl>::ObjectLiteralPropertyT
-ParserBase<Impl>::ParsePropertyDefinition(ObjectLiteralCheckerBase* checker,
-                                          bool in_class, bool has_extends,
-                                          bool* is_computed_name,
-                                          bool* has_seen_constructor,
-                                          IdentifierT* name, bool* ok) {
-  DCHECK(!in_class || has_seen_constructor != nullptr);
+typename ParserBase<Impl>::ClassLiteralPropertyT
+ParserBase<Impl>::ParseClassPropertyDefinition(ClassLiteralChecker* checker,
+                                               bool has_extends,
+                                               bool* is_computed_name,
+                                               bool* has_seen_constructor,
+                                               IdentifierT* name, bool* ok) {
+  DCHECK(has_seen_constructor != nullptr);
   bool is_get = false;
   bool is_set = false;
   bool is_generator = false;
@@ -2074,29 +2039,149 @@ ParserBase<Impl>::ParsePropertyDefinition(ObjectLiteralCheckerBase* checker,
   PropertyKind kind = PropertyKind::kNotSet;
 
   Token::Value name_token = peek();
-  int next_beg_pos = scanner()->peek_location().beg_pos;
-  int next_end_pos = scanner()->peek_location().end_pos;
 
-  ExpressionT name_expression =
-      ParsePropertyName(name, &kind, in_class, &is_generator, &is_get, &is_set,
-                        &is_async, &is_static, is_computed_name,
-                        CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
+  ExpressionT name_expression;
+  if (name_token == Token::STATIC) {
+    Consume(Token::STATIC);
+    kind = PropertyKind::kMethodProperty;

[adamk, 2016/09/01 21:18:23]

If you move this into the LPAREN branch, then you don't need the weird || or
comment in the kNotSet DCHECK in ParsePropertyName.

[bakkot, 2016/09/01 23:24:31]

Done. This was historical, not really intentional. It had the effect that
ParsePropertyName would necessarily set kind to either Method or Accessor, so
that if the name was followed by a colon it would fail in the Method branch
below. With this change, 'static x :' will be interpreted as a ValueProperty by
ParsePropertyName (since it will only see the 'x :' part), and then the error
will be thrown by hitting the ValueProperty branch.

+    if (peek() == Token::LPAREN) {
+      *name = impl()->GetSymbol();  // TODO(bakkot) specialize on 'static'
+      name_expression = factory()->NewStringLiteral(*name, position());
+    } else {
+      is_static = true;
+      name_expression = ParsePropertyName(
+          name, &kind, &is_generator, &is_get, &is_set, &is_async,
+          is_computed_name, CHECK_OK_CUSTOM(EmptyClassLiteralProperty));
+    }
+  } else {
+    name_expression = ParsePropertyName(
+        name, &kind, &is_generator, &is_get, &is_set, &is_async,
+        is_computed_name, CHECK_OK_CUSTOM(EmptyClassLiteralProperty));
+  }
 
   switch (kind) {
+    case PropertyKind::kShorthandProperty:
     case PropertyKind::kValueProperty: {
-      if (in_class) {
-        Token::Value next = Next();
-        ReportUnexpectedToken(next);
-        *ok = false;
-        return impl()->EmptyObjectLiteralProperty();
+      Token::Value next = Next();

[adamk, 2016/09/01 21:18:23]

If you inline the Next() call into the call to ReportUnexpectedToken, you don't
need the curlies around this case block (also saves a line). Similar comment for
other calls to ReportUnexpectedToken().

[bakkot, 2016/09/01 23:24:31]

Done.

+      ReportUnexpectedToken(next);
+      *ok = false;
+      return impl()->EmptyClassLiteralProperty();
+    }
+
+    case PropertyKind::kMethodProperty: {
+      DCHECK(!is_get && !is_set);
+
+      MethodKind method_kind = MethodKind::kNormal;
+      if (is_generator) {
+        method_kind |= MethodKind::kGenerator;

[adamk, 2016/09/01 21:18:23]

I'm not sure MethodKind is buying us a whole lot of readability. The only
callers of CheckClassMethodName already have bools available called
is_generator, is_async, and is_static, and already have to pass a bunch of bools
in the correct order to ParsePropertyName. And then we turn them back into bools
inside!

What would you think of just having CheckClassMethodName take bools, and getting
rid of all these bitwise operations?

[bakkot, 2016/09/01 23:24:31]

Done.

Yeah, that's much better. MethodKind requires much more code than it's worth.
It's been getting progressively less useful with these patches.

+      }
+      if (is_async) {
+        method_kind |= MethodKind::kAsync;
+      }
+      if (is_static) {
+        method_kind |= MethodKind::kStatic;
       }
 
-      DCHECK(!is_get && !is_set && !is_generator && !is_async && !is_static);
+      // MethodDefinition
+      //    PropertyName '(' StrictFormalParameters ')' '{' FunctionBody '}'
+      //    '*' PropertyName '(' StrictFormalParameters ')' '{' FunctionBody '}'
 
       if (!*is_computed_name) {
-        checker->CheckProperty(name_token, PropertyKind::kValueProperty,
-                               MethodKind::kNormal,
-                               CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
+        checker->CheckClassMethodName(
+            name_token, PropertyKind::kMethodProperty, method_kind,
+            CHECK_OK_CUSTOM(EmptyClassLiteralProperty));
+      }
+
+      FunctionKind kind = is_generator
+                              ? FunctionKind::kConciseGeneratorMethod
+                              : is_async ? FunctionKind::kAsyncConciseMethod
+                                         : FunctionKind::kConciseMethod;
+
+      if (!is_static && impl()->IsConstructor(*name)) {
+        *has_seen_constructor = true;
+        kind = has_extends ? FunctionKind::kSubclassConstructor
+                           : FunctionKind::kBaseConstructor;
+      }
+
+      ExpressionT value = impl()->ParseFunctionLiteral(
+          *name, scanner()->location(), kSkipFunctionNameCheck, kind,
+          kNoSourcePosition, FunctionLiteral::kAccessorOrMethod,
+          language_mode(), CHECK_OK_CUSTOM(EmptyClassLiteralProperty));
+
+      return factory()->NewClassLiteralProperty(name_expression, value,
+                                                ClassLiteralProperty::METHOD,
+                                                is_static, *is_computed_name);
+    }
+
+    case PropertyKind::kAccessorProperty: {
+      DCHECK((is_get || is_set) && !is_generator && !is_async);
+
+      MethodKind method_kind = MethodKind::kNormal;
+      if (is_static) {
+        method_kind |= MethodKind::kStatic;
+      }
+
+      if (!*is_computed_name) {
+        checker->CheckClassMethodName(
+            name_token, PropertyKind::kAccessorProperty, method_kind,
+            CHECK_OK_CUSTOM(EmptyClassLiteralProperty));
+        // Make sure the name expression is a string since we need a Name for
+        // Runtime_DefineAccessorPropertyUnchecked and since we can determine
+        // this statically we can skip the extra runtime check.
+        name_expression =
+            factory()->NewStringLiteral(*name, name_expression->position());
+      }
+
+      FunctionKind kind = is_get ? FunctionKind::kGetterFunction
+                                 : FunctionKind::kSetterFunction;
+
+      ExpressionT value = impl()->ParseFunctionLiteral(
+          *name, scanner()->location(), kSkipFunctionNameCheck, kind,
+          kNoSourcePosition, FunctionLiteral::kAccessorOrMethod,
+          language_mode(), CHECK_OK_CUSTOM(EmptyClassLiteralProperty));
+
+      return factory()->NewClassLiteralProperty(
+          name_expression, value,
+          is_get ? ClassLiteralProperty::GETTER : ClassLiteralProperty::SETTER,
+          is_static, *is_computed_name);
+    }
+
+    case PropertyKind::kNotSet: {
+      Token::Value next = Next();
+      ReportUnexpectedToken(next);
+      *ok = false;
+      return impl()->EmptyClassLiteralProperty();
+    }
+  }
+  UNREACHABLE();
+  return impl()->EmptyClassLiteralProperty();
+}
+
+template <typename Impl>
+typename ParserBase<Impl>::ObjectLiteralPropertyT
+ParserBase<Impl>::ParseObjectPropertyDefinition(ObjectLiteralChecker* checker,
+                                                bool* is_computed_name,
+                                                IdentifierT* name, bool* ok) {
+  bool is_get = false;
+  bool is_set = false;
+  bool is_generator = false;
+  bool is_async = false;
+  PropertyKind kind = PropertyKind::kNotSet;
+
+  Token::Value name_token = peek();
+  int next_beg_pos = scanner()->peek_location().beg_pos;
+  int next_end_pos = scanner()->peek_location().end_pos;
+
+  ExpressionT name_expression = ParsePropertyName(
+      name, &kind, &is_generator, &is_get, &is_set, &is_async, is_computed_name,
+      CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
+
+  switch (kind) {
+    case PropertyKind::kValueProperty: {
+      DCHECK(!is_get && !is_set && !is_generator && !is_async);
+
+      if (!*is_computed_name) {
+        checker->CheckDuplicateProto(name_token);
       }
       Consume(Token::COLON);
       int beg_pos = peek_position();
@@ -2105,7 +2190,7 @@ ParserBase<Impl>::ParsePropertyDefinition(ObjectLiteralCheckerBase* checker,
       CheckDestructuringElement(value, beg_pos, scanner()->location().end_pos);
 
       return factory()->NewObjectLiteralProperty(name_expression, value,
-                                                 is_static, *is_computed_name);
+                                                 *is_computed_name);
     }
 
     case PropertyKind::kShorthandProperty: {
@@ -2115,15 +2200,18 @@ ParserBase<Impl>::ParsePropertyDefinition(ObjectLiteralCheckerBase* checker,
       //
       // CoverInitializedName
       //    IdentifierReference Initializer?
-      if (in_class) {
+      DCHECK(!is_get && !is_set && !is_generator && !is_async);
+
+      if (!Token::IsIdentifier(name_token, language_mode(),
+                               this->is_generator(),
+                               parsing_module_ || is_async_function())) {
         Token::Value next = Next();
         ReportUnexpectedToken(next);
         *ok = false;
         return impl()->EmptyObjectLiteralProperty();
       }
 
-      DCHECK(!is_get && !is_set && !is_generator && !is_async && !is_static &&
-             !*is_computed_name);
+      DCHECK(!*is_computed_name);
 
       if (classifier()->duplicate_finder() != nullptr &&
           scanner()->FindSymbol(classifier()->duplicate_finder(), 1) != 0) {
@@ -2170,24 +2258,12 @@ ParserBase<Impl>::ParsePropertyDefinition(ObjectLiteralCheckerBase* checker,
       }
 
       return factory()->NewObjectLiteralProperty(
-          name_expression, value, ObjectLiteralProperty::COMPUTED, is_static,
-          false);
+          name_expression, value, ObjectLiteralProperty::COMPUTED, false);
     }
 
     case PropertyKind::kMethodProperty: {
       DCHECK(!is_get && !is_set);
 
-      MethodKind method_kind = MethodKind::kNormal;
-      if (is_generator) {
-        method_kind |= MethodKind::kGenerator;
-      }
-      if (is_async) {
-        method_kind |= MethodKind::kAsync;
-      }
-      if (is_static) {
-        method_kind |= MethodKind::kStatic;
-      }
-
       // MethodDefinition
       //    PropertyName '(' StrictFormalParameters ')' '{' FunctionBody '}'
       //    '*' PropertyName '(' StrictFormalParameters ')' '{' FunctionBody '}'
@@ -2196,74 +2272,56 @@ ParserBase<Impl>::ParsePropertyDefinition(ObjectLiteralCheckerBase* checker,
           Scanner::Location(next_beg_pos, scanner()->location().end_pos),
           MessageTemplate::kInvalidDestructuringTarget);
 
-      if (!*is_computed_name) {
-        checker->CheckProperty(name_token, PropertyKind::kMethodProperty,
-                               method_kind,
-                               CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
-      }
-
       FunctionKind kind = is_generator
                               ? FunctionKind::kConciseGeneratorMethod
                               : is_async ? FunctionKind::kAsyncConciseMethod
                                          : FunctionKind::kConciseMethod;
 
-      if (in_class && !is_static && impl()->IsConstructor(*name)) {
-        *has_seen_constructor = true;
-        kind = has_extends ? FunctionKind::kSubclassConstructor
-                           : FunctionKind::kBaseConstructor;
-      }
-
       ExpressionT value = impl()->ParseFunctionLiteral(
           *name, scanner()->location(), kSkipFunctionNameCheck, kind,
           kNoSourcePosition, FunctionLiteral::kAccessorOrMethod,
           language_mode(), CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
 
       return factory()->NewObjectLiteralProperty(
-          name_expression, value, ObjectLiteralProperty::COMPUTED, is_static,
+          name_expression, value, ObjectLiteralProperty::COMPUTED,
           *is_computed_name);
     }
 
     case PropertyKind::kAccessorProperty: {
       DCHECK((is_get || is_set) && !is_generator && !is_async);
 
-      MethodKind method_kind = MethodKind::kNormal;
-      if (is_static) {
-        DCHECK(in_class);
-        method_kind |= MethodKind::kStatic;
-      }
-
       classifier()->RecordPatternError(
           Scanner::Location(next_beg_pos, scanner()->location().end_pos),
           MessageTemplate::kInvalidDestructuringTarget);
-      if (!*is_computed_name) {
-        checker->CheckProperty(name_token, PropertyKind::kAccessorProperty,
-                               method_kind,
-                               CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
-      }
-
-      FunctionLiteralT value = impl()->ParseFunctionLiteral(
-          *name, scanner()->location(), kSkipFunctionNameCheck,
-          is_get ? FunctionKind::kGetterFunction
-                 : FunctionKind::kSetterFunction,
-          kNoSourcePosition, FunctionLiteral::kAccessorOrMethod,
-          language_mode(), CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
 
-      // Make sure the name expression is a string since we need a Name for
-      // Runtime_DefineAccessorPropertyUnchecked and since we can determine this
-      // statically we can skip the extra runtime check.
       if (!*is_computed_name) {
+        // Make sure the name expression is a string since we need a Name for
+        // Runtime_DefineAccessorPropertyUnchecked and since we can determine
+        // this statically we can skip the extra runtime check.
         name_expression =
             factory()->NewStringLiteral(*name, name_expression->position());
       }
 
+      FunctionKind kind = is_get ? FunctionKind::kGetterFunction
+                                 : FunctionKind::kSetterFunction;
+
+      ExpressionT value = impl()->ParseFunctionLiteral(
+          *name, scanner()->location(), kSkipFunctionNameCheck, kind,
+          kNoSourcePosition, FunctionLiteral::kAccessorOrMethod,
+          language_mode(), CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
+
       return factory()->NewObjectLiteralProperty(
           name_expression, value, is_get ? ObjectLiteralProperty::GETTER
                                          : ObjectLiteralProperty::SETTER,
-          is_static, *is_computed_name);
+          *is_computed_name);
     }
 
-    case PropertyKind::kNotSet:
-      UNREACHABLE();
+    case PropertyKind::kNotSet: {

[adamk, 2016/09/01 21:18:23]

Can you explain why it's better to have duplication of this error-reporting
rather than handling it in ParsePropertyName? I suspect I'm missing something.

[bakkot, 2016/09/01 23:24:31]

Discussed offline. The real reason is that it makes adding class fields later
easier. The ideological reason is that ParsePropertyName shouldn't really be
throwing errors for unexpected tokens past the end of the property name; that's
the responsibility of whatever wants to consume that token.

+      Token::Value next = Next();
+      ReportUnexpectedToken(next);
+      *ok = false;
+      return impl()->EmptyObjectLiteralProperty();
+    }
   }
   UNREACHABLE();
   return impl()->EmptyObjectLiteralProperty();
@@ -2286,13 +2344,10 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParseObjectLiteral(
   while (peek() != Token::RBRACE) {
     FuncNameInferrer::State fni_state(fni_);
 
-    const bool in_class = false;
-    const bool has_extends = false;
     bool is_computed_name = false;
     IdentifierT name = impl()->EmptyIdentifier();
-    ObjectLiteralPropertyT property =
-        ParsePropertyDefinition(&checker, in_class, has_extends,
-                                &is_computed_name, nullptr, &name, CHECK_OK);
+    ObjectLiteralPropertyT property = ParseObjectPropertyDefinition(
+        &checker, &is_computed_name, &name, CHECK_OK);
 
     if (is_computed_name) {
       has_computed_names = true;
@@ -3917,16 +3972,11 @@ void ParserBase<Impl>::CheckDestructuringElement(ExpressionT expression,
 #undef CHECK_OK_CUSTOM
 
 template <typename Impl>
-void ParserBase<Impl>::ObjectLiteralChecker::CheckProperty(
-    Token::Value property, PropertyKind type, MethodKind method_type,
-    bool* ok) {
-  DCHECK(!IsStaticMethod(method_type));
-  DCHECK(!IsSpecialMethod(method_type) ||
-         type == PropertyKind::kMethodProperty);
-
+void ParserBase<Impl>::ObjectLiteralChecker::CheckDuplicateProto(
+    Token::Value property) {
   if (property == Token::SMI || property == Token::NUMBER) return;
 
-  if (type == PropertyKind::kValueProperty && IsProto()) {
+  if (IsProto()) {
     if (has_seen_proto_) {
       this->parser()->classifier()->RecordExpressionError(
           this->scanner()->location(), MessageTemplate::kDuplicateProto);
@@ -3937,7 +3987,7 @@ void ParserBase<Impl>::ObjectLiteralChecker::CheckProperty(
 }
 
 template <typename Impl>
-void ParserBase<Impl>::ClassLiteralChecker::CheckProperty(
+void ParserBase<Impl>::ClassLiteralChecker::CheckClassMethodName(
     Token::Value property, PropertyKind type, MethodKind method_type,
     bool* ok) {
   DCHECK(type == PropertyKind::kMethodProperty ||