[parser] Skipping inner funcs: add info about variables.

- Declaring a variable called "this" for preparsed functions was unnecessary;
  DeclarationScope ctor already adds the variable.

- "arguments" for preparsed scopes need to be declared after parsing the
  function, like it's done in the parser.

- Now arguments_ can be the dummy variable, so adapted code to it.

- A previous refactoring CL ( https://codereview.chromium.org/2638333002 ) was
  incomplete; it had added ParserBase::ParseFunctionBody but
  PreParser::ParseFunction didn't call it. This CL completes that work. This is
  needed for getting "arguments" declared properly for preparsed functions.

- AllocateVariablesRecursively is already called for preparsed scopes (without
  this CL, that is), and it bails out early. However, before the bailout it used
  to dcheck num_stack_slots_ == 0; that is no longer true since we've done scope
  analysis for preparsed scopes.

- Test fix: we cannot have any lazy inner functions in the test, except the
  topmost lazy inner function. Such functions would also be lazy in the parser
  case, and the parser would just throw away their variables. Then the test
  tries to verify the preparsed data against the scopes without variables and fails.

- Disabled a test w/ a sloppy block function, will get that working again in the
  upcoming CLs.

BUG=v8:5516

Review-Url: https://codereview.chromium.org/2655623005
Cr-Commit-Position: refs/heads/master@{#42685}

src/ast/scopes.cc

@@ -657,7 +657,7 @@ void DeclarationScope::DeclareArguments(AstValueFactory* ast_value_factory) {
     // Note that it might never be accessed, in which case it won't be
     // allocated during variable allocation.
     arguments_ = Declare(zone(), ast_value_factory->arguments_string(), VAR);
-  } else if (IsLexicalVariableMode(arguments_->mode())) {
+  } else if (IsLexical(arguments_)) {
     // Check if there's lexically declared variable named arguments to avoid
     // redeclaration. See ES#sec-functiondeclarationinstantiation, step 20.
     arguments_ = nullptr;
@@ -950,7 +950,8 @@ Variable* Scope::DeclareLocal(const AstRawString* name, VariableMode mode,
   // introduced during variable allocation, and TEMPORARY variables are
   // allocated via NewTemporary().
   DCHECK(IsDeclaredVariableMode(mode));
-  DCHECK(!GetDeclarationScope()->is_being_lazily_parsed());
+  DCHECK_IMPLIES(GetDeclarationScope()->is_being_lazily_parsed(),
+                 mode == VAR || mode == LET || mode == CONST);
   DCHECK(!GetDeclarationScope()->was_lazily_parsed());
   return Declare(zone(), name, mode, kind, init_flag, maybe_assigned_flag);
 }
@@ -1080,7 +1081,15 @@ void Scope::DeclareVariableName(const AstRawString* name, VariableMode mode) {
   DCHECK(scope_info_.is_null());
 
   // Declare the variable in the declaration scope.
-  variables_.DeclareName(zone(), name, mode);
+  if (FLAG_preparser_scope_analysis) {
+    Variable* var = LookupLocal(name);
+    if (var == nullptr) {
+      var = DeclareLocal(name, mode);
+    }
+    var->set_is_used();
+  } else {
+    variables_.DeclareName(zone(), name, mode);
+  }
 }
 
 VariableProxy* Scope::NewUnresolved(AstNodeFactory* factory,
@@ -1909,6 +1918,9 @@ VariableProxy* Scope::FetchFreeVariables(DeclarationScope* max_outer_scope,
 }
 
 bool Scope::MustAllocate(Variable* var) {
+  if (var == kDummyPreParserLexicalVariable || var == kDummyPreParserVariable) {
+    return true;
+  }
   DCHECK(var->location() != VariableLocation::MODULE);
   // Give var a read/write use if there is a chance it might be accessed
   // via an eval() call.  This is only possible if the variable has a
@@ -2085,7 +2097,8 @@ void ModuleScope::AllocateModuleVariables() {
 
 void Scope::AllocateVariablesRecursively() {
   DCHECK(!already_resolved_);
-  DCHECK_EQ(0, num_stack_slots_);
+  DCHECK_IMPLIES(!FLAG_preparser_scope_analysis, num_stack_slots_ == 0);
+
   // Don't allocate variables of preparsed scopes.
   if (is_declaration_scope() && AsDeclarationScope()->was_lazily_parsed()) {
     return;
@@ -2164,8 +2177,12 @@ void Scope::AllocateDebuggerScopeInfos(Isolate* isolate,
 void Scope::CollectVariableData(PreParsedScopeData* data) {
   PreParsedScopeData::ScopeScope scope_scope(data, scope_type(),
                                              start_position(), end_position());
-  // TODO(marja): Add data about the variables.
-
+  for (Variable* local : locals_) {
+    if (local->mode() == VAR || local->mode() == LET ||
+        local->mode() == CONST) {
+      scope_scope.AddVariable(local->location(), local->maybe_assigned());
+    }
+  }
   for (Scope* inner = inner_scope_; inner != nullptr; inner = inner->sibling_) {
     inner->CollectVariableData(data);
   }

src/parsing/preparsed-scope-data.cc

@@ -35,5 +35,12 @@ PreParsedScopeData::ScopeScope::~ScopeScope() {
   data_->backing_store_[index_in_data_ + 1] = variable_count_;
 }
 
+void PreParsedScopeData::ScopeScope::AddVariable(VariableLocation location,
+                                                 bool maybe_assigned) {
+  data_->backing_store_.push_back(location);
+  data_->backing_store_.push_back(maybe_assigned);
+  ++variable_count_;
+}
+
 }  // namespace internal
 }  // namespace v8

src/parsing/preparsed-scope-data.h

@@ -23,8 +23,7 @@ class PreParsedScopeData {
                int start_position, int end_position);
     ~ScopeScope();
 
-    // TODO(marja): The functions for adding information about local variables
-    // will appear here.
+    void AddVariable(VariableLocation location, bool maybe_assigned);
 
    private:
     PreParsedScopeData* data_;

src/parsing/preparser.cc

@@ -131,19 +131,19 @@ PreParser::PreParseResult PreParser::PreParseFunction(
         formals_end_position, CHECK_OK_VALUE(kPreParseSuccess));
     has_duplicate_parameters =
         !classifier()->is_valid_formal_parameter_list_without_duplicates();
-
-    if (track_unresolved_variables_) {
-      function_scope->DeclareVariableName(
-          ast_value_factory()->arguments_string(), VAR);
-      function_scope->DeclareVariableName(ast_value_factory()->this_string(),
-                                          VAR);
-    }
   }
 
   Expect(Token::LBRACE, CHECK_OK_VALUE(kPreParseSuccess));
   LazyParsingResult result = ParseStatementListAndLogFunction(
       &formals, has_duplicate_parameters, may_abort, ok);
 
+  if (!IsArrowFunction(kind) && track_unresolved_variables_) {
+    // Declare arguments after parsing the function since lexical 'arguments'
+    // masks the arguments object. Declare arguments before declaring the
+    // function var since the arguments object masks 'function arguments'.
+    function_scope->DeclareArguments(ast_value_factory());
+  }
+
   if (is_sloppy(function_scope->language_mode())) {
     function_scope->HoistSloppyBlockFunctions(nullptr);
   }
@@ -209,8 +209,6 @@ PreParser::Expression PreParser::ParseFunctionLiteral(
       runtime_call_stats_,
       counters[track_unresolved_variables_][parsing_on_main_thread_]);
 
-  // Parse function body.
-  PreParserStatementList body;
   DeclarationScope* function_scope = NewFunctionScope(kind);
   function_scope->SetLanguageMode(language_mode);
   FunctionState function_state(&function_state_, &scope_state_, function_scope);
@@ -230,8 +228,13 @@ PreParser::Expression PreParser::ParseFunctionLiteral(
                          formals_end_position, CHECK_OK);
 
   Expect(Token::LBRACE, CHECK_OK);
-  ParseStatementList(body, Token::RBRACE, CHECK_OK);
-  Expect(Token::RBRACE, CHECK_OK);
+
+  // Parse function body.
+  PreParserStatementList body;
+  int pos = function_token_pos == kNoSourcePosition ? peek_position()
+                                                    : function_token_pos;
+  ParseFunctionBody(body, function_name, pos, formals, kind, function_type,
+                    CHECK_OK);
 
   // Parsing the body may change the language mode in our scope.
   language_mode = function_scope->language_mode();
@@ -252,7 +255,6 @@ PreParser::Expression PreParser::ParseFunctionLiteral(
   if (is_strict(language_mode)) {
     CheckStrictOctalLiteral(start_position, end_position, CHECK_OK);
   }
-  function_scope->set_end_position(end_position);
 
   if (FLAG_trace_preparse) {
     PrintF("  [%s]: %i-%i\n",

test/cctest/test-parsing.cc

@@ -8711,8 +8711,23 @@ class ScopeTestHelper {
     }
     CHECK_EQ(data->backing_store_[index++], inner_scope_count);
 
-    // Variable count is 0. TODO(marja): implement.
-    CHECK_EQ(data->backing_store_[index++], 0);
+    int variable_count = 0;
+    for (Variable* local : scope->locals_) {
+      if (local->mode() == VAR || local->mode() == LET ||
+          local->mode() == CONST) {
+        ++variable_count;
+      }
+    }
+
+    CHECK_EQ(data->backing_store_[index++], variable_count);
+
+    for (Variable* local : scope->locals_) {
+      if (local->mode() == VAR || local->mode() == LET ||
+          local->mode() == CONST) {
+        CHECK_EQ(data->backing_store_[index++], local->location());
+        CHECK_EQ(data->backing_store_[index++], local->maybe_assigned());
+      }
+    }
 
     for (Scope* inner = scope->inner_scope(); inner != nullptr;
          inner = inner->sibling()) {
@@ -9022,7 +9037,6 @@ TEST(PreParserScopeAnalysis) {
       {"", "var1"},
       {"", "if (true) {}"},
       {"", "function f1() {}"},
-      {"", "if (true) { function f1() {} }"},
   };
 
   for (unsigned i = 0; i < arraysize(inners); ++i) {
@@ -9076,6 +9090,7 @@ TEST(PreParserScopeAnalysis) {
 
     script = factory->NewScript(source);
     i::ParseInfo eager_info(&zone, script);
+    eager_info.set_allow_lazy_parsing(false);
 
     CHECK(i::parsing::ParseProgram(&eager_info));
     CHECK(i::Compiler::Analyze(&eager_info));