[parsing] Produce same Scopes in Parser and PreParser when the params are not simple.

E.g.,
{ function lazy_inner(b = somevar) { let somevar; } }

If we don't produce the same scopes, PreParser thinks that the unresolved
variable inside the default parameter resolves into the variable declared inside
the function. Thus, it's not correctly recorded as a free variable.

One part is already done by https://codereview.chromium.org/2638333002 . But at
the laziness boundary, we still produced different scopes.

Unlike previously thought, this is also needed for lazy inner function
correctness, not only for "preparser scope analysis" (ie., skipping inner
functions).

BUG=v8:5938

Change-Id: I047cd43ef16478bb0f18d1f114845e7d1ab8c5f2
Reviewed-on: https://chromium-review.googlesource.com/439345
Commit-Queue: Marja Hölttä <marja@chromium.org>
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#43044}

src/parsing/preparser.cc

@@ -133,9 +133,25 @@ PreParser::PreParseResult PreParser::PreParseFunction(
         !classifier()->is_valid_formal_parameter_list_without_duplicates();
   }
 
+  DeclarationScope* inner_scope = function_scope;
+  LazyParsingResult result;
+
+  if (!formals.is_simple) {
+    inner_scope = NewVarblockScope();
+    inner_scope->set_start_position(scanner()->location().beg_pos);
+  }
+
+  {
+    BlockState block_state(&scope_state_, inner_scope);
     Expect(Token::LBRACE, CHECK_OK_VALUE(kPreParseSuccess));
-  LazyParsingResult result = ParseStatementListAndLogFunction(
+    result = ParseStatementListAndLogFunction(
         &formals, has_duplicate_parameters, may_abort, ok);
+  }
+
+  if (!formals.is_simple) {
+    SetLanguageMode(function_scope, inner_scope->language_mode());
+    inner_scope->set_end_position(scanner()->location().end_pos);
+  }
 
   if (is_sloppy(function_scope->language_mode())) {
     function_scope->HoistSloppyBlockFunctions(nullptr);
@@ -277,7 +293,7 @@ PreParser::LazyParsingResult PreParser::ParseStatementListAndLogFunction(
   // Position right after terminal '}'.
   DCHECK_EQ(Token::RBRACE, scanner()->peek());
   int body_end = scanner()->peek_location().end_pos;
-  DCHECK(this->scope()->is_function_scope());
+  DCHECK_EQ(this->scope()->is_function_scope(), formals->is_simple);
   log_.LogFunction(
       body_end, formals->num_parameters(), formals->function_length,
       has_duplicate_parameters, function_state_->materialized_literal_count(),

test/cctest/test-parsing.cc

@@ -9195,6 +9195,7 @@ TEST(NoPessimisticContextAllocation) {
       {"", "const {a = my_var} = {}", true},
       {"", "const {a: b = my_var} = {}", true},
       {"a = my_var", "", true},
+      {"a = my_var", "let my_var;", true},
       {"", "function inner2(a = my_var) { }", true},
       {"", "(a = my_var) => { }", true},
       {"{a} = {a: my_var}", "", true},
@@ -9248,6 +9249,10 @@ TEST(NoPessimisticContextAllocation) {
       {"",
        "if (true) { let my_var; if (true) { function my_var() {} } } my_var;",
        true},
+      {"", "function inner2(a = my_var) {}", true},
+      {"", "function inner2(a = my_var) { let my_var; }", true},
+      {"", "(a = my_var) => {}", true},
+      {"", "(a = my_var) => { let my_var; }", true},
       // No pessimistic context allocation:
       {"", "var my_var; my_var;", false},
       {"", "var my_var;", false},

test/mjsunit/regress/regress-5938.js

@@ -0,0 +1,14 @@
+// Copyright 2017 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.
+
+// Flags: --lazy-inner-functions
+
+let global = 0;
+{
+  let confusing = 13;
+  function lazy_func(b = confusing) { let confusing = 0; global = b; }
+  lazy_func();
+}
+
+assertEquals(13, global);