[parser] Refactor of Parse*Statement*, part 8

src/parsing/parser-base.h

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef V8_PARSING_PARSER_BASE_H
 #define V8_PARSING_PARSER_BASE_H
 
 #include "src/ast/ast.h"
 #include "src/ast/scopes.h"
 #include "src/bailout-reason.h"
@@ skipping 638 matching lines @@
     Variable* variable;
     ExpressionT pattern;
     Scope* scope;
     BlockT init_block;
     BlockT inner_block;
     bool for_promise_reject;
     ZoneList<const AstRawString*> bound_names;
     TailCallExpressionList tail_call_expressions;
   };
 
+  struct ForInfo {
+   public:
+    explicit ForInfo(ParserBase* parser)
+        : bound_names(1, parser->zone()),
+          bound_names_are_lexical(false),
+          mode(ForEachStatement::ENUMERATE),
+          each_loc(),
+          parsing_result() {}
+    ZoneList<const AstRawString*> bound_names;
+    bool bound_names_are_lexical;
+    ForEachStatement::VisitMode mode;
+    Scanner::Location each_loc;
+    DeclarationParsingResult parsing_result;
+  };
+
   DeclarationScope* NewScriptScope() const {
     return new (zone()) DeclarationScope(zone(), ast_value_factory());
   }
 
   DeclarationScope* NewVarblockScope() const {
     return new (zone()) DeclarationScope(zone(), scope(), BLOCK_SCOPE);
   }
 
   ModuleScope* NewModuleScope(DeclarationScope* parent) const {
     return new (zone()) ModuleScope(parent, ast_value_factory());
@@ skipping 574 matching lines @@
   StatementT ParseWithStatement(ZoneList<const AstRawString*>* labels,
                                 bool* ok);
   StatementT ParseDoWhileStatement(ZoneList<const AstRawString*>* labels,
                                    bool* ok);
   StatementT ParseWhileStatement(ZoneList<const AstRawString*>* labels,
                                  bool* ok);
   StatementT ParseThrowStatement(bool* ok);
   StatementT ParseSwitchStatement(ZoneList<const AstRawString*>* labels,
                                   bool* ok);
   StatementT ParseTryStatement(bool* ok);
-  Statement* ParseForStatement(ZoneList<const AstRawString*>* labels, bool* ok);
+  StatementT ParseForStatement(ZoneList<const AstRawString*>* labels, bool* ok);
 
   bool IsNextLetKeyword();
   bool IsTrivialExpression();
 
   // Checks if the expression is a valid reference expression (e.g., on the
   // left-hand side of assignments). Although ruled out by ECMA as early errors,
   // we allow calls for web compatibility and rewrite them to a runtime throw.
   ExpressionT CheckAndRewriteReferenceExpression(
       ExpressionT expression, int beg_pos, int end_pos,
       MessageTemplate::Template message, bool* ok);
@@ skipping 3079 matching lines @@
     case Token::SEMICOLON:
       Next();
       return factory()->NewEmptyStatement(kNoSourcePosition);
     case Token::IF:
       return ParseIfStatement(labels, ok);
     case Token::DO:
       return ParseDoWhileStatement(labels, ok);
     case Token::WHILE:
       return ParseWhileStatement(labels, ok);
     case Token::FOR:
-      return impl()->ParseForStatement(labels, ok);
+      return ParseForStatement(labels, ok);
     case Token::CONTINUE:
     case Token::BREAK:
     case Token::RETURN:
     case Token::THROW:
     case Token::TRY: {
       // These statements must have their labels preserved in an enclosing
       // block, as the corresponding AST nodes do not currently store their
       // labels.
       // TODO(nikolaos, marja): Consider adding the labels to the AST nodes.
       if (labels == nullptr) {
@@ skipping 606 matching lines @@
                               MessageTemplate::kUnexpectedTailCallInCatchBlock);
       *ok = false;
       return impl()->NullStatement();
     }
   }
 
   return impl()->RewriteTryStatement(try_block, catch_block, finally_block,
                                      catch_info, pos);
 }
 
-Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
-                                     bool* ok) {
+template <typename Impl>
+typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseForStatement(
+    ZoneList<const AstRawString*>* labels, bool* ok) {
   int stmt_pos = peek_position();
-  Statement* init = NULL;
-  ZoneList<const AstRawString*> bound_names(1, zone());
-  bool bound_names_are_lexical = false;
+  ForInfo for_info(this);
 
   // Create an in-between scope for let-bound iteration variables.
   BlockState for_state(&scope_state_);
   Expect(Token::FOR, CHECK_OK);
   Expect(Token::LPAREN, CHECK_OK);
   for_state.set_start_position(scanner()->location().beg_pos);
   for_state.set_is_hidden();
-  DeclarationParsingResult parsing_result;
+
+  StatementT init = impl()->NullStatement();
   if (peek() != Token::SEMICOLON) {
+    // An initializer is present.
     if (peek() == Token::VAR || peek() == Token::CONST ||
         (peek() == Token::LET && IsNextLetKeyword())) {
-      ParseVariableDeclarations(kForStatement, &parsing_result, nullptr,
-                                CHECK_OK);
+      // The initializer contains declarations.
+      ParseVariableDeclarations(kForStatement, &for_info.parsing_result,
+                                nullptr, CHECK_OK);
+      for_info.bound_names_are_lexical =

[marja, 2016/09/20 19:36:30]

bound_names_are_lexical is now redundant, right? Maybe we shouldn't have it (as
the information is available in for_info.parsing_result), so that there's no
fear that for_info.bound_names_are_lexical and
for_info.parsing_result.descriptor.mode disagree.

If it makes the code clearer, there can be a bound_names_are_lexical helper
variable wherever it's needed (this is quite far away from the use..)

[nickie, 2016/09/21 09:20:02]

Yes.  I'm removing it from ForInfo but I'm keeping it as a local variable here,
to avoid unnecessary calls to IsLexicalVariableMode.

+          IsLexicalVariableMode(for_info.parsing_result.descriptor.mode);
+      for_info.each_loc = scanner()->location();
 
-      ForEachStatement::VisitMode mode = ForEachStatement::ENUMERATE;
-      int each_beg_pos = scanner()->location().beg_pos;
-      int each_end_pos = scanner()->location().end_pos;
-
-      if (CheckInOrOf(&mode)) {
-        if (parsing_result.declarations.length() != 1) {
-          ReportMessageAt(parsing_result.bindings_loc,
-                          MessageTemplate::kForInOfLoopMultiBindings,
-                          ForEachStatement::VisitModeString(mode));
+      if (CheckInOrOf(&for_info.mode)) {
+        // Just one declaration followed by in/of.
+        if (for_info.parsing_result.declarations.length() != 1) {
+          impl()->ReportMessageAt(
+              for_info.parsing_result.bindings_loc,
+              MessageTemplate::kForInOfLoopMultiBindings,
+              ForEachStatement::VisitModeString(for_info.mode));
           *ok = false;
-          return nullptr;
+          return impl()->NullStatement();
         }
-        DeclarationParsingResult::Declaration& decl =
-            parsing_result.declarations[0];
-        if (parsing_result.first_initializer_loc.IsValid() &&
-            (is_strict(language_mode()) || mode == ForEachStatement::ITERATE ||
-             IsLexicalVariableMode(parsing_result.descriptor.mode) ||
-             !decl.pattern->IsVariableProxy() || allow_harmony_for_in())) {
+        if (for_info.parsing_result.first_initializer_loc.IsValid() &&
+            (is_strict(language_mode()) ||
+             for_info.mode == ForEachStatement::ITERATE ||

[marja, 2016/09/21 08:23:21]

Random whining unrelated to this CL: Why do we need to have ITERATE and
ENUMERATE, so that everybody reading the code needs to remember that ITERATE is
"for of" and ENUMERATE is "for in". I have hard time remembering which is which.
IMO it would be clearer if we used the terms "ForIn" and "ForOf". In addition,
bugs where somebody accidentally uses the wrong one would look more obvious (now
they're quite hard to spot).

For example, this code would read: for_info.mode == ForEachStatement.FOR_IN ||
... || allow_harmony_for_in(), so it's clear that they belong together.


If you agree, maybe a follow-up CL?

[nickie, 2016/09/21 09:20:02]

I agree.  Also, it seems that this is used only locally in the parser.  There
are 4+9 occurrences of ENUMERATE+ITERATE.

+             IsLexicalVariableMode(for_info.parsing_result.descriptor.mode) ||
+             !impl()->IsIdentifier(
+                 for_info.parsing_result.declarations[0].pattern) ||
+             allow_harmony_for_in())) {
           // Only increment the use count if we would have let this through
           // without the flag.
           if (allow_harmony_for_in()) {
-            ++use_counts_[v8::Isolate::kForInInitializer];
+            impl()->CountUsage(v8::Isolate::kForInInitializer);
           }
-          ReportMessageAt(parsing_result.first_initializer_loc,
-                          MessageTemplate::kForInOfLoopInitializer,
-                          ForEachStatement::VisitModeString(mode));
+          impl()->ReportMessageAt(
+              for_info.parsing_result.first_initializer_loc,
+              MessageTemplate::kForInOfLoopInitializer,
+              ForEachStatement::VisitModeString(for_info.mode));
           *ok = false;
-          return nullptr;
+          return impl()->NullStatement();
         }
 
-        Block* init_block = nullptr;
-        bound_names_are_lexical =
-            IsLexicalVariableMode(parsing_result.descriptor.mode);
+        BlockT init_block = impl()->RewriteForVarInLegacy(&for_info);
 
-        // special case for legacy for (var ... = ... in ...)
-        if (!bound_names_are_lexical && decl.pattern->IsVariableProxy() &&
-            decl.initializer != nullptr) {
-          DCHECK(!allow_harmony_for_in());
-          ++use_counts_[v8::Isolate::kForInInitializer];
-          const AstRawString* name =
-              decl.pattern->AsVariableProxy()->raw_name();
-          VariableProxy* single_var = NewUnresolved(name);
-          init_block = factory()->NewBlock(
-              nullptr, 2, true, parsing_result.descriptor.declaration_pos);
-          init_block->statements()->Add(
-              factory()->NewExpressionStatement(
-                  factory()->NewAssignment(Token::ASSIGN, single_var,
-                                           decl.initializer, kNoSourcePosition),
-                  kNoSourcePosition),
-              zone());
-        }
+        auto loop =

[marja, 2016/09/20 19:36:30]

I was thinking about the use of "auto" here (and below)... it's not clear from
the context what type it is (some factory funcs, especially in the PreParser,
return generic types of Statements, some of them return specialized
Statements... so it's not possible to say just after reading this line what type
auto suppresses.

However, does the use of auto here allow us to get away with fewer typedefs in
the traits? (A stupid question.) Any other benefits I'm not seeing?

[nickie, 2016/09/21 09:20:02]

Yes, that's precisely what it does.  (BTW, it's used in the same way in other
places too, e.g., while and do-while loops.)  In the preparser, loop is a
PreParserStatement; in the parser, it's a ForEachStatement*.  We would need a
typedef such as Types::ForEachStatementT if we did not to use "auto" here.

[marja, 2016/09/21 10:31:42]

Ok, then it's fine.

+            factory()->NewForEachStatement(for_info.mode, labels, stmt_pos);
+        typename Types::Target target(this, loop);
 
-        // Rewrite a for-in/of statement of the form
-        //
-        //   for (let/const/var x in/of e) b
-        //
-        // into
-        //
-        //   {
-        //     <let x' be a temporary variable>
-        //     for (x' in/of e) {
-        //       let/const/var x;
-        //       x = x';
-        //       b;
-        //     }
-        //     let x;  // for TDZ
-        //   }
+        int each_keyword_pos = scanner()->location().beg_pos;
 
-        Variable* temp = NewTemporary(ast_value_factory()->dot_for_string());
-        ForEachStatement* loop =
-            factory()->NewForEachStatement(mode, labels, stmt_pos);
-        ParserTarget target(this, loop);
-
-        int each_keyword_position = scanner()->location().beg_pos;
-
-        Expression* enumerable;
-        if (mode == ForEachStatement::ITERATE) {
+        ExpressionT enumerable = impl()->EmptyExpression();
+        if (for_info.mode == ForEachStatement::ITERATE) {
           ExpressionClassifier classifier(this);
           enumerable = ParseAssignmentExpression(true, CHECK_OK);
-          RewriteNonPattern(CHECK_OK);
+          impl()->RewriteNonPattern(CHECK_OK);
         } else {
           enumerable = ParseExpression(true, CHECK_OK);
         }
 
         Expect(Token::RPAREN, CHECK_OK);
 
-
-        Block* body_block =
-            factory()->NewBlock(NULL, 3, false, kNoSourcePosition);
-
-        Statement* final_loop;
+        StatementT final_loop = impl()->NullStatement();
         {
           ReturnExprScope no_tail_calls(function_state_,
                                         ReturnExprContext::kInsideForInOfBody);
           BlockState block_state(&scope_state_);
           block_state.set_start_position(scanner()->location().beg_pos);
 
-          Statement* body = ParseScopedStatement(NULL, true, CHECK_OK);
+          StatementT body = ParseScopedStatement(nullptr, true, CHECK_OK);
 
-          auto each_initialization_block =
-              factory()->NewBlock(nullptr, 1, true, kNoSourcePosition);
-          {
-            auto descriptor = parsing_result.descriptor;
-            descriptor.declaration_pos = kNoSourcePosition;
-            descriptor.initialization_pos = kNoSourcePosition;
-            decl.initializer = factory()->NewVariableProxy(temp);
+          std::pair<BlockT, ExpressionT> body_block_and_each_variable =
+              impl()->DesugarBindingInForEachStatement(&for_info, CHECK_OK);
+          body_block_and_each_variable.first->statements()->Add(body, zone());

[adamk, 2016/09/20 16:34:31]

Please create a variable for body_block, it'll make the rest of this code a
little more readable.

[marja, 2016/09/21 08:23:21]

Why do we need the pair anyway, it looks like we're always using either .first
or .second?

[nickie, 2016/09/21 09:20:02]

After offline discussion, I'm removing the pair and introducing two out
parameters to DesugarBindingInForEachStatement.

[nickie, 2016/09/21 09:20:02]

See comment below.

+          final_loop = impl()->InitializeForEachStatement(
+              loop, body_block_and_each_variable.second, enumerable,
+              body_block_and_each_variable.first, each_keyword_pos);
 
-            bool is_for_var_of =
-                mode == ForEachStatement::ITERATE &&
-                parsing_result.descriptor.mode == VariableMode::VAR;
-
-            PatternRewriter::DeclareAndInitializeVariables(
-                this, each_initialization_block, &descriptor, &decl,
-                bound_names_are_lexical || is_for_var_of ? &bound_names
-                                                         : nullptr,
-                CHECK_OK);
-
-            // Annex B.3.5 prohibits the form
-            // `try {} catch(e) { for (var e of {}); }`
-            // So if we are parsing a statement like `for (var ... of ...)`
-            // we need to walk up the scope chain and look for catch scopes
-            // which have a simple binding, then compare their binding against
-            // all of the names declared in the init of the for-of we're
-            // parsing.
-            if (is_for_var_of) {
-              Scope* catch_scope = scope();
-              while (catch_scope != nullptr &&
-                     !catch_scope->is_declaration_scope()) {
-                if (catch_scope->is_catch_scope()) {
-                  auto name = catch_scope->catch_variable_name();
-                  if (name !=
-                      ast_value_factory()
-                          ->dot_catch_string()) {  // i.e. is a simple binding
-                    if (bound_names.Contains(name)) {
-                      ReportMessageAt(parsing_result.bindings_loc,
-                                      MessageTemplate::kVarRedeclaration, name);
-                      *ok = false;
-                      return nullptr;
-                    }
-                  }
-                }
-                catch_scope = catch_scope->outer_scope();
-              }
-            }
-          }
-
-          body_block->statements()->Add(each_initialization_block, zone());
-          body_block->statements()->Add(body, zone());
-          VariableProxy* temp_proxy =
-              factory()->NewVariableProxy(temp, each_beg_pos, each_end_pos);
-          final_loop = InitializeForEachStatement(
-              loop, temp_proxy, enumerable, body_block, each_keyword_position);
           block_state.set_end_position(scanner()->location().end_pos);
-          body_block->set_scope(block_state.FinalizedBlockScope());
+          body_block_and_each_variable.first->set_scope(
+              block_state.FinalizedBlockScope());
         }
 
-        // Create a TDZ for any lexically-bound names.
-        if (bound_names_are_lexical) {
-          DCHECK_NULL(init_block);
-
-          init_block =
-              factory()->NewBlock(nullptr, 1, false, kNoSourcePosition);
-
-          for (int i = 0; i < bound_names.length(); ++i) {
-            // TODO(adamk): This needs to be some sort of special
-            // INTERNAL variable that's invisible to the debugger
-            // but visible to everything else.
-            Declaration* tdz_decl = DeclareVariable(
-                bound_names[i], LET, kNoSourcePosition, CHECK_OK);
-            tdz_decl->proxy()->var()->set_initializer_position(position());
-          }
-        }
+        init_block =
+            impl()->CreateForEachStatementTDZ(init_block, &for_info, ok);
 
         for_state.set_end_position(scanner()->location().end_pos);
         Scope* for_scope = for_state.FinalizedBlockScope();
         // Parsed for-in loop w/ variable declarations.
-        if (init_block != nullptr) {
+        if (!impl()->IsNullStatement(init_block)) {
           init_block->statements()->Add(final_loop, zone());
           init_block->set_scope(for_scope);
           return init_block;
         } else {
           DCHECK_NULL(for_scope);
           return final_loop;
         }
       } else {
-        bound_names_are_lexical =
-            IsLexicalVariableMode(parsing_result.descriptor.mode);
-        init = BuildInitializationBlock(
-            &parsing_result, bound_names_are_lexical ? &bound_names : nullptr,
+        // One or more declaration not followed by in/of.
+        init = impl()->BuildInitializationBlock(
+            &for_info.parsing_result,
+            for_info.bound_names_are_lexical ? &for_info.bound_names : nullptr,
             CHECK_OK);
       }
     } else {
+      // The initializer does not contain declarations.
       int lhs_beg_pos = peek_position();
       ExpressionClassifier classifier(this);
-      Expression* expression = ParseExpressionCoverGrammar(false, CHECK_OK);
+      ExpressionT expression = ParseExpressionCoverGrammar(false, CHECK_OK);
       int lhs_end_pos = scanner()->location().end_pos;
-      ForEachStatement::VisitMode mode = ForEachStatement::ENUMERATE;
 
-      bool is_for_each = CheckInOrOf(&mode);
+      bool is_for_each = CheckInOrOf(&for_info.mode);
       bool is_destructuring = is_for_each && (expression->IsArrayLiteral() ||
                                               expression->IsObjectLiteral());
 
       if (is_destructuring) {
         ValidateAssignmentPattern(CHECK_OK);
       } else {
-        RewriteNonPattern(CHECK_OK);
+        impl()->RewriteNonPattern(CHECK_OK);
       }
 
       if (is_for_each) {
+        // Initializer is reference followed by in/of.
         if (!is_destructuring) {
-          expression = CheckAndRewriteReferenceExpression(
+          expression = impl()->CheckAndRewriteReferenceExpression(
               expression, lhs_beg_pos, lhs_end_pos,
               MessageTemplate::kInvalidLhsInFor, kSyntaxError, CHECK_OK);
         }
 
-        ForEachStatement* loop =
-            factory()->NewForEachStatement(mode, labels, stmt_pos);
-        ParserTarget target(this, loop);
+        auto loop =
+            factory()->NewForEachStatement(for_info.mode, labels, stmt_pos);
+        typename Types::Target target(this, loop);
 
-        int each_keyword_position = scanner()->location().beg_pos;
+        int each_keyword_pos = scanner()->location().beg_pos;
 
-        Expression* enumerable;
-        if (mode == ForEachStatement::ITERATE) {
+        ExpressionT enumerable = impl()->EmptyExpression();
+        if (for_info.mode == ForEachStatement::ITERATE) {
           ExpressionClassifier classifier(this);
           enumerable = ParseAssignmentExpression(true, CHECK_OK);
-          RewriteNonPattern(CHECK_OK);
+          impl()->RewriteNonPattern(CHECK_OK);
         } else {
           enumerable = ParseExpression(true, CHECK_OK);
         }
 
         Expect(Token::RPAREN, CHECK_OK);
 
         {
           ReturnExprScope no_tail_calls(function_state_,
                                         ReturnExprContext::kInsideForInOfBody);
           BlockState block_state(&scope_state_);
           block_state.set_start_position(scanner()->location().beg_pos);
 
           // 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);
+          StatementT body = ParseScopedStatement(nullptr, true, CHECK_OK);
           block_state.set_end_position(scanner()->location().end_pos);
-          Statement* final_loop = InitializeForEachStatement(
-              loop, expression, enumerable, body, each_keyword_position);
+          StatementT final_loop = impl()->InitializeForEachStatement(
+              loop, expression, enumerable, body, each_keyword_pos);
 
           Scope* for_scope = for_state.FinalizedBlockScope();
           DCHECK_NULL(for_scope);
           USE(for_scope);
           Scope* block_scope = block_state.FinalizedBlockScope();
           DCHECK_NULL(block_scope);
           USE(block_scope);
           return final_loop;
         }
       } else {
+        // Initializer is just an expression.
         init = factory()->NewExpressionStatement(expression, lhs_beg_pos);
       }
     }
   }
 
-  // Standard 'for' loop
-  ForStatement* loop = factory()->NewForStatement(labels, stmt_pos);
-  ParserTarget target(this, loop);
+  // Standard 'for' loop, we have parsed the initializer at this point.
+  auto loop = factory()->NewForStatement(labels, stmt_pos);
+  typename Types::Target target(this, loop);
 
-  // Parsed initializer at this point.
   Expect(Token::SEMICOLON, CHECK_OK);
 
-  Expression* cond = NULL;
-  Statement* next = NULL;
-  Statement* body = NULL;
+  ExpressionT cond = impl()->EmptyExpression();
+  StatementT next = impl()->NullStatement();
+  StatementT body = impl()->NullStatement();
 
   // If there are let bindings, then condition and the next statement of the
   // for loop must be parsed in a new scope.
   Scope* inner_scope = scope();
   // TODO(verwaest): Allocate this through a ScopeState as well.
-  if (bound_names_are_lexical && bound_names.length() > 0) {
+  if (for_info.bound_names_are_lexical && for_info.bound_names.length() > 0) {
     inner_scope = NewScopeWithParent(inner_scope, BLOCK_SCOPE);
     inner_scope->set_start_position(scanner()->location().beg_pos);
   }
   {
     BlockState block_state(&scope_state_, inner_scope);
 
     if (peek() != Token::SEMICOLON) {
       cond = ParseExpression(true, CHECK_OK);
     }
     Expect(Token::SEMICOLON, CHECK_OK);
 
     if (peek() != Token::RPAREN) {
-      Expression* exp = ParseExpression(true, CHECK_OK);
+      ExpressionT exp = ParseExpression(true, CHECK_OK);
       next = factory()->NewExpressionStatement(exp, exp->position());
     }
     Expect(Token::RPAREN, CHECK_OK);
 
-    body = ParseScopedStatement(NULL, true, CHECK_OK);
+    body = ParseScopedStatement(nullptr, true, CHECK_OK);
   }
 
-  Statement* result = NULL;
-  if (bound_names_are_lexical && bound_names.length() > 0) {
-    result = DesugarLexicalBindingsInForStatement(
-        inner_scope, parsing_result.descriptor.mode, &bound_names, loop, init,
-        cond, next, body, CHECK_OK);
+  if (for_info.bound_names_are_lexical && for_info.bound_names.length() > 0) {
+    auto result = impl()->DesugarLexicalBindingsInForStatement(
+        loop, init, cond, next, body, inner_scope, &for_info, CHECK_OK);
     for_state.set_end_position(scanner()->location().end_pos);
+    return result;
   } else {
     for_state.set_end_position(scanner()->location().end_pos);
     Scope* for_scope = for_state.FinalizedBlockScope();
-    if (for_scope) {
+    if (for_scope != nullptr) {
       // Rewrite a for statement of the form
       //   for (const x = i; c; n) b
       //
       // into
       //
       //   {
       //     const x = i;
       //     for (; c; n) b
       //   }
       //
       // or, desugar
       //   for (; c; n) b
       // into
       //   {
       //     for (; c; n) b
       //   }
       // just in case b introduces a lexical binding some other way, e.g., if b
       // is a FunctionDeclaration.
-      Block* block = factory()->NewBlock(NULL, 2, false, kNoSourcePosition);
-      if (init != nullptr) {
+      BlockT block = factory()->NewBlock(NULL, 2, false, kNoSourcePosition);
+      if (!impl()->IsNullStatement(init)) {
         block->statements()->Add(init, zone());
       }
       block->statements()->Add(loop, zone());
       block->set_scope(for_scope);
-      loop->Initialize(NULL, cond, next, body);
-      result = block;
+      loop->Initialize(init, cond, next, body);
+      return block;
     } else {
       loop->Initialize(init, cond, next, body);
-      result = loop;
+      return loop;
     }
   }
-  return result;
 }
 
 #undef CHECK_OK
 #undef CHECK_OK_CUSTOM
 
 template <typename Impl>
 void ParserBase<Impl>::ObjectLiteralChecker::CheckDuplicateProto(
     Token::Value property) {
   if (property == Token::SMI || property == Token::NUMBER) return;
 
@@ skipping 40 matching lines @@
     has_seen_constructor_ = true;
     return;
   }
 }
 
 
 }  // namespace internal
 }  // namespace v8
 
 #endif  // V8_PARSING_PARSER_BASE_H