[parser] Skipping inner funcs: omit uninteresting scopes in the data.

This is also needed so that PreParser doesn't need to gather more data for arrow
function params in order to create the uninteresting varblock scopes matching
the scopes created in Parser::BuildParameterInitializationBlock.

This cancels the changes in https://chromium-review.googlesource.com/c/444747
which make PreParser create uninteresting scopes for the normal (non-arrow)
function "eval in default param" case.

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

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

src/parsing/parser-base.h

@@ -3531,7 +3531,6 @@ void ParserBase<Impl>::ParseFormalParameter(FormalParametersT* parameters,
   //   BindingElement[?Yield, ?GeneratorParameter]
   bool is_rest = parameters->has_rest;
 
-  int initializer_start_position = kNoSourcePosition;
   ExpressionT pattern = ParsePrimaryExpression(CHECK_OK_CUSTOM(Void));
   ValidateBindingPattern(CHECK_OK_CUSTOM(Void));
 
@@ -3544,7 +3543,6 @@ void ParserBase<Impl>::ParseFormalParameter(FormalParametersT* parameters,
   ExpressionT initializer = impl()->EmptyExpression();
   if (!is_rest && Check(Token::ASSIGN)) {
     ExpressionClassifier init_classifier(this);
-    initializer_start_position = scanner()->peek_location().beg_pos;
     initializer = ParseAssignmentExpression(true, CHECK_OK_CUSTOM(Void));
     impl()->RewriteNonPattern(CHECK_OK_CUSTOM(Void));
     ValidateFormalParameterInitializer(CHECK_OK_CUSTOM(Void));
@@ -3556,7 +3554,6 @@ void ParserBase<Impl>::ParseFormalParameter(FormalParametersT* parameters,
   }
 
   impl()->AddFormalParameter(parameters, pattern, initializer,
-                             initializer_start_position,
                              scanner()->location().end_pos, is_rest);
 }
 

src/parsing/parser.cc

@@ -2445,7 +2445,6 @@ void Parser::AddArrowFunctionFormalParameters(
   }
 
   AddFormalParameter(parameters, expr, initializer,
-                     initializer ? initializer->position() : kNoSourcePosition,
                      end_pos, is_rest);
 }
 

src/parsing/parser.h

@@ -1059,7 +1059,6 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
   V8_INLINE void AddFormalParameter(ParserFormalParameters* parameters,
                                     Expression* pattern,
                                     Expression* initializer,
-                                    int initializer_start_position,
                                     int initializer_end_position,
                                     bool is_rest) {
     parameters->UpdateArityAndFunctionLength(initializer != nullptr, is_rest);

src/parsing/preparsed-scope-data.cc

@@ -4,24 +4,35 @@
 
 #include "src/parsing/preparsed-scope-data.h"
 
+#include "src/ast/scopes.h"
 #include "src/ast/variables.h"
 #include "src/objects-inl.h"
 
 namespace v8 {
 namespace internal {
 
+bool PreParsedScopeData::HasVariablesWhichNeedAllocationData(Scope* scope) {
+  if (!scope->is_hidden()) {
+    for (Variable* var : *scope->locals()) {
+      if (var->mode() == VAR || var->mode() == LET || var->mode() == CONST) {
+        return true;
+      }
+    }
+  }
+  for (Scope* inner = scope->inner_scope(); inner != nullptr;
+       inner = inner->sibling()) {
+    if (HasVariablesWhichNeedAllocationData(inner)) {
+      return true;
+    }
+  }
+  return false;
+}
+
 PreParsedScopeData::ScopeScope::ScopeScope(PreParsedScopeData* data,
                                            ScopeType scope_type,
                                            int start_position, int end_position)
-    : data_(data),
-      previous_scope_(data->current_scope_),
-      inner_scope_count_(0),
-      variable_count_(0) {
+    : data_(data), previous_scope_(data->current_scope_) {
   data->current_scope_ = this;
-  if (previous_scope_ != nullptr) {
-    ++previous_scope_->inner_scope_count_;
-  }
-
   data->backing_store_.push_back(scope_type);
   data->backing_store_.push_back(start_position);
   data->backing_store_.push_back(end_position);
@@ -34,8 +45,24 @@ PreParsedScopeData::ScopeScope::ScopeScope(PreParsedScopeData* data,
 
 PreParsedScopeData::ScopeScope::~ScopeScope() {
   data_->current_scope_ = previous_scope_;
-  data_->backing_store_[index_in_data_] = inner_scope_count_;
-  data_->backing_store_[index_in_data_ + 1] = variable_count_;
+  if (got_data_) {
+    DCHECK_GT(variable_count_ + inner_scope_count_, 0);
+    if (previous_scope_ != nullptr) {
+      previous_scope_->got_data_ = true;
+      ++previous_scope_->inner_scope_count_;
+    }
+    data_->backing_store_[index_in_data_] = inner_scope_count_;
+    data_->backing_store_[index_in_data_ + 1] = variable_count_;
+  } else {
+    // No interesting data for this scope (or its children); remove from the
+    // data.
+    DCHECK_EQ(data_->backing_store_.size(), index_in_data_ + 2);
+    DCHECK_GE(index_in_data_, 3);
+    DCHECK_EQ(variable_count_, 0);
+    data_->backing_store_.erase(
+        data_->backing_store_.begin() + index_in_data_ - 3,
+        data_->backing_store_.end());
+  }
 }
 
 void PreParsedScopeData::ScopeScope::MaybeAddVariable(Variable* var) {
@@ -51,6 +78,7 @@ void PreParsedScopeData::ScopeScope::MaybeAddVariable(Variable* var) {
     data_->backing_store_.push_back(var->location());
     data_->backing_store_.push_back(var->maybe_assigned());
     ++variable_count_;
+    got_data_ = true;
   }
 }
 

src/parsing/preparsed-scope-data.h

@@ -17,6 +17,10 @@ class PreParsedScopeData {
   PreParsedScopeData() {}
   ~PreParsedScopeData() {}
 
+  // Whether the scope has variables whose context allocation or
+  // maybeassignedness we need to decide based on preparsed scope data.
+  static bool HasVariablesWhichNeedAllocationData(Scope* scope);
+
   class ScopeScope {
    public:
     ScopeScope(PreParsedScopeData* data, ScopeType scope_type,
@@ -30,8 +34,9 @@ class PreParsedScopeData {
     size_t index_in_data_;
     ScopeScope* previous_scope_;
 
-    int inner_scope_count_;
-    int variable_count_;
+    int inner_scope_count_ = 0;
+    int variable_count_ = 0;
+    bool got_data_ = false;
     DISALLOW_COPY_AND_ASSIGN(ScopeScope);
   };
 

src/parsing/preparser.h

@@ -791,37 +791,19 @@ class PreParserFactory {
 
 struct PreParserFormalParameters : FormalParametersBase {
   struct Parameter : public ZoneObject {
-    Parameter(PreParserExpression pattern, bool is_destructuring,
-              bool has_initializer, int initializer_start_position,
-              int initializer_end_position, bool is_rest)
+    Parameter(PreParserExpression pattern, bool is_destructuring, bool is_rest)
         : pattern(pattern),
-          initializer_start_position(initializer_start_position),
-          initializer_end_position(initializer_end_position),
           is_destructuring(is_destructuring),
-          has_initializer(has_initializer),
-          is_rest(is_rest) {
-      DCHECK_IMPLIES(is_rest, !has_initializer);
-      DCHECK_IMPLIES(has_initializer,
-                     initializer_start_position != kNoSourcePosition);
-      DCHECK_IMPLIES(has_initializer,
-                     initializer_end_position != kNoSourcePosition);
-    }
+          is_rest(is_rest) {}
     Parameter** next() { return &next_parameter; }
     Parameter* const* next() const { return &next_parameter; }
 
-    bool is_simple() const {
-      return !is_destructuring && !has_initializer && !is_rest;
-    }
-
     bool is_nondestructuring_rest() const {
       return is_rest && !is_destructuring;
     }
     PreParserExpression pattern;
-    int initializer_start_position;
-    int initializer_end_position;
     Parameter* next_parameter = nullptr;
     bool is_destructuring : 1;
-    bool has_initializer : 1;
     bool is_rest : 1;
   };
   explicit PreParserFormalParameters(DeclarationScope* scope)
@@ -1407,13 +1389,6 @@ class PreParser : public ParserBase<PreParser> {
     if (track_unresolved_variables_) {
       for (auto parameter : parameters.params) {
         if (parameter->is_nondestructuring_rest()) break;
-        if (!parameter->is_simple() && scope()->calls_sloppy_eval()) {
-          Scope* param_scope = NewVarblockScope();
-          param_scope->set_start_position(
-              parameter->initializer_start_position);
-          param_scope->set_end_position(parameter->initializer_end_position);
-          param_scope->RecordEvalCall();
-        }
         if (parameter->pattern.variables_ != nullptr) {
           for (auto variable : *parameter->pattern.variables_) {
             scope()->DeclareVariableName(variable->raw_name(), LET);
@@ -1637,14 +1612,12 @@ class PreParser : public ParserBase<PreParser> {
   V8_INLINE void AddFormalParameter(PreParserFormalParameters* parameters,
                                     PreParserExpression pattern,
                                     PreParserExpression initializer,
-                                    int initializer_start_position,
                                     int initializer_end_position,
                                     bool is_rest) {
     if (track_unresolved_variables_) {
       DCHECK(FLAG_lazy_inner_functions);
       parameters->params.Add(new (zone()) PreParserFormalParameters::Parameter(
-          pattern, !IsIdentifier(pattern), !IsEmptyExpression(initializer),
-          initializer_start_position, initializer_end_position, is_rest));
+          pattern, !IsIdentifier(pattern), is_rest));
     }
     parameters->UpdateArityAndFunctionLength(!initializer.IsEmpty(), is_rest);
   }

test/cctest/parsing/test-preparser.cc

@@ -169,6 +169,9 @@ TEST(PreParserScopeAnalysis) {
       {"var var1; if (true) { const var1 = 0; }"},
       {"const var1 = 0; if (true) { const var1 = 0; }"},
 
+      // Variables deeper in the subscopes (scopes without variables inbetween).
+      {"if (true) { if (true) { function f() { var var1 = 5; } } }"},
+
       // Arguments and this.
       {"arguments;"},
       {"arguments = 5;", SKIP_STRICT},

test/cctest/scope-test-helper.h

@@ -17,22 +17,10 @@ class ScopeTestHelper {
     return var->scope()->MustAllocateInContext(var);
   }
 
-  // True if the scope is and its entire subscope tree are hidden.
-  static bool ScopeTreeIsHidden(Scope* scope) {
-    if (!scope->is_hidden()) {
-      return false;
-    }
-    for (Scope* inner = scope->inner_scope(); inner != nullptr;
-         inner = inner->sibling()) {
-      if (!ScopeTreeIsHidden(inner)) {
-        return false;
-      }
-    }
-    return true;
-  }
-
   static void CompareScopeToData(Scope* scope, const PreParsedScopeData* data,
                                  size_t& index, bool precise_maybe_assigned) {
+    CHECK(PreParsedScopeData::HasVariablesWhichNeedAllocationData(scope));
+    CHECK_GT(data->backing_store_.size(), index + 4);
     CHECK_EQ(data->backing_store_[index++], scope->scope_type());
     CHECK_EQ(data->backing_store_[index++], scope->start_position());
     CHECK_EQ(data->backing_store_[index++], scope->end_position());
@@ -40,9 +28,7 @@ class ScopeTestHelper {
     int inner_scope_count = 0;
     for (Scope* inner = scope->inner_scope(); inner != nullptr;
          inner = inner->sibling()) {
-      // FIXME(marja): This is probably not the right condition for knowing what
-      // scopes are present in the preparse data.
-      if (!ScopeTreeIsHidden(inner)) {
+      if (PreParsedScopeData::HasVariablesWhichNeedAllocationData(inner)) {
         ++inner_scope_count;
       }
     }
@@ -81,7 +67,7 @@ class ScopeTestHelper {
 
     for (Scope* inner = scope->inner_scope(); inner != nullptr;
          inner = inner->sibling()) {
-      if (!ScopeTreeIsHidden(inner)) {
+      if (PreParsedScopeData::HasVariablesWhichNeedAllocationData(inner)) {
         CompareScopeToData(inner, data, index, precise_maybe_assigned);
       }
     }