Remove "is function lazy" logic from Preparser + tiny error reporting refactoring.

It doesn't need to have this logic.

ParseLazyFunctionLiteralBody is basically just ParseStatementList
+ log the function position. But PreParser doesn't need to have
the "which functions to log" logic, since logging the function is
always done exactly when Parser falls back to PreParser. (See
PreParseLazyFunction.)

So in the current state, PreParser would log several functions in
a SingletonLogger, and only the last one would take
effect (that's the one Parser also logs in SkipLazyFunctionBody).

Also updated test-parsing/Regress928 to produce the preparse data
the way we do now (i.e., not running the PreParser directly, but
running the Parser).

Error reporting: when PreParser finds an error, it doesn't need
to ReportUnexpectedToken in PreParseLazyFunction, since it
already has reported the error whenever it found it.

BUG=v8:5515

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

src/parsing/preparse-data.h

@@ -81,6 +81,8 @@ class SingletonLogger : public ParserRecorder {
                            LanguageMode language_mode, bool uses_super_property,
                            bool calls_eval) {
     DCHECK(!has_error_);
+    // Check that we only log at most one function.
+    DCHECK(start_ == -1 && end_ == -1);
     start_ = start;
     end_ = end;
     literals_ = literals;

src/parsing/preparser.cc

@@ -84,7 +84,7 @@ PreParserIdentifier PreParser::GetSymbol() const {
 }
 
 PreParser::PreParseResult PreParser::PreParseLazyFunction(
-    DeclarationScope* function_scope, bool parsing_module, ParserRecorder* log,
+    DeclarationScope* function_scope, bool parsing_module, SingletonLogger* log,
     bool is_inner_function, bool may_abort, int* use_counts) {
   DCHECK_EQ(FUNCTION_SCOPE, function_scope->scope_type());
   parsing_module_ = parsing_module;
@@ -101,7 +101,7 @@ PreParser::PreParseResult PreParser::PreParseLazyFunction(
   DCHECK_EQ(Token::LBRACE, scanner()->current_token());
   bool ok = true;
   int start_position = peek_position();
-  LazyParsingResult result = ParseLazyFunctionLiteralBody(may_abort, &ok);
+  LazyParsingResult result = ParseStatementListAndLogFunction(may_abort, &ok);
   use_counts_ = nullptr;
   track_unresolved_variables_ = false;
   if (result == kLazyParsingAborted) {
@@ -109,7 +109,7 @@ PreParser::PreParseResult PreParser::PreParseLazyFunction(
   } else if (stack_overflow()) {
     return kPreParseStackOverflow;
   } else if (!ok) {
-    ReportUnexpectedToken(scanner()->current_token());
+    DCHECK(log->has_error());
   } else {
     DCHECK_EQ(Token::RBRACE, scanner()->peek());
     if (is_strict(function_scope->language_mode())) {
@@ -145,7 +145,6 @@ PreParser::Expression PreParser::ParseFunctionLiteral(
 
   // Parse function body.
   PreParserStatementList body;
-  bool outer_is_script_scope = scope()->is_script_scope();
   DeclarationScope* function_scope = NewFunctionScope(kind);
   function_scope->SetLanguageMode(language_mode);
   FunctionState function_state(&function_state_, &scope_state_, function_scope);
@@ -163,17 +162,8 @@ PreParser::Expression PreParser::ParseFunctionLiteral(
   CheckArityRestrictions(formals.arity, kind, formals.has_rest, start_position,
                          formals_end_position, CHECK_OK);
 
-  // See Parser::ParseFunctionLiteral for more information about lazy parsing
-  // and lazy compilation.
-  bool is_lazily_parsed = (outer_is_script_scope && allow_lazy() &&
-                           !function_state_->this_function_is_parenthesized());
-
   Expect(Token::LBRACE, CHECK_OK);
-  if (is_lazily_parsed) {
-    ParseLazyFunctionLiteralBody(false, CHECK_OK);
-  } else {
-    ParseStatementList(body, Token::RBRACE, CHECK_OK);
-  }
+  ParseStatementList(body, Token::RBRACE, CHECK_OK);
   Expect(Token::RBRACE, CHECK_OK);
 
   // Parsing the body may change the language mode in our scope.
@@ -196,7 +186,7 @@ PreParser::Expression PreParser::ParseFunctionLiteral(
   return Expression::Default();
 }
 
-PreParser::LazyParsingResult PreParser::ParseLazyFunctionLiteralBody(
+PreParser::LazyParsingResult PreParser::ParseStatementListAndLogFunction(
     bool may_abort, bool* ok) {
   int body_start = position();
   PreParserStatementList body;

src/parsing/preparser.h

@@ -888,7 +888,7 @@ class PreParser : public ParserBase<PreParser> {
   // At return, unless an error occurred, the scanner is positioned before the
   // the final '}'.
   PreParseResult PreParseLazyFunction(DeclarationScope* function_scope,
-                                      bool parsing_module, ParserRecorder* log,
+                                      bool parsing_module, SingletonLogger* log,
                                       bool track_unresolved_variables,
                                       bool may_abort, int* use_counts);
 
@@ -921,7 +921,7 @@ class PreParser : public ParserBase<PreParser> {
       FunctionNameValidity function_name_validity, FunctionKind kind,
       int function_token_pos, FunctionLiteral::FunctionType function_type,
       LanguageMode language_mode, bool* ok);
-  LazyParsingResult ParseLazyFunctionLiteralBody(bool may_abort, bool* ok);
+  LazyParsingResult ParseStatementListAndLogFunction(bool may_abort, bool* ok);
 
   struct TemplateLiteralState {};
 

test/cctest/test-parsing.cc

@@ -461,35 +461,29 @@ TEST(RegressChromium62639) {
 
 
 TEST(Regress928) {
-  v8::V8::Initialize();
-  i::Isolate* isolate = CcTest::i_isolate();
-
-  // Preparsing didn't consider the catch clause of a try statement
-  // as with-content, which made it assume that a function inside
-  // the block could be lazily compiled, and an extra, unexpected,
-  // entry was added to the data.
-  isolate->stack_guard()->SetStackLimit(i::GetCurrentStackPosition() -
-                                        128 * 1024);
+  // Test only applies when lazy parsing.
+  if (!i::FLAG_lazy || (i::FLAG_ignition && i::FLAG_ignition_eager)) return;
+  i::FLAG_min_preparse_length = 0;
 
+  // Tests that the first non-toplevel function is not included in the preparse
+  // data.
   const char* program =
       "try { } catch (e) { var foo = function () { /* first */ } }"
       "var bar = function () { /* second */ }";
 
-  v8::HandleScope handles(CcTest::isolate());
-  auto stream = i::ScannerStream::ForTesting(program);
-  i::CompleteParserRecorder log;
-  i::Scanner scanner(CcTest::i_isolate()->unicode_cache());
-  scanner.Initialize(stream.get());
-  i::Zone zone(CcTest::i_isolate()->allocator());
-  i::AstValueFactory ast_value_factory(&zone,
-                                       CcTest::i_isolate()->heap()->HashSeed());
-  i::PreParser preparser(&zone, &scanner, &ast_value_factory, &log,
-                         CcTest::i_isolate()->stack_guard()->real_climit());
-  preparser.set_allow_lazy(true);
-  i::PreParser::PreParseResult result = preparser.PreParseProgram();
-  CHECK_EQ(i::PreParser::kPreParseSuccess, result);
-  i::ScriptData* sd = log.GetScriptData();
-  i::ParseData* pd = i::ParseData::FromCachedData(sd);
+  v8::Isolate* isolate = CcTest::isolate();
+  v8::HandleScope handles(isolate);
+  v8::Local<v8::Context> context = v8::Context::New(isolate);
+  v8::Context::Scope context_scope(context);
+  v8::ScriptCompiler::Source script_source(v8_str(program));
+  v8::ScriptCompiler::Compile(context, &script_source,
+                              v8::ScriptCompiler::kProduceParserCache)
+      .ToLocalChecked();
+
+  const v8::ScriptCompiler::CachedData* cached_data =
+      script_source.GetCachedData();
+  i::ScriptData script_data(cached_data->data, cached_data->length);
+  std::unique_ptr<i::ParseData> pd(i::ParseData::FromCachedData(&script_data));
   pd->Initialize();
 
   int first_function =
@@ -507,8 +501,6 @@ TEST(Regress928) {
   i::FunctionEntry entry2 = pd->GetFunctionEntry(second_lbrace);
   CHECK(entry2.is_valid());
   CHECK_EQ('}', program[entry2.end_pos() - 1]);
-  delete sd;
-  delete pd;
 }