Restrict FunctionDeclarations in Statement position

src/parsing/parser.cc

Index: src/parsing/parser.cc
diff --git a/src/parsing/parser.cc b/src/parsing/parser.cc
index 36c966110b2f432202993592a552d3c46bc3e0fb..97d1642512d906f6af45371be7aebdd3d696b8fc 100644
--- a/src/parsing/parser.cc
+++ b/src/parsing/parser.cc
@@ -774,6 +774,8 @@ Parser::Parser(ParseInfo* info)
   set_allow_harmony_do_expressions(FLAG_harmony_do_expressions);
   set_allow_harmony_function_name(FLAG_harmony_function_name);
   set_allow_harmony_function_sent(FLAG_harmony_function_sent);
+  set_allow_harmony_restrictive_declarations(
+      FLAG_harmony_restrictive_declarations);
   for (int feature = 0; feature < v8::Isolate::kUseCounterFeatureCount;
        ++feature) {
     use_counts_[feature] = 0;
@@ -1840,24 +1842,17 @@ Statement* Parser::ParseSubStatement(ZoneList<const AstRawString*>* labels,
       return ParseSwitchStatement(labels, ok);
 
     case Token::FUNCTION: {
-      // FunctionDeclaration is only allowed in the context of SourceElements
-      // (Ecma 262 5th Edition, clause 14):
-      // SourceElement:
-      //    Statement
-      //    FunctionDeclaration
-      // Common language extension is to allow function declaration in place
-      // of any statement. This language extension is disabled in strict mode.
-      //
-      // In Harmony mode, this case also handles the extension:
-      // Statement:
-      //    GeneratorDeclaration
-      if (is_strict(language_mode())) {
-        ReportMessageAt(scanner()->peek_location(),
-                        MessageTemplate::kStrictFunction);
-        *ok = false;
-        return NULL;
-      }
-      return ParseFunctionDeclaration(NULL, ok);
+      // FunctionDeclaration only allowed as a StatementListItem, not in
+      // an arbitrary Statement position. Exceptions such as
+      // ES#sec-functiondeclarations-in-ifstatement-statement-clauses
+      // are handled by calling ParseScopedStatement rather than
+      // ParseSubStatement directly.
+      ReportMessageAt(scanner()->peek_location(),
+                      is_strict(language_mode()) ?
+                          MessageTemplate::kStrictFunction :
+                          MessageTemplate::kSloppyFunction);
+      *ok = false;
+      return nullptr;
     }
 
     case Token::DEBUGGER:
@@ -2581,6 +2576,10 @@ Statement* Parser::ParseExpressionOrLabelledStatement(
     // during the scope processing.
     scope_->RemoveUnresolved(var);
     Expect(Token::COLON, CHECK_OK);
+    // ES#sec-labelled-function-declarations Labelled Function Declarations

[adamk, 2016/03/02 00:51:10]

Can you add tests feature labels? It seems we currently parse them...

[Dan Ehrenberg, 2016/03/03 02:32:57]

Done

+    if (peek() == Token::FUNCTION && is_sloppy(language_mode())) {
+      return ParseFunctionDeclaration(labels, ok);
+    }
     return ParseStatement(labels, ok);
   }
 
@@ -2621,11 +2620,11 @@ IfStatement* Parser::ParseIfStatement(ZoneList<const AstRawString*>* labels,
   Expect(Token::LPAREN, CHECK_OK);
   Expression* condition = ParseExpression(true, CHECK_OK);
   Expect(Token::RPAREN, CHECK_OK);
-  Statement* then_statement = ParseSubStatement(labels, CHECK_OK);
+  Statement* then_statement = ParseScopedStatement(labels, false, CHECK_OK);
   Statement* else_statement = NULL;
   if (peek() == Token::ELSE) {
     Next();
-    else_statement = ParseSubStatement(labels, CHECK_OK);
+    else_statement = ParseScopedStatement(labels, false, CHECK_OK);
   } else {
     else_statement = factory()->NewEmptyStatement(RelocInfo::kNoPosition);
   }
@@ -2824,25 +2823,10 @@ Statement* Parser::ParseWithStatement(ZoneList<const AstRawString*>* labels,
 
   scope_->DeclarationScope()->RecordWithStatement();
   Scope* with_scope = NewScope(scope_, WITH_SCOPE);
-  Block* body;
+  Statement* body;
   { BlockState block_state(&scope_, with_scope);
     with_scope->set_start_position(scanner()->peek_location().beg_pos);
-
-    // The body of the with statement must be enclosed in an additional
-    // lexical scope in case the body is a FunctionDeclaration.
-    body = factory()->NewBlock(labels, 1, false, RelocInfo::kNoPosition);
-    Scope* block_scope = NewScope(scope_, BLOCK_SCOPE);
-    block_scope->set_start_position(scanner()->location().beg_pos);
-    {
-      BlockState block_state(&scope_, block_scope);
-      Target target(&this->target_stack_, body);
-      Statement* stmt = ParseSubStatement(labels, CHECK_OK);
-      body->statements()->Add(stmt, zone());
-      block_scope->set_end_position(scanner()->location().end_pos);
-      block_scope = block_scope->FinalizeBlockScope();
-      body->set_scope(block_scope);
-    }
-
+    body = ParseScopedStatement(labels, true, CHECK_OK);
     with_scope->set_end_position(scanner()->location().end_pos);
   }
   return factory()->NewWithStatement(with_scope, expr, body, pos);
@@ -3183,7 +3167,7 @@ DoWhileStatement* Parser::ParseDoWhileStatement(
   Target target(&this->target_stack_, loop);
 
   Expect(Token::DO, CHECK_OK);
-  Statement* body = ParseSubStatement(NULL, CHECK_OK);
+  Statement* body = ParseScopedStatement(NULL, true, CHECK_OK);
   Expect(Token::WHILE, CHECK_OK);
   Expect(Token::LPAREN, CHECK_OK);
 
@@ -3213,7 +3197,7 @@ WhileStatement* Parser::ParseWhileStatement(
   Expect(Token::LPAREN, CHECK_OK);
   Expression* cond = ParseExpression(true, CHECK_OK);
   Expect(Token::RPAREN, CHECK_OK);
-  Statement* body = ParseSubStatement(NULL, CHECK_OK);
+  Statement* body = ParseScopedStatement(NULL, true, CHECK_OK);
 
   if (loop != NULL) loop->Initialize(cond, body);
   return loop;
@@ -3596,6 +3580,31 @@ Statement* Parser::DesugarLexicalBindingsInForStatement(
 }
 
 
+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);
+  } else {
+    if (legacy) {
+      ++use_counts_[v8::Isolate::kLegacyFunctionDeclaration];
+    }
+    // Make a block around the statement for a lexical binding
+    // is introduced by a FunctionDeclaration.
+    Scope* body_scope = NewScope(scope_, BLOCK_SCOPE);
+    BlockState block_state(&scope_, body_scope);
+    Block* block =
+        factory()->NewBlock(NULL, 1, false, RelocInfo::kNoPosition);
+    Statement* body = ParseFunctionDeclaration(NULL, CHECK_OK);
+    block->statements()->Add(body, zone());
+    body_scope->set_end_position(scanner()->location().end_pos);
+    body_scope = body_scope->FinalizeBlockScope();
+    block->set_scope(body_scope);
+    return block;
+  }
+}
+
+
 Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
                                      bool* ok) {
   int stmt_pos = peek_position();
@@ -3708,7 +3717,7 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
         {
           BlockState block_state(&scope_, body_scope);
 
-          Statement* body = ParseSubStatement(NULL, CHECK_OK);
+          Statement* body = ParseScopedStatement(NULL, true, CHECK_OK);
 
           auto each_initialization_block =
               factory()->NewBlock(nullptr, 1, true, RelocInfo::kNoPosition);
@@ -3825,25 +3834,11 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
 
         Expect(Token::RPAREN, CHECK_OK);
 
-        // Make a block around the statement in case a lexical binding
-        // is introduced, e.g. by a FunctionDeclaration.
-        // This block must not use for_scope as its scope because if a
-        // lexical binding is introduced which overlaps with the for-in/of,
-        // expressions in head of the loop should actually have variables
-        // resolved in the outer scope.
-        Scope* body_scope = NewScope(for_scope, BLOCK_SCOPE);
-        {
-          BlockState block_state(&scope_, body_scope);
-          Block* block =
-              factory()->NewBlock(NULL, 1, false, RelocInfo::kNoPosition);
-          Statement* body = ParseSubStatement(NULL, CHECK_OK);
-          block->statements()->Add(body, zone());
-          InitializeForEachStatement(loop, expression, enumerable, block,
-                                     is_destructuring);
-          body_scope->set_end_position(scanner()->location().end_pos);
-          body_scope = body_scope->FinalizeBlockScope();
-          block->set_scope(body_scope);
-        }
+        // For legacy compat reasons, give for loops similar treatment to
+        // if statements in allowing a function declaration for a body
+        Statement* body = ParseScopedStatement(NULL, true, CHECK_OK);
+        InitializeForEachStatement(loop, expression, enumerable, body,
+                                   is_destructuring);
 
         Statement* final_loop = loop->IsForOfStatement()
             ? FinalizeForOfStatement(
@@ -3900,7 +3895,7 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
     }
     Expect(Token::RPAREN, CHECK_OK);
 
-    body = ParseSubStatement(NULL, CHECK_OK);
+    body = ParseScopedStatement(NULL, true, CHECK_OK);
   }
 
   Statement* result = NULL;