Move ParsePostfixExpression into ParserBase.

+ enable a test which checks that Parser and PreParser produce the "invalid left
hand side" errors consistently.

R=mstarzinger@chromium.org
BUG=

Review URL: https://codereview.chromium.org/202333004

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@20149 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/parser.cc

@@ -740,8 +740,8 @@ FunctionLiteral* ParserTraits::ParseFunctionLiteral(
 }
 
 
-Expression* ParserTraits::ParsePostfixExpression(bool* ok) {
-  return parser_->ParsePostfixExpression(ok);
+Expression* ParserTraits::ParseLeftHandSideExpression(bool* ok) {
+  return parser_->ParseLeftHandSideExpression(ok);
 }
 
 
@@ -3043,37 +3043,6 @@ Statement* Parser::ParseForStatement(ZoneStringList* labels, bool* ok) {
 }
 
 
-Expression* Parser::ParsePostfixExpression(bool* ok) {
-  // PostfixExpression ::
-  //   LeftHandSideExpression ('++' | '--')?
-
-  Scanner::Location lhs_location = scanner()->peek_location();
-  Expression* expression = ParseLeftHandSideExpression(CHECK_OK);
-  if (!scanner()->HasAnyLineTerminatorBeforeNext() &&
-      Token::IsCountOp(peek())) {
-    if (expression == NULL || !expression->IsValidLeftHandSide()) {
-      ReportMessageAt(lhs_location, "invalid_lhs_in_postfix_op", true);
-      *ok = false;
-      return NULL;
-    }
-
-    if (strict_mode() == STRICT) {
-      // Postfix expression operand in strict mode may not be eval or arguments.
-      CheckStrictModeLValue(expression, CHECK_OK);
-    }
-    MarkExpressionAsLValue(expression);
-
-    Token::Value next = Next();
-    expression =
-        factory()->NewCountOperation(next,
-                                     false /* postfix */,
-                                     expression,
-                                     position());
-  }
-  return expression;
-}
-
-
 Expression* Parser::ParseLeftHandSideExpression(bool* ok) {
   // LeftHandSideExpression ::
   //   (NewExpression | MemberExpression) ...

src/parser.h

@@ -490,11 +490,6 @@ class ParserTraits {
   static void CheckAssigningFunctionLiteralToProperty(Expression* left,
                                                       Expression* right);
 
-  // Determine whether the expression is a valid assignment left-hand side.
-  static bool IsValidLeftHandSide(Expression* expression) {
-    return expression->IsValidLeftHandSide();
-  }
-
   // Determine if the expression is a variable proxy and mark it as being used
   // in an assignment or with a increment/decrement operator. This is currently
   // used on for the statically checking assignments to harmony const bindings.
@@ -589,7 +584,7 @@ class ParserTraits {
       int function_token_position,
       FunctionLiteral::FunctionType type,
       bool* ok);
-  Expression* ParsePostfixExpression(bool* ok);
+  Expression* ParseLeftHandSideExpression(bool* ok);
 
  private:
   Parser* parser_;
@@ -748,7 +743,6 @@ class Parser : public ParserBase<ParserTraits> {
   Block* ParseScopedBlock(ZoneStringList* labels, bool* ok);
 
   Expression* ParseUnaryExpression(bool* ok);
-  Expression* ParsePostfixExpression(bool* ok);
   Expression* ParseLeftHandSideExpression(bool* ok);
   Expression* ParseMemberWithNewPrefixesExpression(bool* ok);
   Expression* ParseMemberExpression(bool* ok);

src/preparser.cc

@@ -146,8 +146,8 @@ PreParserExpression PreParserTraits::ParseFunctionLiteral(
 }
 
 
-PreParserExpression PreParserTraits::ParsePostfixExpression(bool* ok) {
-  return pre_parser_->ParsePostfixExpression(ok);
+PreParserExpression PreParserTraits::ParseLeftHandSideExpression(bool* ok) {
+  return pre_parser_->ParseLeftHandSideExpression(ok);
 }
 
 
@@ -842,23 +842,6 @@ PreParser::Statement PreParser::ParseDebuggerStatement(bool* ok) {
 #undef DUMMY
 
 
-PreParser::Expression PreParser::ParsePostfixExpression(bool* ok) {
-  // PostfixExpression ::
-  //   LeftHandSideExpression ('++' | '--')?
-
-  Expression expression = ParseLeftHandSideExpression(CHECK_OK);
-  if (!scanner()->HasAnyLineTerminatorBeforeNext() &&
-      Token::IsCountOp(peek())) {
-    if (strict_mode() == STRICT) {
-      CheckStrictModeLValue(expression, CHECK_OK);
-    }
-    Next();
-    return Expression::Default();
-  }
-  return expression;
-}
-
-
 PreParser::Expression PreParser::ParseLeftHandSideExpression(bool* ok) {
   // LeftHandSideExpression ::
   //   (NewExpression | MemberExpression) ...

src/preparser.h

@@ -395,6 +395,7 @@ class ParserBase : public Traits {
   ExpressionT ParseConditionalExpression(bool accept_IN, bool* ok);
   ExpressionT ParseBinaryExpression(int prec, bool accept_IN, bool* ok);
   ExpressionT ParseUnaryExpression(bool* ok);
+  ExpressionT ParsePostfixExpression(bool* ok);
 
   // Used to detect duplicates in object literals. Each of the values
   // kGetterProperty, kSetterProperty and kValueProperty represents
@@ -580,7 +581,12 @@ class PreParserExpression {
     return code_ == kPropertyExpression || code_ == kThisPropertyExpression;
   }
 
-  // Dummy implementation for making expression->AsCall() work (see below).
+  bool IsValidLeftHandSide() {
+    return IsIdentifier() || IsProperty();
+  }
+
+  // Dummy implementation for making expression->somefunc() work in both Parser
+  // and PreParser.
   PreParserExpression* operator->() { return this; }
 
   // These are only used when doing function name inferring, and PreParser
@@ -833,11 +839,6 @@ class PreParserTraits {
   static void CheckAssigningFunctionLiteralToProperty(
       PreParserExpression left, PreParserExpression right) {}
 
-  // Determine whether the expression is a valid assignment left-hand side.
-  static bool IsValidLeftHandSide(PreParserExpression expression) {
-    return expression.IsIdentifier() || expression.IsProperty();
-  }
-
   static PreParserExpression MarkExpressionAsLValue(
       PreParserExpression expression) {
     // TODO(marja): To be able to produce the same errors, the preparser needs
@@ -944,7 +945,7 @@ class PreParserTraits {
       int function_token_position,
       FunctionLiteral::FunctionType type,
       bool* ok);
-  PreParserExpression ParsePostfixExpression(bool* ok);
+  PreParserExpression ParseLeftHandSideExpression(bool* ok);
 
  private:
   PreParser* pre_parser_;
@@ -1108,7 +1109,6 @@ class PreParser : public ParserBase<PreParserTraits> {
   Statement ParseTryStatement(bool* ok);
   Statement ParseDebuggerStatement(bool* ok);
   Expression ParseConditionalExpression(bool accept_IN, bool* ok);
-  Expression ParsePostfixExpression(bool* ok);
   Expression ParseLeftHandSideExpression(bool* ok);
   Expression ParseMemberExpression(bool* ok);
   Expression ParseMemberExpressionContinuation(PreParserExpression expression,
@@ -1664,7 +1664,7 @@ ParserBase<Traits>::ParseAssignmentExpression(bool accept_IN, bool* ok) {
     return expression;
   }
 
-  if (!this->IsValidLeftHandSide(expression)) {
+  if (!expression->IsValidLeftHandSide()) {
     this->ReportMessageAt(lhs_location, "invalid_lhs_in_assignment", true);
     *ok = false;
     return this->EmptyExpression();
@@ -1834,7 +1834,7 @@ ParserBase<Traits>::ParseUnaryExpression(bool* ok) {
     op = Next();
     Scanner::Location lhs_location = scanner()->peek_location();
     ExpressionT expression = ParseUnaryExpression(CHECK_OK);
-    if (!this->IsValidLeftHandSide(expression)) {
+    if (!expression->IsValidLeftHandSide()) {
       ReportMessageAt(lhs_location, "invalid_lhs_in_prefix_op", true);
       *ok = false;
       return this->EmptyExpression();
@@ -1857,6 +1857,39 @@ ParserBase<Traits>::ParseUnaryExpression(bool* ok) {
 }
 
 
+template <class Traits>
+typename ParserBase<Traits>::ExpressionT
+ParserBase<Traits>::ParsePostfixExpression(bool* ok) {
+  // PostfixExpression ::
+  //   LeftHandSideExpression ('++' | '--')?
+
+  Scanner::Location lhs_location = scanner()->peek_location();
+  ExpressionT expression = this->ParseLeftHandSideExpression(CHECK_OK);
+  if (!scanner()->HasAnyLineTerminatorBeforeNext() &&
+      Token::IsCountOp(peek())) {
+    if (!expression->IsValidLeftHandSide()) {
+      ReportMessageAt(lhs_location, "invalid_lhs_in_postfix_op", true);
+      *ok = false;
+      return this->EmptyExpression();
+    }
+
+    if (strict_mode() == STRICT) {
+      // Postfix expression operand in strict mode may not be eval or arguments.
+      this->CheckStrictModeLValue(expression, CHECK_OK);
+    }
+    expression = this->MarkExpressionAsLValue(expression);
+
+    Token::Value next = Next();
+    expression =
+        factory()->NewCountOperation(next,
+                                     false /* postfix */,
+                                     expression,
+                                     position());
+  }
+  return expression;
+}
+
+
 #undef CHECK_OK
 #undef CHECK_OK_CUSTOM
 

test/cctest/test-parsing.cc

@@ -2332,8 +2332,7 @@ TEST(ErrorsNewExpression) {
     "new foo bar",
     "new ) foo",
     "new ++foo",
-    // TODO(marja): Activate this test once the preparser checks correctly.
-    // "new foo ++",
+    "new foo ++",
     NULL
   };
 
@@ -2528,7 +2527,7 @@ TEST(StrictDelete) {
 }
 
 
-TEST(ErrorInvalidLeftHandSide) {
+TEST(InvalidLeftHandSide) {
   const char* assignment_context_data[][2] = {
     // {"", " = 1;"},
     // {"\"use strict\"; ", " = 1;"},
@@ -2593,6 +2592,5 @@ TEST(ErrorInvalidLeftHandSide) {
   RunParserSyncTest(prefix_context_data, bad_statement_data_common, kError);
 
   RunParserSyncTest(postfix_context_data, good_statement_data, kSuccess);
-  // TODO(marja): This doesn't work yet.
-  // RunParserSyncTest(postfix_context_data, bad_statement_data_common, kError);
+  RunParserSyncTest(postfix_context_data, bad_statement_data_common, kError);
 }