PreParser: track variable declarations and parameters

src/parsing/preparser.h

Index: src/parsing/preparser.h
diff --git a/src/parsing/preparser.h b/src/parsing/preparser.h
index f8afe17b0a0b8af8a0e848e7893495259ece0b25..ef60b3d4d3be418bfdcfcfed9cd71de289832370 100644
--- a/src/parsing/preparser.h
+++ b/src/parsing/preparser.h
@@ -153,9 +153,11 @@ class PreParserExpression {
     return PreParserExpression(TypeField::encode(kExpression));
   }
 
-  static PreParserExpression Assignment() {
+  static PreParserExpression Assignment(
+      ZoneList<const AstRawString*>* identifiers = nullptr) {

[Toon Verwaest, 2016/12/01 10:50:56]

What about just using a single-linked-list?

[marja, 2016/12/05 16:05:19]

Will be done in a follow-up CL, k? PreParserExpression already has this as a
ZoneList and I didn't want to change that in this CL.

     return PreParserExpression(TypeField::encode(kExpression) |
-                               ExpressionTypeField::encode(kAssignment));
+                                   ExpressionTypeField::encode(kAssignment),
+                               identifiers);
   }
 
   static PreParserExpression ObjectLiteral(
@@ -608,7 +610,8 @@ class PreParserFactory {
                                     PreParserExpression left,
                                     PreParserExpression right,
                                     int pos) {
-    return PreParserExpression::Assignment();
+    // For tracking identifiers for parameters with a default value.
+    return PreParserExpression::Assignment(left.identifiers_);
   }
   PreParserExpression NewYield(PreParserExpression generator_object,
                                PreParserExpression expression, int pos,
@@ -750,10 +753,34 @@ class PreParserFactory {
 
 
 struct PreParserFormalParameters : FormalParametersBase {
+  struct Parameter : public ZoneObject {
+    Parameter(const AstRawString* name, bool is_optional, bool is_rest)
+        : name(name),
+          pattern(PreParserExpression::Default()),
+          is_optional(is_optional),

[Toon Verwaest, 2016/12/01 10:50:56]

Do we care about is_optional and is_rest? I especially doubt we care about
is_rest?

[marja, 2016/12/05 16:05:19]

Removed

+          is_rest(is_rest) {}
+    Parameter(PreParserExpression pattern, bool is_optional, bool is_rest)
+        : name(nullptr),
+          pattern(pattern),
+          is_optional(is_optional),
+          is_rest(is_rest) {}
+    Parameter()
+        : name(nullptr),
+          pattern(PreParserExpression::Default()),
+          is_optional(false),
+          is_rest(false) {}
+    Parameter** next() { return &next_parameter; }
+    Parameter* const* next() const { return &next_parameter; }
+    const AstRawString* name;
+    PreParserExpression pattern;
+    bool is_optional;

[Toon Verwaest, 2016/12/01 10:50:56]

bool ... : 1;

[marja, 2016/12/05 16:05:19]

Removed

+    bool is_rest;
+    Parameter* next_parameter = nullptr;
+  };
   explicit PreParserFormalParameters(DeclarationScope* scope)
       : FormalParametersBase(scope) {}
 
-  void* params = nullptr;  // Dummy
+  ThreadedList<Parameter> params;
 };
 
 
@@ -1454,14 +1481,48 @@ class PreParser : public ParserBase<PreParser> {
                                     PreParserExpression initializer,
                                     int initializer_end_position,
                                     bool is_rest) {
+    if (track_unresolved_variables_) {
+      DCHECK(FLAG_lazy_inner_functions);
+      bool is_simple = pattern.IsIdentifier() && IsEmptyExpression(initializer);
+      if (is_simple) {
+        DCHECK_EQ(1, pattern.identifiers_->length());
+        parameters->params.Add(
+            new (zone()) PreParserFormalParameters::Parameter(
+                (*pattern.identifiers_)[0], !IsEmptyExpression(initializer),

[Toon Verwaest, 2016/12/01 10:50:56]

pattern.identifiers_->at(0)?

[marja, 2016/12/05 16:05:19]

Simplified this code so this branch is done.

+                is_rest));
+      } else {
+        parameters->params.Add(
+            new (zone()) PreParserFormalParameters::Parameter(
+                pattern, !IsEmptyExpression(initializer), is_rest));
+      }
+    }
     parameters->UpdateArityAndFunctionLength(!initializer.IsEmpty(), is_rest);
   }
 
-  V8_INLINE void DeclareFormalParameters(DeclarationScope* scope,
-                                         void* parameters) {
+  V8_INLINE void DeclareFormalParameters(
+      DeclarationScope* scope,
+      const ThreadedList<PreParserFormalParameters::Parameter>& parameters) {
     if (!classifier()->is_simple_parameter_list()) {
       scope->SetHasNonSimpleParameters();
     }
+    if (track_unresolved_variables_) {
+      DCHECK(FLAG_lazy_inner_functions);
+      for (auto parameter : parameters) {
+        if (parameter->name != nullptr) {
+          DCHECK(FLAG_lazy_inner_functions);
+          bool is_duplicate = false;

[Toon Verwaest, 2016/12/01 10:50:56]

Definitely seems like we don't care about is_duplicate; so we shouldn't do the
O(n^2) work :)

[marja, 2016/12/05 16:05:19]

Simplified this; I'm always using DeclareVariableName now, since it shouldn't
matter; for preparsed scopes, parameters and variables are the same.

+          scope->DeclareParameterName(parameter->name, parameter->is_optional,
+                                      parameter->is_rest, &is_duplicate,
+                                      ast_value_factory());
+        }
+        if (parameter->pattern.identifiers_ != nullptr) {
+          DCHECK(FLAG_lazy_inner_functions);
+          for (auto i : *parameter->pattern.identifiers_) {
+            scope->DeclareVariableName(i, VAR);
+          }
+        }
+      }
+    }
   }
 
   V8_INLINE void DeclareArrowFunctionFormalParameters(
@@ -1470,6 +1531,9 @@ class PreParser : public ParserBase<PreParser> {
       bool* ok) {
     // TODO(wingo): Detect duplicated identifiers in paramlists.  Detect
     // parameter lists that are too long.
+    // FIXME(marja): Add code to declare arrow function parameters (once we add
+    // lazy inner arrow functions - at the moment inner arrow functions are
+    // parsed eagerly).

[Toon Verwaest, 2016/12/01 10:50:56]

What about arrow functions fully parsed inside of
preparse-with-variable-resolution functions?

(function() { function f() { g = (...) => ... } })()

[marja, 2016/12/05 16:05:19]

Yeah, the comment was wrong. Added tests to demonstrate that we still
over-context-allocate in these cases and fixed the comment.

   }
 
   V8_INLINE void ReindexLiterals(const PreParserFormalParameters& parameters) {}