[Parsing] Always pass a valid character-stream into the parser

Changes the contract of the parser to have a valid character stream passed
in the ParseInfo rather than the current situation where either:
 - it is passed explicitly (e.g., for streaming scripts)
 - a Script is passed and a character stream is created for this
 - or a source and encoding is passed in ParseInfo and the character stream
   is created from this.

The parse info also now owns the character stream (in a unique_ptr). It
can be destroyed using a new ResetCharacterStream() function. This will
enable the character stream to be kept live if any functions are asm.js
modules which were parsed, in order to be reused by the asm.js parser.

BUG=v8:5203

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
Change-Id: Ifda167391b2eadb38ebf9fcb2f565d2ed9ea3c6f
Reviewed-on: https://chromium-review.googlesource.com/616767
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Marja Hölttä <marja@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47474}

src/background-parsing-task.cc

@@ -6,6 +6,7 @@
 
 #include "src/objects-inl.h"
 #include "src/parsing/parser.h"
+#include "src/parsing/scanner-character-streams.h"
 #include "src/vm-state-inl.h"
 
 namespace v8 {
@@ -33,21 +34,24 @@ BackgroundParsingTask::BackgroundParsingTask(
   // will happen in the main thread after parsing.
   ParseInfo* info = new ParseInfo(isolate->allocator());
   info->InitFromIsolate(isolate);
+  if (V8_UNLIKELY(FLAG_runtime_stats)) {
+    info->set_runtime_call_stats(new (info->zone()) RuntimeCallStats());
+  }
   info->set_toplevel();
-  source->info.reset(info);
-  info->set_source_stream(source->source_stream.get());
-  info->set_source_stream_encoding(source->encoding);
+  std::unique_ptr<Utf16CharacterStream> stream(
+      ScannerStream::For(source->source_stream.get(), source->encoding,
+                         info->runtime_call_stats()));
+  info->set_character_stream(std::move(stream));
   info->set_unicode_cache(&source_->unicode_cache);
   info->set_compile_options(options);
   info->set_allow_lazy_parsing();
-  if (V8_UNLIKELY(FLAG_runtime_stats)) {
-    info->set_runtime_call_stats(new (info->zone()) RuntimeCallStats());
-  }
   if (V8_UNLIKELY(info->block_coverage_enabled())) {
     info->AllocateSourceRangeMap();
   }
+  info->set_cached_data(&script_data_);
+
+  source->info.reset(info);
 
-  source_->info->set_cached_data(&script_data_);
   // Parser needs to stay alive for finalizing the parsing on the main
   // thread.
   source_->parser.reset(new Parser(source_->info.get()));
@@ -55,7 +59,6 @@ BackgroundParsingTask::BackgroundParsingTask(
                                          MaybeHandle<ScopeInfo>());
 }
 
-
 void BackgroundParsingTask::Run() {
   DisallowHeapAllocation no_allocation;
   DisallowHandleAllocation no_handles;
@@ -77,5 +80,6 @@ void BackgroundParsingTask::Run() {
     script_data_ = nullptr;
   }
 }
+
 }  // namespace internal
 }  // namespace v8

src/compiler-dispatcher/unoptimized-compile-job.cc

@@ -174,9 +174,11 @@ void UnoptimizedCompileJob::PrepareToParseOnMainThread(Isolate* isolate) {
 
   Handle<String> source(String::cast(script->source()), isolate);
   parse_info_.reset(new ParseInfo(isolate->allocator()));
+  parse_info_->InitFromIsolate(isolate);
   if (source->IsExternalTwoByteString() || source->IsExternalOneByteString()) {
-    character_stream_.reset(ScannerStream::For(
+    std::unique_ptr<Utf16CharacterStream> stream(ScannerStream::For(
         source, shared_->start_position(), shared_->end_position()));
+    parse_info_->set_character_stream(std::move(stream));
   } else {
     source = String::Flatten(source);
     const void* data;
@@ -235,13 +237,11 @@ void UnoptimizedCompileJob::PrepareToParseOnMainThread(Isolate* isolate) {
                     .ToHandleChecked();
     }
     wrapper_ = isolate->global_handles()->Create(*wrapper);
-
-    character_stream_.reset(
+    std::unique_ptr<Utf16CharacterStream> stream(
         ScannerStream::For(wrapper_, shared_->start_position() - offset,
                            shared_->end_position() - offset));
+    parse_info_->set_character_stream(std::move(stream));
   }
-  parse_info_->InitFromIsolate(isolate);
-  parse_info_->set_character_stream(character_stream_.get());
   parse_info_->set_hash_seed(isolate->heap()->HashSeed());
   parse_info_->set_is_named_expression(shared_->is_named_expression());
   parse_info_->set_compiler_hints(shared_->compiler_hints());
@@ -331,11 +331,9 @@ void UnoptimizedCompileJob::FinalizeParsingOnMainThread(Isolate* isolate) {
 
   parser_->HandleSourceURLComments(isolate, script);
 
-  parse_info_->set_character_stream(nullptr);
   parse_info_->set_unicode_cache(nullptr);
   parser_.reset();
   unicode_cache_.reset();
-  character_stream_.reset();
 }
 
 void UnoptimizedCompileJob::AnalyzeOnMainThread(Isolate* isolate) {
@@ -434,7 +432,6 @@ void UnoptimizedCompileJob::ResetOnMainThread(Isolate* isolate) {
   compilation_job_.reset();
   parser_.reset();
   unicode_cache_.reset();
-  character_stream_.reset();
   parse_info_.reset();
 
   if (!source_.is_null()) {

src/compiler-dispatcher/unoptimized-compile-job.h

@@ -107,7 +107,6 @@ class V8_EXPORT_PRIVATE UnoptimizedCompileJob : public CompilerDispatcherJob {
 
   // Members required for parsing.
   std::unique_ptr<UnicodeCache> unicode_cache_;
-  std::unique_ptr<Utf16CharacterStream> character_stream_;
   std::unique_ptr<ParseInfo> parse_info_;
   std::unique_ptr<Parser> parser_;
 

src/parsing/parse-info.cc

@@ -19,9 +19,6 @@ namespace internal {
 ParseInfo::ParseInfo(AccountingAllocator* zone_allocator)
     : zone_(std::make_shared<Zone>(zone_allocator, ZONE_NAME)),
       flags_(0),
-      source_stream_(nullptr),
-      source_stream_encoding_(ScriptCompiler::StreamedSource::ONE_BYTE),
-      character_stream_(nullptr),
       extension_(nullptr),
       compile_options_(ScriptCompiler::kNoCompileOptions),
       script_scope_(nullptr),
@@ -35,6 +32,7 @@ ParseInfo::ParseInfo(AccountingAllocator* zone_allocator)
       parameters_end_pos_(kNoSourcePosition),
       function_literal_id_(FunctionLiteral::kIdTypeInvalid),
       max_function_literal_id_(FunctionLiteral::kIdTypeInvalid),
+      character_stream_(nullptr),
       cached_data_(nullptr),
       ast_value_factory_(nullptr),
       ast_string_constants_(nullptr),
@@ -99,6 +97,8 @@ ParseInfo::ParseInfo(Handle<Script> script)
   }
 }
 
+ParseInfo::~ParseInfo() {}
+
 // static
 ParseInfo* ParseInfo::AllocateWithoutScript(Handle<SharedFunctionInfo> shared) {
   Isolate* isolate = shared->GetIsolate();
@@ -193,5 +193,13 @@ void ParseInfo::AllocateSourceRangeMap() {
   set_source_range_map(new (zone()) SourceRangeMap(zone()));
 }
 
+void ParseInfo::ResetCharacterStream() { character_stream_.reset(); }
+
+void ParseInfo::set_character_stream(
+    std::unique_ptr<Utf16CharacterStream> character_stream) {
+  DCHECK(character_stream_.get() == nullptr);
+  character_stream_.swap(character_stream);
+}
+
 }  // namespace internal
 }  // namespace v8

src/parsing/parse-info.h

@@ -40,7 +40,7 @@ class V8_EXPORT_PRIVATE ParseInfo {
   ParseInfo(Handle<Script> script);
   ParseInfo(Handle<SharedFunctionInfo> shared);
 
-  ~ParseInfo() {}
+  ~ParseInfo();
 
   void InitFromIsolate(Isolate* isolate);
 
@@ -91,25 +91,12 @@ class V8_EXPORT_PRIVATE ParseInfo {
                                       : NO_PARSE_RESTRICTION;
   }
 
-  ScriptCompiler::ExternalSourceStream* source_stream() const {
-    return source_stream_;
-  }
-  void set_source_stream(ScriptCompiler::ExternalSourceStream* source_stream) {
-    source_stream_ = source_stream;
-  }
-
-  ScriptCompiler::StreamedSource::Encoding source_stream_encoding() const {
-    return source_stream_encoding_;
-  }
-  void set_source_stream_encoding(
-      ScriptCompiler::StreamedSource::Encoding source_stream_encoding) {
-    source_stream_encoding_ = source_stream_encoding;
-  }
-
-  Utf16CharacterStream* character_stream() const { return character_stream_; }
-  void set_character_stream(Utf16CharacterStream* character_stream) {
-    character_stream_ = character_stream;
+  Utf16CharacterStream* character_stream() const {
+    return character_stream_.get();
   }
+  void set_character_stream(
+      std::unique_ptr<Utf16CharacterStream> character_stream);
+  void ResetCharacterStream();
 
   v8::Extension* extension() const { return extension_; }
   void set_extension(v8::Extension* extension) { extension_ = extension; }
@@ -273,9 +260,6 @@ class V8_EXPORT_PRIVATE ParseInfo {
   //------------- Inputs to parsing and scope analysis -----------------------
   std::shared_ptr<Zone> zone_;
   unsigned flags_;
-  ScriptCompiler::ExternalSourceStream* source_stream_;
-  ScriptCompiler::StreamedSource::Encoding source_stream_encoding_;
-  Utf16CharacterStream* character_stream_;
   v8::Extension* extension_;
   ScriptCompiler::CompileOptions compile_options_;
   DeclarationScope* script_scope_;
@@ -295,6 +279,7 @@ class V8_EXPORT_PRIVATE ParseInfo {
   MaybeHandle<ScopeInfo> maybe_outer_scope_info_;
 
   //----------- Inputs+Outputs of parsing and scope analysis -----------------
+  std::unique_ptr<Utf16CharacterStream> character_stream_;
   ScriptData** cached_data_;  // used if available, populated if requested.
   ConsumedPreParsedScopeData consumed_preparsed_scope_data_;
   std::shared_ptr<AstValueFactory> ast_value_factory_;

src/parsing/parser.cc

@@ -22,7 +22,6 @@
 #include "src/parsing/expression-scope-reparenter.h"
 #include "src/parsing/parse-info.h"
 #include "src/parsing/rewriter.h"
-#include "src/parsing/scanner-character-streams.h"
 #include "src/runtime/runtime.h"
 #include "src/string-stream.h"
 #include "src/tracing/trace-event.h"
@@ -495,8 +494,7 @@ Parser::Parser(ParseInfo* info)
   // Even though we were passed ParseInfo, we should not store it in
   // Parser - this makes sure that Isolate is not accidentally accessed via
   // ParseInfo during background parsing.
-  DCHECK(!info->script().is_null() || info->source_stream() != nullptr ||
-         info->character_stream() != nullptr);
+  DCHECK(info->character_stream() != nullptr);
   // Determine if functions can be lazily compiled. This is necessary to
   // allow some of our builtin JS files to be lazily compiled. These
   // builtins cannot be handled lazily by the parser, since we have to know
@@ -561,8 +559,6 @@ FunctionLiteral* Parser::ParseProgram(Isolate* isolate, ParseInfo* info) {
       runtime_call_stats_, info->is_eval() ? &RuntimeCallStats::ParseEval
                                            : &RuntimeCallStats::ParseProgram);
   TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.compile"), "V8.ParseProgram");
-  Handle<String> source(String::cast(info->script()->source()));
-  isolate->counters()->total_parse_size()->Increment(source->length());
   base::ElapsedTimer timer;
   if (FLAG_trace_parse) {
     timer.Start();
@@ -584,17 +580,10 @@ FunctionLiteral* Parser::ParseProgram(Isolate* isolate, ParseInfo* info) {
 
   DeserializeScopeChain(info, info->maybe_outer_scope_info());
 
-  source = String::Flatten(source);
-  FunctionLiteral* result;
+  scanner_.Initialize(info->character_stream(), info->is_module());
+  FunctionLiteral* result = DoParseProgram(info);
+  info->ResetCharacterStream();  // Character stream no longer used.
 
-  {
-    std::unique_ptr<Utf16CharacterStream> stream(ScannerStream::For(source));
-    scanner_.Initialize(stream.get(), info->is_module());
-    result = DoParseProgram(info);
-  }
-  if (result != NULL) {
-    DCHECK_EQ(scanner_.peek_location().beg_pos, source->length());
-  }
   HandleSourceURLComments(isolate, info->script());
 
   if (FLAG_trace_parse && result != nullptr) {
@@ -735,8 +724,6 @@ FunctionLiteral* Parser::ParseFunction(Isolate* isolate, ParseInfo* info,
   RuntimeCallTimerScope runtime_timer(runtime_call_stats_,
                                       &RuntimeCallStats::ParseFunction);
   TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.compile"), "V8.ParseFunction");
-  Handle<String> source(String::cast(info->script()->source()));
-  isolate->counters()->total_parse_size()->Increment(source->length());
   base::ElapsedTimer timer;
   if (FLAG_trace_parse) {
     timer.Start();
@@ -750,18 +737,15 @@ FunctionLiteral* Parser::ParseFunction(Isolate* isolate, ParseInfo* info,
   }
 
   // Initialize parser state.
-  source = String::Flatten(source);
-  FunctionLiteral* result;
-  {
-    std::unique_ptr<Utf16CharacterStream> stream(ScannerStream::For(
-        source, shared_info->start_position(), shared_info->end_position()));
-    Handle<String> name(shared_info->name());
-    scanner_.Initialize(stream.get(), info->is_module());
-    result = DoParseFunction(info, ast_value_factory()->GetString(name));
-    if (result != nullptr) {
-      Handle<String> inferred_name(shared_info->inferred_name());
-      result->set_inferred_name(inferred_name);
-    }
+  Handle<String> name(shared_info->name());
+  info->set_function_name(ast_value_factory()->GetString(name));
+  scanner_.Initialize(info->character_stream(), info->is_module());
+
+  FunctionLiteral* result = DoParseFunction(info, info->function_name());
+  info->ResetCharacterStream();  // Character stream no longer used.
+  if (result != nullptr) {
+    Handle<String> inferred_name(shared_info->inferred_name());
+    result->set_inferred_name(inferred_name);
   }
 
   if (FLAG_trace_parse && result != NULL) {
@@ -3388,19 +3372,7 @@ void Parser::ParseOnBackground(ParseInfo* info) {
     }
   }
 
-  std::unique_ptr<Utf16CharacterStream> stream;
-  Utf16CharacterStream* stream_ptr;
-  if (info->character_stream()) {
-    DCHECK(info->source_stream() == nullptr);
-    stream_ptr = info->character_stream();
-  } else {
-    DCHECK(info->character_stream() == nullptr);
-    stream.reset(ScannerStream::For(info->source_stream(),
-                                    info->source_stream_encoding(),
-                                    runtime_call_stats_));
-    stream_ptr = stream.get();
-  }
-  scanner_.Initialize(stream_ptr, info->is_module());
+  scanner_.Initialize(info->character_stream(), info->is_module());
   DCHECK(info->maybe_outer_scope_info().is_null());
 
   DCHECK(original_scope_);
@@ -3417,6 +3389,7 @@ void Parser::ParseOnBackground(ParseInfo* info) {
   } else {
     result = DoParseFunction(info, info->function_name());
   }
+  info->ResetCharacterStream();  // Character stream no longer used.
 
   info->set_literal(result);
 

src/parsing/parsing.cc

@@ -10,6 +10,7 @@
 #include "src/objects-inl.h"
 #include "src/parsing/parse-info.h"
 #include "src/parsing/parser.h"
+#include "src/parsing/scanner-character-streams.h"
 #include "src/vm-state-inl.h"
 
 namespace v8 {
@@ -22,6 +23,13 @@ bool ParseProgram(ParseInfo* info, Isolate* isolate) {
 
   VMState<PARSER> state(isolate);
 
+  // Create a character stream for the parser.
+  Handle<String> source(String::cast(info->script()->source()));
+  source = String::Flatten(source);
+  isolate->counters()->total_parse_size()->Increment(source->length());
+  std::unique_ptr<Utf16CharacterStream> stream(ScannerStream::For(source));
+  info->set_character_stream(std::move(stream));
+
   Parser parser(info);
 
   FunctionLiteral* result = nullptr;
@@ -47,6 +55,14 @@ bool ParseFunction(ParseInfo* info, Handle<SharedFunctionInfo> shared_info,
   DCHECK(!shared_info.is_null());
   DCHECK_NULL(info->literal());
 
+  // Create a character stream for the parser.
+  Handle<String> source(String::cast(info->script()->source()));
+  source = String::Flatten(source);
+  isolate->counters()->total_parse_size()->Increment(source->length());
+  std::unique_ptr<Utf16CharacterStream> stream(ScannerStream::For(
+      source, shared_info->start_position(), shared_info->end_position()));
+  info->set_character_stream(std::move(stream));
+
   VMState<PARSER> state(isolate);
 
   Parser parser(info);

test/cctest/test-parsing.cc

@@ -887,7 +887,6 @@ static void CheckParsesToNumber(const char* source) {
   i::Handle<i::Script> script = factory->NewScript(source_code);
 
   i::ParseInfo info(script);
-  i::Parser parser(&info);
   info.set_allow_lazy_parsing(false);
   info.set_toplevel(true);