Unify paren handling in Parser and PreParser.

It is only needed in (Pre)Parser::ParseExpressionOrLabelledStatement for not
recognizing parenthesized identifiers as labels and in
(Pre)Parser::ParseSourceElements for not recognizing a parenthesized string as
directive prologue.

Parser Expressions don't keep track of whether they're parenthesized, so
PreParser Expressions shouldn't either.

In addition, add helper funcs for checking if an Expression is identifier or a
given identifier. (PreParser Expressions can do this.)

BUG=3126
LOG=N
R=ulan@chromium.org

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

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

src/ast.cc

@@ -82,6 +82,16 @@ 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,6 +351,10 @@ 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,10 +964,8 @@ Statement* Parser::ParseModuleElement(ZoneStringList* labels,
           !scanner().HasAnyLineTerminatorBeforeNext() &&
           stmt != NULL) {
         ExpressionStatement* estmt = stmt->AsExpressionStatement();
-        if (estmt != NULL &&
-            estmt->expression()->AsVariableProxy() != NULL &&
-            estmt->expression()->AsVariableProxy()->name()->Equals(
-                isolate()->heap()->module_string()) &&
+        if (estmt != NULL && estmt->expression()->IsIdentifierNamed(
+                                 isolate()->heap()->module_string()) &&
             !scanner().literal_contains_escapes()) {
           return ParseModuleDeclaration(NULL, ok);
         }
@@ -2126,9 +2124,11 @@ 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->AsVariableProxy() != NULL &&
-      !expr->AsVariableProxy()->is_this()) {
+      expr->IsIdentifier()) {
     // Expression is a single identifier, and not, e.g., a parenthesized
     // identifier.
     VariableProxy* var = expr->AsVariableProxy();
@@ -2165,9 +2165,7 @@ Statement* Parser::ParseExpressionOrLabelledStatement(ZoneStringList* labels,
       peek() == Token::FUNCTION &&
       !scanner().HasAnyLineTerminatorBeforeNext() &&
       expr != NULL &&
-      expr->AsVariableProxy() != NULL &&
-      expr->AsVariableProxy()->name()->Equals(
-          isolate()->heap()->native_string()) &&
+      expr->IsIdentifierNamed(isolate()->heap()->native_string()) &&
       !scanner().literal_contains_escapes()) {
     return ParseNativeDeclaration(ok);
   }
@@ -2177,9 +2175,7 @@ Statement* Parser::ParseExpressionOrLabelledStatement(ZoneStringList* labels,
   if (!FLAG_harmony_modules ||
       peek() != Token::IDENTIFIER ||
       scanner().HasAnyLineTerminatorBeforeNext() ||
-      expr->AsVariableProxy() == NULL ||
-      !expr->AsVariableProxy()->name()->Equals(
-          isolate()->heap()->module_string()) ||
+      !expr->IsIdentifierNamed(isolate()->heap()->module_string()) ||
       scanner().literal_contains_escapes()) {
     ExpectSemicolon(CHECK_OK);
   }
@@ -2946,8 +2942,7 @@ Expression* Parser::ParseAssignmentExpression(bool accept_IN, bool* ok) {
   Property* property = expression ? expression->AsProperty() : NULL;
   if (op == Token::ASSIGN &&
       property != NULL &&
-      property->obj()->AsVariableProxy() != NULL &&
-      property->obj()->AsVariableProxy()->is_this()) {
+      property->obj()->IsIdentifier()) {
     current_function_state_->AddProperty();
   }
 

src/preparser.cc

@@ -168,15 +168,18 @@ PreParser::SourceElements PreParser::ParseSourceElements(int end_token,
   // SourceElements ::
   //   (Statement)* <end_token>
 
-  bool allow_directive_prologue = true;
+  bool directive_prologue = true;
   while (peek() != end_token) {
+    if (directive_prologue && peek() != Token::STRING) {
+      directive_prologue = false;
+    }
     Statement statement = ParseSourceElement(CHECK_OK);
-    if (allow_directive_prologue) {
+    if (directive_prologue) {
       if (statement.IsUseStrictLiteral()) {
         set_language_mode(allow_harmony_scoping() ?
                           EXTENDED_MODE : STRICT_MODE);
       } else if (!statement.IsStringLiteral()) {
-        allow_directive_prologue = false;
+        directive_prologue = false;
       }
     }
   }
@@ -468,16 +471,20 @@ PreParser::Statement PreParser::ParseExpressionOrLabelledStatement(bool* ok) {
   //   Expression ';'
   //   Identifier ':' Statement
 
+  bool starts_with_identifier = peek_any_identifier();
   Expression expr = ParseExpression(true, CHECK_OK);
-  if (expr.IsRawIdentifier()) {
+  // 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.
     ASSERT(!expr.AsIdentifier().IsFutureReserved());
     ASSERT(is_classic_mode() ||
            (!expr.AsIdentifier().IsFutureStrictReserved() &&
             !expr.AsIdentifier().IsYield()));
-    if (peek() == Token::COLON) {
-      Consume(Token::COLON);
-      return ParseStatement(ok);
-    }
+    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.
@@ -1170,7 +1177,6 @@ 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,8 +347,6 @@ 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() {
@@ -389,21 +387,8 @@ 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;
     }
@@ -420,27 +405,10 @@ 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
@@ -452,9 +420,6 @@ 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,
@@ -480,13 +445,11 @@ class PreParser : public ParserBase {
     // Preserves being an unparenthesized string literal, possibly
     // "use strict".
     static Statement ExpressionStatement(Expression expression) {
-      if (!expression.IsParenthesized()) {
-        if (expression.IsUseStrictLiteral()) {
-          return Statement(kUseStrictExpressionStatement);
-        }
-        if (expression.IsStringLiteral()) {
-          return Statement(kStringLiteralExpressionStatement);
-        }
+      if (expression.IsUseStrictLiteral()) {
+        return Statement(kUseStrictExpressionStatement);
+      }
+      if (expression.IsStringLiteral()) {
+        return Statement(kStringLiteralExpressionStatement);
       }
       return Default();
     }

test/cctest/test-parsing.cc

@@ -1798,3 +1798,36 @@ 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);
+}