From 80752a29b6afad7c4aca37dfd5ba91c7c6d1a02c Mon Sep 17 00:00:00 2001 From: Wiktor Garbacz Date: Wed, 22 Mar 2017 15:05:35 +0100 Subject: [PATCH] Make isolate explicit param of parsing:: functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A step towards removing isolate from ParseInfo. Removing isolate from ParseInfo will make it easier to create and execute parse tasks on background threads. BUG=v8:6093 Change-Id: I0a3546618d01b9232014da94cf8d0f72427a0d1d Reviewed-on: https://chromium-review.googlesource.com/458006 Commit-Queue: Wiktor Garbacz Reviewed-by: Ulan Degenbaev Reviewed-by: Daniel Vogelheim Reviewed-by: Marja Hölttä Cr-Commit-Position: refs/heads/master@{#44176} --- src/compiler.cc | 6 ++-- src/debug/debug-scopes.cc | 3 +- src/parsing/parser.h | 4 +-- src/parsing/parsing.cc | 12 +++---- src/parsing/parsing.h | 9 ++++-- src/runtime/runtime-internal.cc | 2 +- test/cctest/compiler/function-tester.cc | 2 +- .../compiler/test-loop-assignment-analysis.cc | 2 +- test/cctest/parsing/test-preparser.cc | 6 ++-- test/cctest/test-parsing.cc | 32 +++++++++---------- test/fuzzer/parser.cc | 2 +- .../compiler-dispatcher-unittest.cc | 4 +-- tools/parser-shell.cc | 6 ++-- 13 files changed, 47 insertions(+), 43 deletions(-) diff --git a/src/compiler.cc b/src/compiler.cc index 12f8ecb137..4d2c73b52f 100644 --- a/src/compiler.cc +++ b/src/compiler.cc @@ -674,7 +674,7 @@ MUST_USE_RESULT MaybeHandle GetUnoptimizedCode( // Parse and update ParseInfo with the results. { - if (!parsing::ParseAny(info->parse_info(), + if (!parsing::ParseAny(info->parse_info(), info->isolate(), inner_function_mode != Compiler::CONCURRENT)) { return MaybeHandle(); } @@ -1126,7 +1126,7 @@ Handle CompileToplevel(CompilationInfo* info) { { VMState state(info->isolate()); if (parse_info->literal() == nullptr) { - if (!parsing::ParseProgram(parse_info, false)) { + if (!parsing::ParseProgram(parse_info, info->isolate(), false)) { return Handle::null(); } @@ -1212,7 +1212,7 @@ bool Compiler::Analyze(CompilationInfo* info, } bool Compiler::ParseAndAnalyze(ParseInfo* info, Isolate* isolate) { - if (!parsing::ParseAny(info)) return false; + if (!parsing::ParseAny(info, isolate)) return false; if (info->is_toplevel()) { EnsureSharedFunctionInfosArrayOnScript(info, isolate); } diff --git a/src/debug/debug-scopes.cc b/src/debug/debug-scopes.cc index 9cc70a2528..2d4c01ebe3 100644 --- a/src/debug/debug-scopes.cc +++ b/src/debug/debug-scopes.cc @@ -109,7 +109,8 @@ ScopeIterator::ScopeIterator(Isolate* isolate, FrameInspector* frame_inspector, // Inner function. info.reset(new ParseInfo(shared_info)); } - if (parsing::ParseAny(info.get()) && Rewriter::Rewrite(info.get(), isolate)) { + if (parsing::ParseAny(info.get(), isolate) && + Rewriter::Rewrite(info.get(), isolate)) { DeclarationScope* scope = info->literal()->scope(); if (!ignore_nested_scopes || collect_non_locals) { CollectNonLocals(info.get(), scope); diff --git a/src/parsing/parser.h b/src/parsing/parser.h index ddaf53f3bd..52907c6c36 100644 --- a/src/parsing/parser.h +++ b/src/parsing/parser.h @@ -231,8 +231,8 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase) { private: friend class ParserBase; friend class v8::internal::ExpressionClassifier>; - friend bool v8::internal::parsing::ParseProgram(ParseInfo*, bool); - friend bool v8::internal::parsing::ParseFunction(ParseInfo*, bool); + friend bool v8::internal::parsing::ParseProgram(ParseInfo*, Isolate*, bool); + friend bool v8::internal::parsing::ParseFunction(ParseInfo*, Isolate*, bool); bool AllowsLazyParsingWithoutUnresolvedVariables() const { return scope()->AllowsLazyParsingWithoutUnresolvedVariables( diff --git a/src/parsing/parsing.cc b/src/parsing/parsing.cc index ede13ac995..b6e0685d60 100644 --- a/src/parsing/parsing.cc +++ b/src/parsing/parsing.cc @@ -15,7 +15,7 @@ namespace v8 { namespace internal { namespace parsing { -bool ParseProgram(ParseInfo* info, bool internalize) { +bool ParseProgram(ParseInfo* info, Isolate* isolate, bool internalize) { DCHECK(info->is_toplevel()); DCHECK_NULL(info->literal()); @@ -24,7 +24,6 @@ bool ParseProgram(ParseInfo* info, bool internalize) { FunctionLiteral* result = nullptr; // Ok to use Isolate here; this function is only called in the main thread. DCHECK(parser.parsing_on_main_thread_); - Isolate* isolate = info->isolate(); parser.SetCachedData(info); result = parser.ParseProgram(isolate, info); @@ -41,7 +40,7 @@ bool ParseProgram(ParseInfo* info, bool internalize) { return (result != nullptr); } -bool ParseFunction(ParseInfo* info, bool internalize) { +bool ParseFunction(ParseInfo* info, Isolate* isolate, bool internalize) { DCHECK(!info->is_toplevel()); DCHECK_NULL(info->literal()); @@ -50,7 +49,6 @@ bool ParseFunction(ParseInfo* info, bool internalize) { FunctionLiteral* result = nullptr; // Ok to use Isolate here; this function is only called in the main thread. DCHECK(parser.parsing_on_main_thread_); - Isolate* isolate = info->isolate(); result = parser.ParseFunction(isolate, info); info->set_literal(result); @@ -64,9 +62,9 @@ bool ParseFunction(ParseInfo* info, bool internalize) { return (result != nullptr); } -bool ParseAny(ParseInfo* info, bool internalize) { - return info->is_toplevel() ? ParseProgram(info, internalize) - : ParseFunction(info, internalize); +bool ParseAny(ParseInfo* info, Isolate* isolate, bool internalize) { + return info->is_toplevel() ? ParseProgram(info, isolate, internalize) + : ParseFunction(info, isolate, internalize); } } // namespace parsing diff --git a/src/parsing/parsing.h b/src/parsing/parsing.h index 3902377e0d..aeac11ceea 100644 --- a/src/parsing/parsing.h +++ b/src/parsing/parsing.h @@ -18,16 +18,19 @@ namespace parsing { // function literal. Returns false (and deallocates any allocated AST // nodes) if parsing failed. Internalizes AST nodes on the heap if // |internalize|. -V8_EXPORT_PRIVATE bool ParseProgram(ParseInfo* info, bool internalize = true); +V8_EXPORT_PRIVATE bool ParseProgram(ParseInfo* info, Isolate* isolate, + bool internalize = true); // Like ParseProgram but for an individual function. Internalizes AST nodes on // the heap if |internalize|. -V8_EXPORT_PRIVATE bool ParseFunction(ParseInfo* info, bool internalize = true); +V8_EXPORT_PRIVATE bool ParseFunction(ParseInfo* info, Isolate* isolate, + bool internalize = true); // If you don't know whether info->is_toplevel() is true or not, use this method // to dispatch to either of the above functions. Prefer to use the above methods // whenever possible. Internalizes AST nodes on the heap if |internalize|. -V8_EXPORT_PRIVATE bool ParseAny(ParseInfo* info, bool internalize = true); +V8_EXPORT_PRIVATE bool ParseAny(ParseInfo* info, Isolate* isolate, + bool internalize = true); } // namespace parsing } // namespace internal diff --git a/src/runtime/runtime-internal.cc b/src/runtime/runtime-internal.cc index 33a3b2f354..8c566c081d 100644 --- a/src/runtime/runtime-internal.cc +++ b/src/runtime/runtime-internal.cc @@ -416,7 +416,7 @@ Handle RenderCallSite(Isolate* isolate, Handle object) { MessageLocation location; if (ComputeLocation(isolate, &location)) { std::unique_ptr info(new ParseInfo(location.shared())); - if (parsing::ParseAny(info.get())) { + if (parsing::ParseAny(info.get(), isolate)) { CallPrinter printer(isolate, location.shared()->IsUserJavaScript()); Handle str = printer.Print(info->literal(), location.start_pos()); if (str->length() > 0) return str; diff --git a/test/cctest/compiler/function-tester.cc b/test/cctest/compiler/function-tester.cc index 0bad07e8c3..4776689995 100644 --- a/test/cctest/compiler/function-tester.cc +++ b/test/cctest/compiler/function-tester.cc @@ -189,7 +189,7 @@ Handle FunctionTester::CompileGraph(Graph* graph) { CompilationInfo info(parse_info.zone(), &parse_info, function->GetIsolate(), function); - CHECK(parsing::ParseFunction(info.parse_info())); + CHECK(parsing::ParseFunction(info.parse_info(), info.isolate())); info.SetOptimizing(); Handle code = Pipeline::GenerateCodeForTesting(&info, graph); diff --git a/test/cctest/compiler/test-loop-assignment-analysis.cc b/test/cctest/compiler/test-loop-assignment-analysis.cc index 7994b2a713..ca506d8d63 100644 --- a/test/cctest/compiler/test-loop-assignment-analysis.cc +++ b/test/cctest/compiler/test-loop-assignment-analysis.cc @@ -37,7 +37,7 @@ struct TestHelper : public HandleAndZoneScope { CompilationInfo info(parse_info.zone(), &parse_info, function->GetIsolate(), function); - CHECK(parsing::ParseFunction(&parse_info)); + CHECK(parsing::ParseFunction(&parse_info, info.isolate())); CHECK(Rewriter::Rewrite(&parse_info, function->GetIsolate())); DeclarationScope::Analyze(&parse_info, info.isolate(), AnalyzeMode::kRegular); diff --git a/test/cctest/parsing/test-preparser.cc b/test/cctest/parsing/test-preparser.cc index 842c0b3da8..2fcff02d95 100644 --- a/test/cctest/parsing/test-preparser.cc +++ b/test/cctest/parsing/test-preparser.cc @@ -595,7 +595,7 @@ TEST(PreParserScopeAnalysis) { // No need to run scope analysis; preparser scope data is produced when // parsing. - CHECK(i::parsing::ParseProgram(&lazy_info)); + CHECK(i::parsing::ParseProgram(&lazy_info, isolate)); // Then parse eagerly and check against the scope data. inner_function = outers[outer_ix].eager_inner; @@ -623,7 +623,7 @@ TEST(PreParserScopeAnalysis) { i::ParseInfo eager_normal(script); eager_normal.set_allow_lazy_parsing(false); - CHECK(i::parsing::ParseProgram(&eager_normal)); + CHECK(i::parsing::ParseProgram(&eager_normal, isolate)); CHECK(i::Compiler::Analyze(&eager_normal, isolate)); i::Scope* normal_scope = @@ -635,7 +635,7 @@ TEST(PreParserScopeAnalysis) { i::ParseInfo eager_using_scope_data(script); eager_using_scope_data.set_allow_lazy_parsing(false); - CHECK(i::parsing::ParseProgram(&eager_using_scope_data)); + CHECK(i::parsing::ParseProgram(&eager_using_scope_data, isolate)); // Don't run scope analysis (that would obviously decide the correct // allocation for the variables). diff --git a/test/cctest/test-parsing.cc b/test/cctest/test-parsing.cc index 2e4296282c..ce0a30bd43 100644 --- a/test/cctest/test-parsing.cc +++ b/test/cctest/test-parsing.cc @@ -811,7 +811,7 @@ TEST(ScopeUsesArgumentsSuperThis) { i::ParseInfo info(script); // The information we're checking is only produced when eager parsing. info.set_allow_lazy_parsing(false); - CHECK(i::parsing::ParseProgram(&info)); + CHECK(i::parsing::ParseProgram(&info, isolate)); CHECK(i::Rewriter::Rewrite(&info, isolate)); i::DeclarationScope::Analyze(&info, isolate, i::AnalyzeMode::kRegular); CHECK(info.literal() != NULL); @@ -1164,7 +1164,7 @@ TEST(ScopePositions) { i::Handle script = factory->NewScript(source); i::ParseInfo info(script); info.set_language_mode(source_data[i].language_mode); - i::parsing::ParseProgram(&info); + i::parsing::ParseProgram(&info, isolate); CHECK_NOT_NULL(info.literal()); // Check scope types and positions. @@ -1209,7 +1209,7 @@ TEST(DiscardFunctionBody) { factory->NewStringFromUtf8(i::CStrVector(source)).ToHandleChecked(); i::Handle script = factory->NewScript(source_code); i::ParseInfo info(script); - i::parsing::ParseProgram(&info); + i::parsing::ParseProgram(&info, isolate); function = info.literal(); CHECK_NOT_NULL(function); CHECK_NOT_NULL(function->body()); @@ -1348,7 +1348,7 @@ void TestParserSyncWithFlags(i::Handle source, info.set_allow_lazy_parsing(flags.Contains(kAllowLazy)); SetGlobalFlags(flags); if (is_module) info.set_module(); - i::parsing::ParseProgram(&info); + i::parsing::ParseProgram(&info, isolate); function = info.literal(); } @@ -2466,7 +2466,7 @@ TEST(DontRegressPreParserDataSizes) { i::ScriptData* sd = NULL; info.set_cached_data(&sd); info.set_compile_options(v8::ScriptCompiler::kProduceParserCache); - i::parsing::ParseProgram(&info); + i::parsing::ParseProgram(&info, CcTest::i_isolate()); i::ParseData* pd = i::ParseData::FromCachedData(sd); if (pd->FunctionCount() != test_cases[i].functions) { @@ -3357,7 +3357,7 @@ TEST(InnerAssignment) { i::Handle f = i::Handle::cast(o); i::Handle shared = i::handle(f->shared()); info = std::unique_ptr(new i::ParseInfo(shared)); - CHECK(i::parsing::ParseFunction(info.get())); + CHECK(i::parsing::ParseFunction(info.get(), isolate)); } else { i::Handle source = factory->InternalizeUtf8String(program.start()); @@ -3366,7 +3366,7 @@ TEST(InnerAssignment) { i::Handle script = factory->NewScript(source); info = std::unique_ptr(new i::ParseInfo(script)); info->set_allow_lazy_parsing(false); - CHECK(i::parsing::ParseProgram(info.get())); + CHECK(i::parsing::ParseProgram(info.get(), isolate)); } CHECK(i::Compiler::Analyze(info.get(), isolate)); CHECK(info->literal() != NULL); @@ -3471,7 +3471,7 @@ TEST(MaybeAssignedParameters) { i::Handle shared = i::handle(f->shared()); info = std::unique_ptr(new i::ParseInfo(shared)); info->set_allow_lazy_parsing(allow_lazy); - CHECK(i::parsing::ParseFunction(info.get())); + CHECK(i::parsing::ParseFunction(info.get(), isolate)); CHECK(i::Compiler::Analyze(info.get(), isolate)); CHECK_NOT_NULL(info->literal()); @@ -3510,7 +3510,7 @@ static void TestMaybeAssigned(Input input, const char* variable, bool module, info->set_module(module); info->set_allow_lazy_parsing(allow_lazy_parsing); - CHECK(i::parsing::ParseProgram(info.get())); + CHECK(i::parsing::ParseProgram(info.get(), isolate)); CHECK(i::Compiler::Analyze(info.get(), isolate)); CHECK_NOT_NULL(info->literal()); @@ -6350,7 +6350,7 @@ TEST(BasicImportExportParsing) { i::Handle script = factory->NewScript(source); i::ParseInfo info(script); info.set_module(); - if (!i::parsing::ParseProgram(&info)) { + if (!i::parsing::ParseProgram(&info, isolate)) { i::Handle exception_handle( i::JSObject::cast(isolate->pending_exception())); i::Handle message_string = i::Handle::cast( @@ -6373,7 +6373,7 @@ TEST(BasicImportExportParsing) { { i::Handle script = factory->NewScript(source); i::ParseInfo info(script); - CHECK(!i::parsing::ParseProgram(&info)); + CHECK(!i::parsing::ParseProgram(&info, isolate)); isolate->clear_pending_exception(); } } @@ -6464,7 +6464,7 @@ TEST(ImportExportParsingErrors) { i::Handle script = factory->NewScript(source); i::ParseInfo info(script); info.set_module(); - CHECK(!i::parsing::ParseProgram(&info)); + CHECK(!i::parsing::ParseProgram(&info, isolate)); isolate->clear_pending_exception(); } } @@ -6500,7 +6500,7 @@ TEST(ModuleTopLevelFunctionDecl) { i::Handle script = factory->NewScript(source); i::ParseInfo info(script); info.set_module(); - CHECK(!i::parsing::ParseProgram(&info)); + CHECK(!i::parsing::ParseProgram(&info, isolate)); isolate->clear_pending_exception(); } } @@ -6697,7 +6697,7 @@ TEST(ModuleParsingInternals) { i::Handle script = factory->NewScript(source); i::ParseInfo info(script); info.set_module(); - CHECK(i::parsing::ParseProgram(&info)); + CHECK(i::parsing::ParseProgram(&info, isolate)); CHECK(i::Compiler::Analyze(&info, isolate)); i::FunctionLiteral* func = info.literal(); i::ModuleScope* module_scope = func->scope()->AsModuleScope(); @@ -6955,7 +6955,7 @@ void TestLanguageMode(const char* source, i::Handle script = factory->NewScript(factory->NewStringFromAsciiChecked(source)); i::ParseInfo info(script); - i::parsing::ParseProgram(&info); + i::parsing::ParseProgram(&info, isolate); CHECK(info.literal() != NULL); CHECK_EQ(expected_language_mode, info.literal()->language_mode()); } @@ -9603,7 +9603,7 @@ TEST(NoPessimisticContextAllocation) { i::Handle script = factory->NewScript(source); i::ParseInfo info(script); - CHECK(i::parsing::ParseProgram(&info)); + CHECK(i::parsing::ParseProgram(&info, isolate)); CHECK(i::Compiler::Analyze(&info, isolate)); CHECK(info.literal() != NULL); diff --git a/test/fuzzer/parser.cc b/test/fuzzer/parser.cc index 2b31bf5aab..e364d83149 100644 --- a/test/fuzzer/parser.cc +++ b/test/fuzzer/parser.cc @@ -36,7 +36,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { v8::internal::Handle script = factory->NewScript(source.ToHandleChecked()); v8::internal::ParseInfo info(script); - v8::internal::parsing::ParseProgram(&info); + v8::internal::parsing::ParseProgram(&info, i_isolate); isolate->RequestGarbageCollectionForTesting( v8::Isolate::kFullGarbageCollection); return 0; diff --git a/test/unittests/compiler-dispatcher/compiler-dispatcher-unittest.cc b/test/unittests/compiler-dispatcher/compiler-dispatcher-unittest.cc index 73c395bf91..384143e0fc 100644 --- a/test/unittests/compiler-dispatcher/compiler-dispatcher-unittest.cc +++ b/test/unittests/compiler-dispatcher/compiler-dispatcher-unittest.cc @@ -926,7 +926,7 @@ TEST_F(CompilerDispatcherTest, CompileParsedOutOfScope) { ASSERT_FALSE(shared->is_compiled()); ParseInfo parse_info(shared); - ASSERT_TRUE(parsing::ParseAny(&parse_info)); + ASSERT_TRUE(parsing::ParseAny(&parse_info, i_isolate())); DeferredHandleScope handles_scope(i_isolate()); { ASSERT_TRUE(Compiler::Analyze(&parse_info, i_isolate())); } std::shared_ptr compilation_handles( @@ -993,7 +993,7 @@ TEST_F(CompilerDispatcherTestWithoutContext, CompileExtensionWithoutContext) { ParseInfo parse_info(script); parse_info.set_extension(&extension); - ASSERT_TRUE(parsing::ParseAny(&parse_info)); + ASSERT_TRUE(parsing::ParseAny(&parse_info, i_isolate())); Handle shared_infos_array(i_isolate()->factory()->NewFixedArray( parse_info.max_function_literal_id() + 1)); parse_info.script()->set_shared_function_infos(*shared_infos_array); diff --git a/tools/parser-shell.cc b/tools/parser-shell.cc index 29f52075cd..7c7da243b5 100644 --- a/tools/parser-shell.cc +++ b/tools/parser-shell.cc @@ -99,7 +99,8 @@ std::pair RunBaselineParser( info.set_compile_options(v8::ScriptCompiler::kProduceParserCache); v8::base::ElapsedTimer timer; timer.Start(); - bool success = parsing::ParseProgram(&info); + bool success = + parsing::ParseProgram(&info, reinterpret_cast(isolate)); parse_time1 = timer.Elapsed(); if (!success) { fprintf(stderr, "Parsing failed\n"); @@ -113,7 +114,8 @@ std::pair RunBaselineParser( info.set_compile_options(v8::ScriptCompiler::kConsumeParserCache); v8::base::ElapsedTimer timer; timer.Start(); - bool success = parsing::ParseProgram(&info); + bool success = + parsing::ParseProgram(&info, reinterpret_cast(isolate)); parse_time2 = timer.Elapsed(); if (!success) { fprintf(stderr, "Parsing failed\n");