From 8b67fcbbd1a665dc561124ff57ffc3ec514ae02e Mon Sep 17 00:00:00 2001 From: Alexander Markov Date: Fri, 1 Nov 2019 19:08:42 +0000 Subject: [PATCH] [vm] Cleanup script tags VM creates Script objects with kKernelTag only, so this CL cleans up all uses of script tags and Script::kind() along with code which is no longer reachable. Change-Id: Ia765e7757264aa614e18eddd16d21937f855c129 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/123889 Reviewed-by: Ryan Macnak Commit-Queue: Alexander Markov --- runtime/lib/mirrors.cc | 6 +- runtime/vm/clustered_snapshot.cc | 4 +- .../vm/compiler/frontend/bytecode_reader.cc | 6 +- runtime/vm/debugger.cc | 3 - runtime/vm/exceptions.cc | 6 +- runtime/vm/kernel_loader.cc | 5 +- runtime/vm/object.cc | 144 +++++------------- runtime/vm/object.h | 15 +- runtime/vm/object_service.cc | 2 +- runtime/vm/object_test.cc | 3 +- runtime/vm/profiler_test.cc | 12 +- runtime/vm/raw_object.h | 14 +- runtime/vm/unit_test.cc | 3 +- 13 files changed, 63 insertions(+), 160 deletions(-) diff --git a/runtime/lib/mirrors.cc b/runtime/lib/mirrors.cc index e4e976a6ea5..0b97abb2981 100644 --- a/runtime/lib/mirrors.cc +++ b/runtime/lib/mirrors.cc @@ -1697,10 +1697,8 @@ DEFINE_NATIVE_ENTRY(DeclarationMirror_location, 0, 1) { return Instance::null(); // No source. } const Array& scripts = Array::Handle(zone, lib.LoadedScripts()); - for (intptr_t i = 0; i < scripts.Length(); i++) { - script ^= scripts.At(i); - if (script.kind() == RawScript::kLibraryTag) break; - } + ASSERT(scripts.Length() > 0); + script ^= scripts.At(scripts.Length() - 1); ASSERT(!script.IsNull()); const String& uri = String::Handle(zone, script.url()); return CreateSourceLocation(uri, 1, 1); diff --git a/runtime/vm/clustered_snapshot.cc b/runtime/vm/clustered_snapshot.cc index 17a946a653f..2c75b067916 100644 --- a/runtime/vm/clustered_snapshot.cc +++ b/runtime/vm/clustered_snapshot.cc @@ -1100,7 +1100,7 @@ class ScriptSerializationCluster : public SerializationCluster { WriteFromTo(script); s->Write(script->ptr()->line_offset_); s->Write(script->ptr()->col_offset_); - s->Write(script->ptr()->kind_and_tags_); + s->Write(script->ptr()->flags_); s->Write(script->ptr()->kernel_script_index_); } } @@ -1133,7 +1133,7 @@ class ScriptDeserializationCluster : public DeserializationCluster { ReadFromTo(script); script->ptr()->line_offset_ = d->Read(); script->ptr()->col_offset_ = d->Read(); - script->ptr()->kind_and_tags_ = d->Read(); + script->ptr()->flags_ = d->Read(); script->ptr()->kernel_script_index_ = d->Read(); script->ptr()->load_timestamp_ = 0; } diff --git a/runtime/vm/compiler/frontend/bytecode_reader.cc b/runtime/vm/compiler/frontend/bytecode_reader.cc index 91d2f3fbca3..f4b8122532d 100644 --- a/runtime/vm/compiler/frontend/bytecode_reader.cc +++ b/runtime/vm/compiler/frontend/bytecode_reader.cc @@ -1540,7 +1540,7 @@ RawObject* BytecodeReaderHelper::ReadObjectContents(uint32_t header) { ReadSourceFile(uri, bytecode_component_->GetSourceFilesOffset() + reader_.ReadUInt()); } else { - script = Script::New(uri, Object::null_string(), RawScript::kKernelTag); + script = Script::New(uri, Object::null_string()); } script.set_kernel_program_info(H.GetKernelProgramInfo()); return script.raw(); @@ -1910,8 +1910,8 @@ RawScript* BytecodeReaderHelper::ReadSourceFile(const String& uri, source = ReadString(/* is_canonical = */ false); } - const Script& script = Script::Handle( - Z, Script::New(import_uri, uri, source, RawScript::kKernelTag)); + const Script& script = + Script::Handle(Z, Script::New(import_uri, uri, source)); script.set_line_starts(line_starts); if (source.IsNull() && line_starts.IsNull()) { diff --git a/runtime/vm/debugger.cc b/runtime/vm/debugger.cc index a8f1db07e1b..e49bd50a1a6 100644 --- a/runtime/vm/debugger.cc +++ b/runtime/vm/debugger.cc @@ -1053,7 +1053,6 @@ void ActivationFrame::ExtractTokenPositionFromAsyncClosure() { } ASSERT(!IsInterpreted()); - ASSERT(script.kind() == RawScript::kKernelTag); const intptr_t await_jump_var = GetAwaitJumpVariable(); if (await_jump_var < 0) { return; @@ -3063,7 +3062,6 @@ TokenPosition Debugger::ResolveBreakpointPos(bool in_bytecode, const TokenPosition begin_pos = best_fit_pos; TokenPosition end_of_line_pos; - ASSERT(script.kind() == RawScript::kKernelTag); if (best_line == -1) { script.GetTokenLocation(begin_pos, &best_line, NULL); } @@ -4379,7 +4377,6 @@ bool Debugger::IsAtAsyncJump(ActivationFrame* top_frame) { } ASSERT(!top_frame->IsInterpreted()); const Script& script = Script::Handle(zone, top_frame->SourceScript()); - ASSERT(script.kind() == RawScript::kKernelTag); const auto& yield_positions = GrowableObjectArray::Handle( zone, script.GetYieldPositions(top_frame->function())); // No yield statements diff --git a/runtime/vm/exceptions.cc b/runtime/vm/exceptions.cc index 1723edcc4b4..761ea025ca1 100644 --- a/runtime/vm/exceptions.cc +++ b/runtime/vm/exceptions.cc @@ -822,11 +822,7 @@ void Exceptions::CreateAndThrowTypeError(TokenPosition location, intptr_t column = -1; ASSERT(!script.IsNull()); if (location.IsReal()) { - if (script.HasSource() || script.kind() == RawScript::kKernelTag) { - script.GetTokenLocation(location, &line, &column); - } else { - script.GetTokenLocation(location, &line, NULL); - } + script.GetTokenLocation(location, &line, &column); } // Initialize '_url', '_line', and '_column' arguments. args.SetAt(0, String::Handle(zone, script.url())); diff --git a/runtime/vm/kernel_loader.cc b/runtime/vm/kernel_loader.cc index 3ab242d20d4..28d4ffe96b1 100644 --- a/runtime/vm/kernel_loader.cc +++ b/runtime/vm/kernel_loader.cc @@ -2059,7 +2059,7 @@ RawScript* KernelLoader::LoadScriptAt(intptr_t index, for (intptr_t i = 0; i < libs.Length(); i++) { lib ^= libs.At(i); script = lib.LookupScript(uri_string, /* useResolvedUri = */ true); - if (!script.IsNull() && script.kind() == RawScript::kKernelTag) { + if (!script.IsNull()) { sources = script.Source(); line_starts = script.line_starts(); break; @@ -2071,8 +2071,7 @@ RawScript* KernelLoader::LoadScriptAt(intptr_t index, } const Script& script = - Script::Handle(Z, Script::New(import_uri_string, uri_string, sources, - RawScript::kKernelTag)); + Script::Handle(Z, Script::New(import_uri_string, uri_string, sources)); script.set_kernel_script_index(index); script.set_kernel_program_info(kernel_program_info_); script.set_line_starts(line_starts); diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc index 4b1984c0e62..7e8508a4bcf 100644 --- a/runtime/vm/object.cc +++ b/runtime/vm/object.cc @@ -8067,43 +8067,36 @@ RawString* Function::GetSource() const { Zone* zone = Thread::Current()->zone(); const Script& func_script = Script::Handle(zone, script()); - if (func_script.kind() == RawScript::kKernelTag) { - intptr_t from_line; - intptr_t from_col; - intptr_t to_line; - intptr_t to_col; - intptr_t to_length; - func_script.GetTokenLocation(token_pos(), &from_line, &from_col); - func_script.GetTokenLocation(end_token_pos(), &to_line, &to_col, - &to_length); + intptr_t from_line; + intptr_t from_col; + intptr_t to_line; + intptr_t to_col; + intptr_t to_length; + func_script.GetTokenLocation(token_pos(), &from_line, &from_col); + func_script.GetTokenLocation(end_token_pos(), &to_line, &to_col, &to_length); - if (to_length == 1) { - // Handle special cases for end tokens of closures (where we exclude the - // last token): - // (1) "foo(() => null, bar);": End token is `,', but we don't print it. - // (2) "foo(() => null);": End token is ')`, but we don't print it. - // (3) "var foo = () => null;": End token is `;', but in this case the - // token semicolon belongs to the assignment so we skip it. - const String& src = String::Handle(func_script.Source()); - if (src.IsNull() || src.Length() == 0) { - return Symbols::OptimizedOut().raw(); - } - uint16_t end_char = src.CharAt(end_token_pos().value()); - if ((end_char == ',') || // Case 1. - (end_char == ')') || // Case 2. - (end_char == ';' && - String::Handle(zone, name()) - .Equals(""))) { // Case 3. - to_length = 0; - } + if (to_length == 1) { + // Handle special cases for end tokens of closures (where we exclude the + // last token): + // (1) "foo(() => null, bar);": End token is `,', but we don't print it. + // (2) "foo(() => null);": End token is ')`, but we don't print it. + // (3) "var foo = () => null;": End token is `;', but in this case the + // token semicolon belongs to the assignment so we skip it. + const String& src = String::Handle(func_script.Source()); + if (src.IsNull() || src.Length() == 0) { + return Symbols::OptimizedOut().raw(); + } + uint16_t end_char = src.CharAt(end_token_pos().value()); + if ((end_char == ',') || // Case 1. + (end_char == ')') || // Case 2. + (end_char == ';' && String::Handle(zone, name()) + .Equals(""))) { // Case 3. + to_length = 0; } - - return func_script.GetSnippet(from_line, from_col, to_line, - to_col + to_length); } - UNREACHABLE(); - return String::null(); + return func_script.GetSnippet(from_line, from_col, to_line, + to_col + to_length); } // Construct fingerprint from token stream. The token stream contains also @@ -9518,7 +9511,7 @@ void Script::LookupSourceAndLineStarts(Zone* zone) const { for (intptr_t i = 0; i < libs.Length(); i++) { lib ^= libs.At(i); script = lib.LookupScript(uri, /* useResolvedUri = */ true); - if (!script.IsNull() && script.kind() == RawScript::kKernelTag) { + if (!script.IsNull()) { const auto& source = String::Handle(zone, script.Source()); const auto& line_starts = TypedData::Handle(zone, script.line_starts()); if (!source.IsNull() || !line_starts.IsNull()) { @@ -9534,7 +9527,6 @@ void Script::LookupSourceAndLineStarts(Zone* zone) const { } RawGrowableObjectArray* Script::GenerateLineNumberArray() const { - ASSERT(kind() == RawScript::kKernelTag); Zone* zone = Thread::Current()->zone(); const GrowableObjectArray& info = GrowableObjectArray::Handle(zone, GrowableObjectArray::New()); @@ -9589,25 +9581,6 @@ RawGrowableObjectArray* Script::GenerateLineNumberArray() const { return info.raw(); } -const char* Script::GetKindAsCString() const { - switch (kind()) { - case RawScript::kScriptTag: - return "script"; - case RawScript::kLibraryTag: - return "library"; - case RawScript::kSourceTag: - return "source"; - case RawScript::kEvaluateTag: - return "evaluate"; - case RawScript::kKernelTag: - return "kernel"; - default: - UNIMPLEMENTED(); - } - UNREACHABLE(); - return NULL; -} - void Script::set_url(const String& value) const { StorePointer(&raw_ptr()->url_, value.raw()); } @@ -9662,7 +9635,7 @@ RawTypedData* Script::line_starts() const { RawArray* Script::debug_positions() const { #if !defined(DART_PRECOMPILED_RUNTIME) Array& debug_positions_array = Array::Handle(raw_ptr()->debug_positions_); - if (debug_positions_array.IsNull() && kind() == RawScript::kKernelTag) { + if (debug_positions_array.IsNull()) { // This is created lazily. Now we need it. kernel::CollectTokenPositionsFor(*this); } @@ -9670,23 +9643,17 @@ RawArray* Script::debug_positions() const { return raw_ptr()->debug_positions_; } -void Script::set_kind(RawScript::Kind value) const { - set_kind_and_tags( - RawScript::KindBits::update(value, raw_ptr()->kind_and_tags_)); -} - -void Script::set_kind_and_tags(uint8_t value) const { - StoreNonPointer(&raw_ptr()->kind_and_tags_, value); +void Script::set_flags(uint8_t value) const { + StoreNonPointer(&raw_ptr()->flags_, value); } void Script::SetLazyLookupSourceAndLineStarts(bool value) const { - set_kind_and_tags(RawScript::LazyLookupSourceAndLineStartsBit::update( - value, raw_ptr()->kind_and_tags_)); + set_flags(RawScript::LazyLookupSourceAndLineStartsBit::update( + value, raw_ptr()->flags_)); } bool Script::IsLazyLookupSourceAndLineStarts() const { - return RawScript::LazyLookupSourceAndLineStartsBit::decode( - raw_ptr()->kind_and_tags_); + return RawScript::LazyLookupSourceAndLineStartsBit::decode(raw_ptr()->flags_); } void Script::set_load_timestamp(int64_t value) const { @@ -9715,31 +9682,12 @@ intptr_t Script::GetTokenLineUsingLineStarts( return 0; } - if (kind() == RawScript::kKernelTag) { #if !defined(DART_PRECOMPILED_RUNTIME) - kernel::KernelLineStartsReader line_starts_reader(line_starts_data, zone); - return line_starts_reader.LineNumberForPosition(target_token_pos.value()); + kernel::KernelLineStartsReader line_starts_reader(line_starts_data, zone); + return line_starts_reader.LineNumberForPosition(target_token_pos.value()); #else - return 0; + return 0; #endif // !defined(DART_PRECOMPILED_RUNTIME) - } else { - ASSERT(line_starts_data.Length() > 0); - intptr_t offset = target_token_pos.Pos(); - intptr_t min = 0; - intptr_t max = line_starts_data.Length() - 1; - - // Binary search to find the line containing this offset. - while (min < max) { - int midpoint = (max - min + 1) / 2 + min; - int32_t token_pos = line_starts_data.GetInt32(midpoint * 4); - if (token_pos > offset) { - max = midpoint - 1; - } else { - min = midpoint; - } - } - return min + 1; // Line numbers start at 1. - } } #if !defined(DART_PRECOMPILED_RUNTIME) @@ -9767,7 +9715,6 @@ void Script::GetTokenLocation(TokenPosition token_pos, ASSERT(line != NULL); Zone* zone = Thread::Current()->zone(); - ASSERT(kind() == RawScript::kKernelTag); LookupSourceAndLineStarts(zone); if (line_starts() == TypedData::null()) { // Scripts in the AOT snapshot do not have a line starts array. @@ -9806,7 +9753,6 @@ void Script::GetTokenLocation(TokenPosition token_pos, void Script::TokenRangeAtLine(intptr_t line_number, TokenPosition* first_token_index, TokenPosition* last_token_index) const { - ASSERT(kind() == RawScript::kKernelTag); ASSERT(first_token_index != NULL && last_token_index != NULL); ASSERT(line_number > 0); @@ -9944,16 +9890,13 @@ RawScript* Script::New() { return reinterpret_cast(raw); } -RawScript* Script::New(const String& url, - const String& source, - RawScript::Kind kind) { - return Script::New(url, url, source, kind); +RawScript* Script::New(const String& url, const String& source) { + return Script::New(url, url, source); } RawScript* Script::New(const String& url, const String& resolved_url, - const String& source, - RawScript::Kind kind) { + const String& source) { Thread* thread = Thread::Current(); Zone* zone = thread->zone(); const Script& result = Script::Handle(zone, Script::New()); @@ -9962,8 +9905,7 @@ RawScript* Script::New(const String& url, String::Handle(zone, Symbols::New(thread, resolved_url))); result.set_source(source); result.SetLocationOffset(0, 0); - result.set_kind_and_tags(0); - result.set_kind(kind); + result.set_flags(0); result.set_kernel_script_index(0); result.set_load_timestamp( FLAG_remove_script_timestamps_for_test ? 0 : OS::GetCurrentTimeMillis()); @@ -22009,11 +21951,7 @@ static void PrintStackTraceFrame(Zone* zone, line = token_pos.value(); } else { if (!script.IsNull() && token_pos.IsSourcePosition()) { - if (script.HasSource() || script.kind() == RawScript::kKernelTag) { - script.GetTokenLocation(token_pos.SourcePosition(), &line, &column); - } else { - script.GetTokenLocation(token_pos.SourcePosition(), &line, NULL); - } + script.GetTokenLocation(token_pos.SourcePosition(), &line, &column); } } diff --git a/runtime/vm/object.h b/runtime/vm/object.h index 60ea7d29107..069b1a5ce90 100644 --- a/runtime/vm/object.h +++ b/runtime/vm/object.h @@ -3904,11 +3904,6 @@ class Script : public Object { void LookupSourceAndLineStarts(Zone* zone) const; RawGrowableObjectArray* GenerateLineNumberArray() const; - RawScript::Kind kind() const { - return RawScript::KindBits::decode(raw_ptr()->kind_and_tags_); - } - - const char* GetKindAsCString() const; intptr_t line_offset() const { return raw_ptr()->line_offset_; } intptr_t col_offset() const { return raw_ptr()->col_offset_; } @@ -3973,14 +3968,11 @@ class Script : public Object { return RoundedAllocationSize(sizeof(RawScript)); } - static RawScript* New(const String& url, - const String& source, - RawScript::Kind kind); + static RawScript* New(const String& url, const String& source); static RawScript* New(const String& url, const String& resolved_url, - const String& source, - RawScript::Kind kind); + const String& source); #if !defined(DART_PRECOMPILED_RUNTIME) void LoadSourceFromKernel(const uint8_t* kernel_buffer, @@ -3993,8 +3985,7 @@ class Script : public Object { private: void set_resolved_url(const String& value) const; void set_source(const String& value) const; - void set_kind(RawScript::Kind value) const; - void set_kind_and_tags(uint8_t value) const; + void set_flags(uint8_t value) const; void set_load_timestamp(int64_t value) const; RawArray* debug_positions() const; diff --git a/runtime/vm/object_service.cc b/runtime/vm/object_service.cc index 02c90ac9dc3..768588869f3 100644 --- a/runtime/vm/object_service.cc +++ b/runtime/vm/object_service.cc @@ -426,7 +426,7 @@ void Script::PrintJSONImpl(JSONStream* stream, bool ref) const { lib_id.ToCString(), encoded_uri, load_timestamp()); } jsobj.AddPropertyStr("uri", uri); - jsobj.AddProperty("_kind", GetKindAsCString()); + jsobj.AddProperty("_kind", "kernel"); if (ref) { return; } diff --git a/runtime/vm/object_test.cc b/runtime/vm/object_test.cc index f748ed2d8e3..41a77bcb7c9 100644 --- a/runtime/vm/object_test.cc +++ b/runtime/vm/object_test.cc @@ -2262,8 +2262,7 @@ ISOLATE_UNIT_TEST_CASE(Script) { const char* source_chars = "This will not compile."; const String& url = String::Handle(String::New(url_chars)); const String& source = String::Handle(String::New(source_chars)); - const Script& script = - Script::Handle(Script::New(url, source, RawScript::kScriptTag)); + const Script& script = Script::Handle(Script::New(url, source)); EXPECT(!script.IsNull()); EXPECT(script.IsScript()); String& str = String::Handle(script.url()); diff --git a/runtime/vm/profiler_test.cc b/runtime/vm/profiler_test.cc index e39faf01858..d067f5f1752 100644 --- a/runtime/vm/profiler_test.cc +++ b/runtime/vm/profiler_test.cc @@ -290,14 +290,10 @@ class ProfileStackWalker { token_pos = token_pos.FromSynthetic(); } - String& str = String::Handle(zone); - if (script.kind() == RawScript::kKernelTag) { - intptr_t line = 0, column = 0, token_len = 0; - script.GetTokenLocation(token_pos, &line, &column, &token_len); - str = script.GetSnippet(line, column, line, column + token_len); - } else { - UNREACHABLE(); - } + intptr_t line = 0, column = 0, token_len = 0; + script.GetTokenLocation(token_pos, &line, &column, &token_len); + const auto& str = String::Handle( + zone, script.GetSnippet(line, column, line, column + token_len)); return str.IsNull() ? NULL : str.ToCString(); } diff --git a/runtime/vm/raw_object.h b/runtime/vm/raw_object.h index f0970bbe93d..d3dd8b91504 100644 --- a/runtime/vm/raw_object.h +++ b/runtime/vm/raw_object.h @@ -1139,17 +1139,8 @@ class RawField : public RawObject { class RawScript : public RawObject { public: - enum Kind { - kScriptTag = 0, - kLibraryTag, - kSourceTag, - kEvaluateTag, - kKernelTag, - }; enum { - kKindPos = 0, - kKindSize = 3, - kLazyLookupSourceAndLineStartsPos = kKindPos + kKindSize, + kLazyLookupSourceAndLineStartsPos = 0, kLazyLookupSourceAndLineStartsSize = 1, }; @@ -1185,13 +1176,12 @@ class RawScript : public RawObject { int32_t line_offset_; int32_t col_offset_; - using KindBits = BitField; using LazyLookupSourceAndLineStartsBit = BitField; - uint8_t kind_and_tags_; + uint8_t flags_; intptr_t kernel_script_index_; int64_t load_timestamp_; diff --git a/runtime/vm/unit_test.cc b/runtime/vm/unit_test.cc index 67aa62db646..66addfe2e16 100644 --- a/runtime/vm/unit_test.cc +++ b/runtime/vm/unit_test.cc @@ -699,8 +699,7 @@ void AssemblerTest::Assemble() { // assembler instructions that do runtime calls. const char* kDummyScript = "assembler_test_dummy_function() {}"; const Script& script = Script::Handle( - Script::New(function_name, String::Handle(String::New(kDummyScript)), - RawScript::kSourceTag)); + Script::New(function_name, String::Handle(String::New(kDummyScript)))); const Library& lib = Library::Handle(Library::CoreLibrary()); const Class& cls = Class::ZoneHandle( Class::New(lib, function_name, script, TokenPosition::kMinSource));