Implement ES2015 labelled function declaration restrictions

ES#sec-islabelledfunction specifies that labelled function declarations
may not occur as the body of a control flow construct such as an if
statement. This patch implements those restrictions, which also
eliminates a previous case resulting in a DCHECK failure which is now
a SyntaxError.

BUG=chromium:595309
R=adamk
LOG=Y

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

Cr-Commit-Position: refs/heads/master@{#35049}

src/messages.h

@@ -398,6 +398,9 @@ class CallSite {
   T(JsonParseUnexpectedTokenNumber, "Unexpected number in JSON at position %") \
   T(JsonParseUnexpectedTokenString, "Unexpected string in JSON at position %") \
   T(LabelRedeclaration, "Label '%' has already been declared")                 \
+  T(LabelledFunctionDeclaration,                                               \
+    "Labelled function declaration not allowed as the body of a control flow " \
+    "structure")                                                               \
   T(MalformedArrowFunParamList, "Malformed arrow function parameter list")     \
   T(MalformedRegExp, "Invalid regular expression: /%/: %")                     \
   T(MalformedRegExpFlags, "Invalid regular expression flags")                  \

src/parsing/parser-base.h

@@ -24,6 +24,10 @@ enum FunctionNameValidity {
   kFunctionNameValidityUnknown
 };
 
+enum AllowLabelledFunctionStatement {
+  kAllowLabelledFunctionStatement,
+  kDisallowLabelledFunctionStatement,
+};
 
 struct FormalParametersBase {
   explicit FormalParametersBase(Scope* scope) : scope(scope) {}

src/parsing/parser.cc

@@ -1271,7 +1271,7 @@ Statement* Parser::ParseStatementListItem(bool* ok) {
     default:
       break;
   }
-  return ParseStatement(NULL, ok);
+  return ParseStatement(NULL, kAllowLabelledFunctionStatement, ok);
 }
 
 
@@ -1721,8 +1721,8 @@ Statement* Parser::ParseExportDeclaration(bool* ok) {
   return result;
 }
 
-
 Statement* Parser::ParseStatement(ZoneList<const AstRawString*>* labels,
+                                  AllowLabelledFunctionStatement allow_function,
                                   bool* ok) {
   // Statement ::
   //   EmptyStatement
@@ -1732,12 +1732,12 @@ Statement* Parser::ParseStatement(ZoneList<const AstRawString*>* labels,
     Next();
     return factory()->NewEmptyStatement(RelocInfo::kNoPosition);
   }
-  return ParseSubStatement(labels, ok);
+  return ParseSubStatement(labels, allow_function, ok);
 }
 
-
-Statement* Parser::ParseSubStatement(ZoneList<const AstRawString*>* labels,
-                                     bool* ok) {
+Statement* Parser::ParseSubStatement(
+    ZoneList<const AstRawString*>* labels,
+    AllowLabelledFunctionStatement allow_function, bool* ok) {
   // Statement ::
   //   Block
   //   VariableStatement
@@ -1826,7 +1826,7 @@ Statement* Parser::ParseSubStatement(ZoneList<const AstRawString*>* labels,
       return ParseVariableStatement(kStatement, NULL, ok);
 
     default:
-      return ParseExpressionOrLabelledStatement(labels, ok);
+      return ParseExpressionOrLabelledStatement(labels, allow_function, ok);
   }
 }
 
@@ -2415,9 +2415,9 @@ static bool ContainsLabel(ZoneList<const AstRawString*>* labels,
   return false;
 }
 
-
 Statement* Parser::ParseExpressionOrLabelledStatement(
-    ZoneList<const AstRawString*>* labels, bool* ok) {
+    ZoneList<const AstRawString*>* labels,
+    AllowLabelledFunctionStatement allow_function, bool* ok) {
   // ExpressionStatement | LabelledStatement ::
   //   Expression ';'
   //   Identifier ':' Statement
@@ -2470,9 +2470,13 @@ Statement* Parser::ParseExpressionOrLabelledStatement(
     Expect(Token::COLON, CHECK_OK);
     // ES#sec-labelled-function-declarations Labelled Function Declarations
     if (peek() == Token::FUNCTION && is_sloppy(language_mode())) {
-      return ParseFunctionDeclaration(labels, ok);
+      if (allow_function == kAllowLabelledFunctionStatement) {
+        return ParseFunctionDeclaration(labels, ok);
+      } else {
+        return ParseScopedStatement(labels, true, ok);
+      }
     }
-    return ParseStatement(labels, ok);
+    return ParseStatement(labels, kDisallowLabelledFunctionStatement, ok);
   }
 
   // If we have an extension, we allow a native function declaration.
@@ -3460,7 +3464,7 @@ Statement* Parser::ParseScopedStatement(ZoneList<const AstRawString*>* labels,
                                         bool legacy, bool* ok) {
   if (is_strict(language_mode()) || peek() != Token::FUNCTION ||
       (legacy && allow_harmony_restrictive_declarations())) {
-    return ParseSubStatement(labels, ok);
+    return ParseSubStatement(labels, kDisallowLabelledFunctionStatement, ok);
   } else {
     if (legacy) {
       ++use_counts_[v8::Isolate::kLegacyFunctionDeclaration];

src/parsing/parser.h

@@ -757,8 +757,12 @@ class Parser : public ParserBase<ParserTraits> {
                           ZoneList<const AstRawString*>* local_names,
                           Scanner::Location* reserved_loc, bool* ok);
   ZoneList<ImportDeclaration*>* ParseNamedImports(int pos, bool* ok);
-  Statement* ParseStatement(ZoneList<const AstRawString*>* labels, bool* ok);
-  Statement* ParseSubStatement(ZoneList<const AstRawString*>* labels, bool* ok);
+  Statement* ParseStatement(ZoneList<const AstRawString*>* labels,
+                            AllowLabelledFunctionStatement allow_function,
+                            bool* ok);
+  Statement* ParseSubStatement(ZoneList<const AstRawString*>* labels,
+                               AllowLabelledFunctionStatement allow_function,
+                               bool* ok);
   Statement* ParseStatementAsUnlabelled(ZoneList<const AstRawString*>* labels,
                                    bool* ok);
   Statement* ParseFunctionDeclaration(ZoneList<const AstRawString*>* names,
@@ -900,7 +904,8 @@ class Parser : public ParserBase<ParserTraits> {
                                    ZoneList<const AstRawString*>* names,
                                    bool* ok);
   Statement* ParseExpressionOrLabelledStatement(
-      ZoneList<const AstRawString*>* labels, bool* ok);
+      ZoneList<const AstRawString*>* labels,
+      AllowLabelledFunctionStatement allow_function, bool* ok);
   IfStatement* ParseIfStatement(ZoneList<const AstRawString*>* labels,
                                 bool* ok);
   Statement* ParseContinueStatement(bool* ok);

src/parsing/preparser.cc

@@ -194,7 +194,7 @@ PreParser::Statement PreParser::ParseStatementListItem(bool* ok) {
     default:
       break;
   }
-  return ParseStatement(ok);
+  return ParseStatement(kAllowLabelledFunctionStatement, ok);
 }
 
 
@@ -263,8 +263,8 @@ void PreParser::ParseStatementList(int end_token, bool* ok,
 #define DUMMY )  // to make indentation work
 #undef DUMMY
 
-
-PreParser::Statement PreParser::ParseStatement(bool* ok) {
+PreParser::Statement PreParser::ParseStatement(
+    AllowLabelledFunctionStatement allow_function, bool* ok) {
   // Statement ::
   //   EmptyStatement
   //   ...
@@ -273,19 +273,20 @@ PreParser::Statement PreParser::ParseStatement(bool* ok) {
     Next();
     return Statement::Default();
   }
-  return ParseSubStatement(ok);
+  return ParseSubStatement(allow_function, ok);
 }
 
 PreParser::Statement PreParser::ParseScopedStatement(bool legacy, bool* ok) {
   if (is_strict(language_mode()) || peek() != Token::FUNCTION ||
       (legacy && allow_harmony_restrictive_declarations())) {
-    return ParseSubStatement(ok);
+    return ParseSubStatement(kDisallowLabelledFunctionStatement, ok);
   } else {
     return ParseFunctionDeclaration(CHECK_OK);
   }
 }
 
-PreParser::Statement PreParser::ParseSubStatement(bool* ok) {
+PreParser::Statement PreParser::ParseSubStatement(
+    AllowLabelledFunctionStatement allow_function, bool* ok) {
   // Statement ::
   //   Block
   //   VariableStatement
@@ -372,7 +373,7 @@ PreParser::Statement PreParser::ParseSubStatement(bool* ok) {
       return ParseVariableStatement(kStatement, ok);
 
     default:
-      return ParseExpressionOrLabelledStatement(ok);
+      return ParseExpressionOrLabelledStatement(allow_function, ok);
   }
 }
 
@@ -555,8 +556,8 @@ PreParser::Statement PreParser::ParseVariableDeclarations(
   return Statement::Default();
 }
 
-
-PreParser::Statement PreParser::ParseExpressionOrLabelledStatement(bool* ok) {
+PreParser::Statement PreParser::ParseExpressionOrLabelledStatement(
+    AllowLabelledFunctionStatement allow_function, bool* ok) {
   // ExpressionStatement | LabelledStatement ::
   //   Expression ';'
   //   Identifier ':' Statement
@@ -591,9 +592,14 @@ PreParser::Statement PreParser::ParseExpressionOrLabelledStatement(bool* ok) {
     Consume(Token::COLON);
     // ES#sec-labelled-function-declarations Labelled Function Declarations
     if (peek() == Token::FUNCTION && is_sloppy(language_mode())) {
-      return ParseFunctionDeclaration(ok);
+      if (allow_function == kAllowLabelledFunctionStatement) {
+        return ParseFunctionDeclaration(ok);
+      } else {
+        return ParseScopedStatement(true, ok);
+      }
     }
-    Statement statement = ParseStatement(ok);
+    Statement statement =
+        ParseStatement(kDisallowLabelledFunctionStatement, ok);
     return statement.IsJumpStatement() ? Statement::Default() : statement;
     // Preparsing is disabled for extensions (because the extension details
     // aren't passed to lazily compiled functions), so we don't

src/parsing/preparser.h

@@ -1019,8 +1019,10 @@ class PreParser : public ParserBase<PreParserTraits> {
   Statement ParseStatementListItem(bool* ok);
   void ParseStatementList(int end_token, bool* ok,
                           Scanner::BookmarkScope* bookmark = nullptr);
-  Statement ParseStatement(bool* ok);
-  Statement ParseSubStatement(bool* ok);
+  Statement ParseStatement(AllowLabelledFunctionStatement allow_function,
+                           bool* ok);
+  Statement ParseSubStatement(AllowLabelledFunctionStatement allow_function,
+                              bool* ok);
   Statement ParseScopedStatement(bool legacy, bool* ok);
   Statement ParseFunctionDeclaration(bool* ok);
   Statement ParseClassDeclaration(bool* ok);
@@ -1033,7 +1035,8 @@ class PreParser : public ParserBase<PreParserTraits> {
                                       Scanner::Location* first_initializer_loc,
                                       Scanner::Location* bindings_loc,
                                       bool* ok);
-  Statement ParseExpressionOrLabelledStatement(bool* ok);
+  Statement ParseExpressionOrLabelledStatement(
+      AllowLabelledFunctionStatement allow_function, bool* ok);
   Statement ParseIfStatement(bool* ok);
   Statement ParseContinueStatement(bool* ok);
   Statement ParseBreakStatement(bool* ok);

test/cctest/test-parsing.cc

@@ -7217,10 +7217,13 @@ TEST(FunctionDeclarationError) {
     {"(function() { {", "} })()"},
     { NULL, NULL }
   };
+  // Invalid in all contexts
   const char* error_data[] = {
     "try function foo() {} catch (e) {}",
     NULL
   };
+  // Valid in sloppy mode only, and only when the
+  // --harmony-restrictive-declarations flag is off
   const char* unrestricted_data[] = {
     "do function foo() {} while (0);",
     "for (;false;) function foo() {}",
@@ -7232,16 +7235,28 @@ TEST(FunctionDeclarationError) {
     "for (x in {}) function f() { };",
     "var x; for (x in {}) function foo() {}",
     "with ({}) function f() { };",
+    "do label: function foo() {} while (0);",
+    "for (;false;) label: function foo() {}",
+    "for (var i = 0; i < 1; i++) label: function f() { };",
+    "for (var x in {a: 1}) label: function f() { };",
+    "for (var x in {}) label: function f() { };",
+    "for (var x in {}) label: function foo() {}",
+    "for (x in {a: 1}) label: function f() { };",
+    "for (x in {}) label: function f() { };",
+    "var x; for (x in {}) label: function foo() {}",
+    "with ({}) label: function f() { };",
+    "if (true) label: function f() {}",
+    "if (true) {} else label: function f() {}",
     NULL
   };
+  // Valid only in sloppy mode, with or without
+  // --harmony-restrictive-declarations
   const char* sloppy_data[] = {
     "if (true) function foo() {}",
     "if (false) {} else function f() { };",
     "label: function f() { }",
     "label: if (true) function f() { }",
     "label: if (true) {} else function f() { }",
-    "if (true) label: function f() {}",
-    "if (true) {} else label: function f() {}",
     NULL
   };
   // clang-format on
@@ -7249,6 +7264,7 @@ TEST(FunctionDeclarationError) {
   static const ParserFlag restrictive_flags[] = {
       kAllowHarmonyRestrictiveDeclarations};
 
+  // Nothing parses in strict mode without a SyntaxError
   RunParserSyncTest(strict_context, error_data, kError);
   RunParserSyncTest(strict_context, error_data, kError, NULL, 0,
                     restrictive_flags, arraysize(restrictive_flags));
@@ -7259,6 +7275,7 @@ TEST(FunctionDeclarationError) {
   RunParserSyncTest(strict_context, sloppy_data, kError, NULL, 0,
                     restrictive_flags, arraysize(restrictive_flags));
 
+  // In sloppy mode, some things are successful, depending on the flag
   RunParserSyncTest(sloppy_context, error_data, kError);
   RunParserSyncTest(sloppy_context, error_data, kError, NULL, 0,
                     restrictive_flags, arraysize(restrictive_flags));