[parser] Skipping inner funcs: params part 3

Produce the same scopes / variables for parameters (part 3).

This CL fixes the ordering + variable types in PreParser when there are
simple parameters + a rest parameter. In that case, Parser declares
unnamed temporaries for the non-rest params, then the rest param, then
the named variables (which are not parameters) for the non-rest params.

BUG=v8:5516
R=vogelheim@chromium.org

Change-Id: I9b006595039c8002b0508d1d2a200aa9a0f3eae0
Reviewed-on: https://chromium-review.googlesource.com/443527Reviewed-by: Daniel Vogelheim <vogelheim@chromium.org>
Commit-Queue: Marja Hölttä <marja@chromium.org>
Cr-Commit-Position: refs/heads/master@{#43241}

src/parsing/preparser.cc

@@ -148,14 +148,20 @@ PreParser::PreParseResult PreParser::PreParseFunction(
         &formals, has_duplicate_parameters, may_abort, ok);
   }
 
-  if (is_sloppy(inner_scope->language_mode())) {
-    inner_scope->HoistSloppyBlockFunctions(nullptr);
-  }
-
   if (!formals.is_simple) {
+    BuildParameterInitializationBlock(formals, ok);
+
+    if (is_sloppy(inner_scope->language_mode())) {
+      inner_scope->HoistSloppyBlockFunctions(nullptr);
+    }
+
     SetLanguageMode(function_scope, inner_scope->language_mode());
     inner_scope->set_end_position(scanner()->peek_location().end_pos);
     inner_scope->FinalizeBlockScope();
+  } else {
+    if (is_sloppy(function_scope->language_mode())) {
+      function_scope->HoistSloppyBlockFunctions(nullptr);
+    }
   }
 
   if (!IsArrowFunction(kind) && track_unresolved_variables_) {

src/parsing/preparser.h

@@ -791,13 +791,19 @@ class PreParserFactory {
 
 struct PreParserFormalParameters : FormalParametersBase {
   struct Parameter : public ZoneObject {
-    Parameter(PreParserExpression pattern, bool is_rest)
-        : pattern(pattern), is_rest(is_rest) {}
+    Parameter(PreParserExpression pattern, bool is_destructuring, bool is_rest)
+        : pattern(pattern),
+          is_destructuring(is_destructuring),
+          is_rest(is_rest) {}
     Parameter** next() { return &next_parameter; }
     Parameter* const* next() const { return &next_parameter; }
+    bool is_nondestructuring_rest() const {
+      return is_rest && !is_destructuring;
+    }
     PreParserExpression pattern;
     Parameter* next_parameter = nullptr;
-    bool is_rest;
+    bool is_destructuring : 1;
+    bool is_rest : 1;
   };
   explicit PreParserFormalParameters(DeclarationScope* scope)
       : FormalParametersBase(scope) {}
@@ -1378,6 +1384,16 @@ class PreParser : public ParserBase<PreParser> {
 
   V8_INLINE PreParserStatement BuildParameterInitializationBlock(
       const PreParserFormalParameters& parameters, bool* ok) {
+    if (track_unresolved_variables_) {
+      for (auto parameter : parameters.params) {
+        if (parameter->is_nondestructuring_rest()) break;
+        if (parameter->pattern.variables_ != nullptr) {
+          for (auto variable : *parameter->pattern.variables_) {
+            scope()->DeclareVariableName(variable->raw_name(), LET);
+          }
+        }
+      }
+    }
     return PreParserStatement::Default();
   }
 
@@ -1598,8 +1614,8 @@ class PreParser : public ParserBase<PreParser> {
                                     bool is_rest) {
     if (track_unresolved_variables_) {
       DCHECK(FLAG_lazy_inner_functions);
-      parameters->params.Add(
-          new (zone()) PreParserFormalParameters::Parameter(pattern, is_rest));
+      parameters->params.Add(new (zone()) PreParserFormalParameters::Parameter(
+          pattern, !IsIdentifier(pattern), is_rest));
     }
     parameters->UpdateArityAndFunctionLength(!initializer.IsEmpty(), is_rest);
   }
@@ -1607,17 +1623,18 @@ class PreParser : public ParserBase<PreParser> {
   V8_INLINE void DeclareFormalParameters(
       DeclarationScope* scope,
       const ThreadedList<PreParserFormalParameters::Parameter>& parameters) {
-    if (!classifier()->is_simple_parameter_list()) {
-      scope->SetHasNonSimpleParameters();
-    }
+    bool is_simple = classifier()->is_simple_parameter_list();
+    if (!is_simple) scope->SetHasNonSimpleParameters();
     if (track_unresolved_variables_) {
       DCHECK(FLAG_lazy_inner_functions);
       for (auto parameter : parameters) {
-        if (parameter->pattern.variables_ != nullptr) {
-          for (auto variable : *parameter->pattern.variables_) {
-            scope->DeclareParameterName(
-                variable->raw_name(), parameter->is_rest, ast_value_factory());
-          }
+        bool use_name = is_simple || parameter->is_nondestructuring_rest();
+        if (use_name) {
+          DCHECK_NOT_NULL(parameter->pattern.variables_);
+          DCHECK_EQ(parameter->pattern.variables_->length(), 1);
+          auto variable = (*parameter->pattern.variables_)[0];
+          scope->DeclareParameterName(variable->raw_name(), parameter->is_rest,
+                                      ast_value_factory());
         }
       }
     }

test/cctest/parsing/test-preparser.cc

@@ -327,6 +327,16 @@ TEST(PreParserScopeAnalysis) {
       {"var1", "function f1() { var1; }"},
       {"var1", "function f1() { var1 = 9; }"},
 
+      {"var1, var2", ""},
+      {"var1, var2", "var2;"},
+      {"var1, var2", "var2 = 9;"},
+      {"var1, var2", "function f1() { var2; }"},
+      {"var1, var2", "function f1() { var2 = 9; }"},
+      {"var1, var2", "var1;"},
+      {"var1, var2", "var1 = 9;"},
+      {"var1, var2", "function f1() { var1; }"},
+      {"var1, var2", "function f1() { var1 = 9; }"},
+
       // Duplicate parameters.
       {"var1, var1", ""},
       {"var1, var1", "var1;"},
@@ -341,11 +351,18 @@ TEST(PreParserScopeAnalysis) {
       {"...var2", "function f1() { var2; }"},
       {"...var2", "function f1() { var2 = 9; }"},
 
+      {"var1, ...var2", ""},
+      {"var1, ...var2", "var2;"},
+      {"var1, ...var2", "var2 = 9;"},
+      {"var1, ...var2", "function f1() { var2; }"},
+      {"var1, ...var2", "function f1() { var2 = 9; }"},
+
       // FIXME(marja): destructuring parameters, default parameters, shadowing
       // parameters, default parameters referring to other parameters, arguments
       // parameter, eval in default parameter, params and locals, multiple
-      // params, many params and rest, destructuring rest, rest with default
-      // value, locals shadowing params.
+      // params, destructuring rest, locals shadowing params, locals shadowing
+      // rest, locals shadowing destructuring params, shadowing by a hoisted
+      // sloppy block function.
   };
 
   for (unsigned outer_ix = 0; outer_ix < arraysize(outers); ++outer_ix) {