[parser] Allow duplicate __proto__ keys in patterns

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 b3b516ead13d8256bdcfc39806911e9480cb9908..5df2fa4adbca483d8864f4c945d93bda1df38ff3 100644
--- a/src/parsing/parser-base.h
+++ b/src/parsing/parser-base.h
@@ -1182,7 +1182,8 @@ class ParserBase : public Traits {
     explicit ObjectLiteralCheckerBase(ParserBase* parser) : parser_(parser) {}
 
     virtual void CheckProperty(Token::Value property, PropertyKind type,
-                               MethodKind method_type, bool* ok) = 0;
+                               MethodKind method_type,
+                               ExpressionClassifier* classifier, bool* ok) = 0;
 
     virtual ~ObjectLiteralCheckerBase() {}
 
@@ -1201,7 +1202,8 @@ class ParserBase : public Traits {
         : ObjectLiteralCheckerBase(parser), has_seen_proto_(false) {}
 
     void CheckProperty(Token::Value property, PropertyKind type,
-                       MethodKind method_type, bool* ok) override;
+                       MethodKind method_type, ExpressionClassifier* classifier,
+                       bool* ok) override;
 
    private:
     bool IsProto() { return this->scanner()->LiteralMatches("__proto__", 9); }
@@ -1216,7 +1218,8 @@ class ParserBase : public Traits {
         : ObjectLiteralCheckerBase(parser), has_seen_constructor_(false) {}
 
     void CheckProperty(Token::Value property, PropertyKind type,
-                       MethodKind method_type, bool* ok) override;
+                       MethodKind method_type, ExpressionClassifier* classifier,
+                       bool* ok) override;
 
    private:
     bool IsConstructor() {
@@ -1961,6 +1964,7 @@ ParserBase<Traits>::ParsePropertyDefinition(
       //    PropertyName ':' AssignmentExpression
       if (!*is_computed_name) {
         checker->CheckProperty(name_token, kValueProperty, MethodKind::kNormal,
+                               classifier,
                                CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
       }
       Consume(Token::COLON);
@@ -2055,6 +2059,7 @@ ParserBase<Traits>::ParsePropertyDefinition(
     //    '*' PropertyName '(' StrictFormalParameters ')' '{' FunctionBody '}'
     if (!*is_computed_name) {
       checker->CheckProperty(name_token, kMethodProperty, method_kind,
+                             classifier,
                              CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
     }
 
@@ -2105,6 +2110,7 @@ ParserBase<Traits>::ParsePropertyDefinition(
 
     if (!*is_computed_name) {
       checker->CheckProperty(name_token, kAccessorProperty, method_kind,
+                             classifier,
                              CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
     }
 
@@ -2409,6 +2415,7 @@ ParserBase<Traits>::ParseAssignmentExpression(bool accept_IN,
 
   if (IsValidPattern(expression) && peek() == Token::ASSIGN) {
     classifier->ForgiveCoverInitializedNameError();
+    classifier->ForgiveExpressionError();

[adamk, 2016/08/18 20:35:22]

Is this necessary? What fails if you don't "forgive"?

[gsathya, 2016/08/19 20:11:18]

As discussed offline -- leaving this here. 

     ValidateAssignmentPattern(classifier, CHECK_OK);
     is_destructuring_assignment = true;
   } else {
@@ -2439,6 +2446,9 @@ ParserBase<Traits>::ParseAssignmentExpression(bool accept_IN,
           ExpressionClassifier::CoverInitializedNameProduction |
           ExpressionClassifier::AsyncArrowFormalParametersProduction);
 
+  if (!classifier->is_valid_expression())

[adamk, 2016/08/18 20:35:22]

Was this necessary? I wouldn't think you'd need to add this.

[gsathya, 2016/08/19 20:11:18]

Removed.

+    ReportClassifierError(classifier->expression_error());
+
   // TODO(1231235): We try to estimate the set of properties set by
   // constructors. We define a new property whenever there is an
   // assignment to a property of 'this'. We should probably only add
@@ -3677,7 +3687,7 @@ void ParserBase<Traits>::CheckDestructuringElement(
 template <typename Traits>
 void ParserBase<Traits>::ObjectLiteralChecker::CheckProperty(
     Token::Value property, PropertyKind type, MethodKind method_type,
-    bool* ok) {
+    ExpressionClassifier* classifier, bool* ok) {

[adamk, 2016/08/18 20:35:22]

Hmm, it's a little strange that this takes "ok" now that it can never fail, but
I guess that's needed because of the class case?

[gsathya, 2016/08/19 20:11:18]

Yeah :/ It's the same with classifier being added to
ClassLiteralChecker::CheckProperty.

   DCHECK(!IsStaticMethod(method_type));
   DCHECK(!IsSpecialMethod(method_type) || type == kMethodProperty);
 
@@ -3685,19 +3695,19 @@ void ParserBase<Traits>::ObjectLiteralChecker::CheckProperty(
 
   if (type == kValueProperty && IsProto()) {
     if (has_seen_proto_) {
-      this->parser()->ReportMessage(MessageTemplate::kDuplicateProto);
-      *ok = false;
+      classifier->RecordExpressionError(this->scanner()->location(),
+                                        MessageTemplate::kDuplicateProto,
+                                        kSyntaxError);
       return;
     }
     has_seen_proto_ = true;
-    return;
   }
 }
 
 template <typename Traits>
 void ParserBase<Traits>::ClassLiteralChecker::CheckProperty(
     Token::Value property, PropertyKind type, MethodKind method_type,
-    bool* ok) {
+    ExpressionClassifier* classifier, bool* ok) {
   DCHECK(type == kMethodProperty || type == kAccessorProperty);
 
   if (property == Token::SMI || property == Token::NUMBER) return;