Revert "Unify paren handling in Parser and PreParser."

This reverts r19140.

Reason: Octane regression.

BUG=
TBR=verwaest@chromium.org

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

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

src/ast.cc

@@ -82,16 +82,6 @@ bool Expression::IsUndefinedLiteral(Isolate* isolate) {
 }
 
 
-bool Expression::IsIdentifier() {
-  return (AsVariableProxy() != NULL && !AsVariableProxy()->is_this());
-}
-
-
-bool Expression::IsIdentifierNamed(String* name) {
-  return (AsVariableProxy() != NULL && AsVariableProxy()->name()->Equals(name));
-}
-
-
 VariableProxy::VariableProxy(Zone* zone, Variable* var, int position)
     : Expression(zone, position),
       name_(var->name()),

src/ast.h

@@ -351,10 +351,6 @@ class Expression : public AstNode {
   // True if we can prove that the expression is the undefined literal.
   bool IsUndefinedLiteral(Isolate* isolate);
 
-  bool IsIdentifier();
-
-  bool IsIdentifierNamed(String* name);
-
   // Expression type bounds
   Bounds bounds() { return bounds_; }
   void set_bounds(Bounds bounds) { bounds_ = bounds; }

src/parser.cc

@@ -964,8 +964,10 @@ Statement* Parser::ParseModuleElement(ZoneStringList* labels,
           !scanner().HasAnyLineTerminatorBeforeNext() &&
           stmt != NULL) {
         ExpressionStatement* estmt = stmt->AsExpressionStatement();
-        if (estmt != NULL && estmt->expression()->IsIdentifierNamed(
-                                 isolate()->heap()->module_string()) &&
+        if (estmt != NULL &&
+            estmt->expression()->AsVariableProxy() != NULL &&
+            estmt->expression()->AsVariableProxy()->name()->Equals(
+                isolate()->heap()->module_string()) &&
             !scanner().literal_contains_escapes()) {
           return ParseModuleDeclaration(NULL, ok);
         }
@@ -2124,11 +2126,9 @@ Statement* Parser::ParseExpressionOrLabelledStatement(ZoneStringList* labels,
   int pos = peek_position();
   bool starts_with_idenfifier = peek_any_identifier();
   Expression* expr = ParseExpression(true, CHECK_OK);
-  // Even if the expression starts with an identifier, it is not necessarily an
-  // identifier. For example, "foo + bar" starts with an identifier but is not
-  // an identifier.
   if (peek() == Token::COLON && starts_with_idenfifier && expr != NULL &&
-      expr->IsIdentifier()) {
+      expr->AsVariableProxy() != NULL &&
+      !expr->AsVariableProxy()->is_this()) {
     // Expression is a single identifier, and not, e.g., a parenthesized
     // identifier.
     VariableProxy* var = expr->AsVariableProxy();
@@ -2165,7 +2165,9 @@ Statement* Parser::ParseExpressionOrLabelledStatement(ZoneStringList* labels,
       peek() == Token::FUNCTION &&
       !scanner().HasAnyLineTerminatorBeforeNext() &&
       expr != NULL &&
-      expr->IsIdentifierNamed(isolate()->heap()->native_string()) &&
+      expr->AsVariableProxy() != NULL &&
+      expr->AsVariableProxy()->name()->Equals(
+          isolate()->heap()->native_string()) &&
       !scanner().literal_contains_escapes()) {
     return ParseNativeDeclaration(ok);
   }
@@ -2175,7 +2177,9 @@ Statement* Parser::ParseExpressionOrLabelledStatement(ZoneStringList* labels,
   if (!FLAG_harmony_modules ||
       peek() != Token::IDENTIFIER ||
       scanner().HasAnyLineTerminatorBeforeNext() ||
-      !expr->IsIdentifierNamed(isolate()->heap()->module_string()) ||
+      expr->AsVariableProxy() == NULL ||
+      !expr->AsVariableProxy()->name()->Equals(
+          isolate()->heap()->module_string()) ||
       scanner().literal_contains_escapes()) {
     ExpectSemicolon(CHECK_OK);
   }
@@ -2942,7 +2946,8 @@ Expression* Parser::ParseAssignmentExpression(bool accept_IN, bool* ok) {
   Property* property = expression ? expression->AsProperty() : NULL;
   if (op == Token::ASSIGN &&
       property != NULL &&
-      property->obj()->IsIdentifier()) {
+      property->obj()->AsVariableProxy() != NULL &&
+      property->obj()->AsVariableProxy()->is_this()) {
     current_function_state_->AddProperty();
   }
 

src/preparser.cc

@@ -168,18 +168,15 @@ PreParser::SourceElements PreParser::ParseSourceElements(int end_token,
   // SourceElements ::
   //   (Statement)* <end_token>
 
-  bool directive_prologue = true;
+  bool allow_directive_prologue = true;
   while (peek() != end_token) {
-    if (directive_prologue && peek() != Token::STRING) {
-      directive_prologue = false;
-    }
     Statement statement = ParseSourceElement(CHECK_OK);
-    if (directive_prologue) {
+    if (allow_directive_prologue) {
       if (statement.IsUseStrictLiteral()) {
         set_language_mode(allow_harmony_scoping() ?
                           EXTENDED_MODE : STRICT_MODE);
       } else if (!statement.IsStringLiteral()) {
-        directive_prologue = false;
+        allow_directive_prologue = false;
       }
     }
   }
@@ -471,20 +468,16 @@ PreParser::Statement PreParser::ParseExpressionOrLabelledStatement(bool* ok) {
   //   Expression ';'
   //   Identifier ':' Statement
 
-  bool starts_with_identifier = peek_any_identifier();
   Expression expr = ParseExpression(true, CHECK_OK);
-  // Even if the expression starts with an identifier, it is not necessarily an
-  // identifier. For example, "foo + bar" starts with an identifier but is not
-  // an identifier.
-  if (peek() == Token::COLON && starts_with_identifier && expr.IsIdentifier()) {
-    // Expression is a single identifier, and not, e.g., a parenthesized
-    // identifier.
+  if (expr.IsRawIdentifier()) {
     ASSERT(!expr.AsIdentifier().IsFutureReserved());
     ASSERT(is_classic_mode() ||
            (!expr.AsIdentifier().IsFutureStrictReserved() &&
             !expr.AsIdentifier().IsYield()));
-    Consume(Token::COLON);
-    return ParseStatement(ok);
+    if (peek() == Token::COLON) {
+      Consume(Token::COLON);
+      return ParseStatement(ok);
+    }
     // Preparsing is disabled for extensions (because the extension details
     // aren't passed to lazily compiled functions), so we don't
     // accept "native function" in the preparser.
@@ -1177,6 +1170,7 @@ PreParser::Expression PreParser::ParsePrimaryExpression(bool* ok) {
       parenthesized_function_ = (peek() == Token::FUNCTION);
       result = ParseExpression(true, CHECK_OK);
       Expect(Token::RPAREN, CHECK_OK);
+      result = result.Parenthesize();
       break;
 
     case Token::MOD:

src/preparser.h

@@ -347,6 +347,8 @@ class PreParser : public ParserBase {
   // if bit 1 is set, it's a string literal.
   // If neither is set, it's no particular type, and both set isn't
   // use yet.
+  // Bit 2 is used to mark the expression as being parenthesized,
+  // so "(foo)" isn't recognized as a pure identifier (and possible label).
   class Expression {
    public:
     static Expression Default() {
@@ -387,8 +389,21 @@ class PreParser : public ParserBase {
           static_cast<PreParser::Identifier::Type>(code_ >> kIdentifierShift));
     }
 
+    bool IsParenthesized() {
+      // If bit 0 or 1 is set, we interpret bit 2 as meaning parenthesized.
+      return (code_ & 7) > 4;
+    }
+
+    bool IsRawIdentifier() {
+      return !IsParenthesized() && IsIdentifier();
+    }
+
     bool IsStringLiteral() { return (code_ & kStringLiteralFlag) != 0; }
 
+    bool IsRawStringLiteral() {
+      return !IsParenthesized() && IsStringLiteral();
+    }
+
     bool IsUseStrictLiteral() {
       return (code_ & kStringLiteralMask) == kUseStrictString;
     }
@@ -405,10 +420,27 @@ class PreParser : public ParserBase {
       return code_ == kStrictFunctionExpression;
     }
 
+    Expression Parenthesize() {
+      int type = code_ & 3;
+      if (type != 0) {
+        // Identifiers and string literals can be parenthesized.
+        // They no longer work as labels or directive prologues,
+        // but are still recognized in other contexts.
+        return Expression(code_ | kParenthesizedExpressionFlag);
+      }
+      // For other types of expressions, it's not important to remember
+      // the parentheses.
+      return *this;
+    }
+
    private:
     // First two/three bits are used as flags.
     // Bit 0 and 1 represent identifiers or strings literals, and are
     // mutually exclusive, but can both be absent.
+    // If bit 0 or 1 are set, bit 2 marks that the expression has
+    // been wrapped in parentheses (a string literal can no longer
+    // be a directive prologue, and an identifier can no longer be
+    // a label.
     enum  {
       kUnknownExpression = 0,
       // Identifiers
@@ -420,6 +452,9 @@ class PreParser : public ParserBase {
       kUseStrictString = kStringLiteralFlag | 8,
       kStringLiteralMask = kUseStrictString,
 
+      // Only if identifier or string literal.
+      kParenthesizedExpressionFlag = 4,
+
       // Below here applies if neither identifier nor string literal.
       kThisExpression = 4,
       kThisPropertyExpression = 8,
@@ -445,11 +480,13 @@ class PreParser : public ParserBase {
     // Preserves being an unparenthesized string literal, possibly
     // "use strict".
     static Statement ExpressionStatement(Expression expression) {
-      if (expression.IsUseStrictLiteral()) {
-        return Statement(kUseStrictExpressionStatement);
-      }
-      if (expression.IsStringLiteral()) {
-        return Statement(kStringLiteralExpressionStatement);
+      if (!expression.IsParenthesized()) {
+        if (expression.IsUseStrictLiteral()) {
+          return Statement(kUseStrictExpressionStatement);
+        }
+        if (expression.IsStringLiteral()) {
+          return Statement(kStringLiteralExpressionStatement);
+        }
       }
       return Default();
     }

test/cctest/test-parsing.cc

@@ -1798,36 +1798,3 @@ TEST(NoErrorsIllegalWordsAsLabels) {
 
   RunParserSyncTest(context_data, statement_data, kSuccess);
 }
-
-
-TEST(ErrorsParenthesizedLabels) {
-  // Parenthesized identifiers shouldn't be recognized as labels.
-  const char* context_data[][2] = {
-    { "", ""},
-    { "function test_func() {", "}" },
-    { NULL, NULL }
-  };
-
-  const char* statement_data[] = {
-    "(mylabel): while(true) { break mylabel; }",
-    NULL
-  };
-
-  RunParserSyncTest(context_data, statement_data, kError);
-}
-
-
-TEST(NoErrorsParenthesizedDirectivePrologue) {
-  // Parenthesized directive prologue shouldn't be recognized.
-  const char* context_data[][2] = {
-    { "", ""},
-    { NULL, NULL }
-  };
-
-  const char* statement_data[] = {
-    "(\"use strict\"); var eval;",
-    NULL
-  };
-
-  RunParserSyncTest(context_data, statement_data, kSuccess);
-}