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

diff --git a/src/ast.cc b/src/ast.cc
index 1a9919b..1756257 100644 (file)
--- a/src/ast.cc
+++ b/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()),

diff --git a/src/ast.h b/src/ast.h
index 2b33820..2b04599 100644 (file)
--- a/src/ast.h
+++ b/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; }

diff --git a/src/parser.cc b/src/parser.cc
index 211f737..ae2dd5f 100644 (file)
--- a/src/parser.cc
+++ b/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();
   }
 

diff --git a/src/preparser.cc b/src/preparser.cc
index 4150d33..becf914 100644 (file)
--- a/src/preparser.cc
+++ b/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:

diff --git a/src/preparser.h b/src/preparser.h
index d0c7e3e..3a68bed 100644 (file)
--- a/src/preparser.h
+++ b/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();
     }

diff --git a/test/cctest/test-parsing.cc b/test/cctest/test-parsing.cc
index 0106e59..9768e30 100644 (file)
--- a/test/cctest/test-parsing.cc
+++ b/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);
+}