From 3e4a1de6e98c1f5a3d8fff92af4a78d1a8dc88c4 Mon Sep 17 00:00:00 2001 From: Jens Johansen Date: Thu, 28 Mar 2019 10:00:51 +0000 Subject: [PATCH] Use dart:foo-patch import uri for patches; remove VM workaround for sdk uris Closes #32087. Change-Id: I93a757125bf29bc283bf6536fa5e53a4f9334891 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/97311 Commit-Queue: Jens Johansen Reviewed-by: Vyacheslav Egorov --- .../lib/src/fasta/source/source_loader.dart | 19 +++- runtime/vm/kernel_loader.cc | 96 +------------------ runtime/vm/kernel_loader.h | 5 - 3 files changed, 17 insertions(+), 103 deletions(-) diff --git a/pkg/front_end/lib/src/fasta/source/source_loader.dart b/pkg/front_end/lib/src/fasta/source/source_loader.dart index 470caa2086fd..8bfe6e310d52 100644 --- a/pkg/front_end/lib/src/fasta/source/source_loader.dart +++ b/pkg/front_end/lib/src/fasta/source/source_loader.dart @@ -203,11 +203,22 @@ class SourceLoader extends Loader { Token token = result.tokens; if (!suppressLexicalErrors) { List source = getSource(bytes); + Uri importUri = library.uri; + if (library.isPatch) { + // For patch files we create a "fake" import uri. + // We cannot use the import uri from the patched libarary because + // several different files would then have the same import uri, + // and the VM does not support that. Also, what would, for instance, + // setting a breakpoint on line 42 of some import uri mean, if the uri + // represented several files? + List newPathSegments = + new List.from(importUri.pathSegments); + newPathSegments.add(library.fileUri.pathSegments.last); + newPathSegments[0] = "${newPathSegments[0]}-patch"; + importUri = importUri.replace(pathSegments: newPathSegments); + } target.addSourceInformation( - library.isPatch ? library.fileUri : library.uri, - library.fileUri, - result.lineStarts, - source); + importUri, library.fileUri, result.lineStarts, source); } while (token is ErrorToken) { if (!suppressLexicalErrors) { diff --git a/runtime/vm/kernel_loader.cc b/runtime/vm/kernel_loader.cc index b0183c20c8d4..183ae42c6e73 100644 --- a/runtime/vm/kernel_loader.cc +++ b/runtime/vm/kernel_loader.cc @@ -1018,11 +1018,9 @@ RawLibrary* KernelLoader::LoadLibrary(intptr_t index) { library.SetLoadInProgress(); } - StringIndex import_uri_index = - H.CanonicalNameString(library_helper.canonical_name_); library_helper.ReadUntilIncluding(LibraryHelper::kSourceUriIndex); - const Script& script = Script::Handle( - Z, ScriptAt(library_helper.source_uri_index_, library, import_uri_index)); + const Script& script = + Script::Handle(Z, ScriptAt(library_helper.source_uri_index_)); library_helper.ReadUntilExcluding(LibraryHelper::kAnnotations); intptr_t annotations_kernel_offset = @@ -1357,80 +1355,6 @@ void KernelLoader::LoadPreliminaryClass(ClassHelper* class_helper, } } -// Workaround for http://dartbug.com/32087: currently Kernel front-end -// embeds absolute build-time paths to core library sources into Kernel -// binaries this introduces discrepancy between how stack traces were -// looked like in legacy pipeline and how they look in Dart 2 pipeline and -// breaks users' code that attempts to pattern match and filter various -// irrelevant frames (e.g. frames from dart:async). -// This also breaks debugging experience in external debuggers because -// debugger attempts to open files that don't exist in the local file -// system. -// To work around this issue we reformat urls of scripts belonging to -// dart:-scheme libraries to look like they looked like in legacy pipeline: -// -// dart:libname/filename.dart -// dart:libname/runtime/lib/filename.dart -// dart:libname/runtime/bin/filename.dart -// -void KernelLoader::FixCoreLibraryScriptUri(const Library& library, - const Script& script) { - struct Helper { - static bool EndsWithCString(const String& haystack, - const char* needle, - intptr_t needle_length, - intptr_t end_pos) { - const intptr_t start = end_pos - needle_length + 1; - if (start >= 0) { - for (intptr_t i = 0; i < needle_length; i++) { - if (haystack.CharAt(start + i) != needle[i]) { - return false; - } - } - return true; - } - return false; - } - }; - - if (library.is_dart_scheme()) { - String& url = String::Handle(zone_, script.url()); - if (!url.StartsWith(Symbols::DartScheme())) { - // Search backwards until '/' is found. That gives us the filename. - // Note: can't use reusable handle in the code below because - // concat also needs it. - intptr_t pos = url.Length() - 1; - while (pos >= 0 && url.CharAt(pos) != '/') { - pos--; - } - - static const char* kRuntimeLib = "runtime/lib/"; - static const intptr_t kRuntimeLibLen = strlen(kRuntimeLib); - const bool inside_runtime_lib = - Helper::EndsWithCString(url, kRuntimeLib, kRuntimeLibLen, pos); - - static const char* kRuntimeBin = "runtime/bin/"; - static const intptr_t kRuntimeBinLen = strlen(kRuntimeBin); - const bool inside_runtime_bin = - Helper::EndsWithCString(url, kRuntimeBin, kRuntimeBinLen, pos); - - String& tmp = String::Handle(zone_); - url = String::SubString(url, pos + 1); - if (inside_runtime_lib) { - tmp = String::New(kRuntimeLib, Heap::kNew); - url = String::Concat(tmp, url); - } else if (inside_runtime_bin) { - tmp = String::New(kRuntimeBin, Heap::kNew); - url = String::Concat(tmp, url); - } - tmp = library.url(); - url = String::Concat(Symbols::Slash(), url); - url = String::Concat(tmp, url); - script.set_url(url); - } - } -} - void KernelLoader::LoadClass(const Library& library, const Class& toplevel_class, intptr_t class_end, @@ -1452,7 +1376,6 @@ void KernelLoader::LoadClass(const Library& library, const Script& script = Script::Handle(Z, ScriptAt(class_helper.source_uri_index_)); out_class->set_script(script); - FixCoreLibraryScriptUri(library, script); } if (out_class->token_pos() == TokenPosition::kNoSource) { class_helper.ReadUntilIncluding(ClassHelper::kStartPosition); @@ -2011,7 +1934,6 @@ const Object& KernelLoader::ClassForScriptAt(const Class& klass, patch_class ^= patch_classes_.At(source_uri_index); if (patch_class.IsNull() || patch_class.origin_class() != klass.raw()) { ASSERT(!library_kernel_data_.IsNull()); - FixCoreLibraryScriptUri(Library::Handle(klass.library()), correct_script); patch_class = PatchClass::New(klass, correct_script); patch_class.set_library_kernel_data(library_kernel_data_); patch_class.set_library_kernel_offset(library_kernel_offset_); @@ -2078,20 +2000,6 @@ RawScript* KernelLoader::LoadScriptAt(intptr_t index, return script.raw(); } -RawScript* KernelLoader::ScriptAt(intptr_t index, - const Library& library, - StringIndex import_uri) { - ASSERT(!library.IsNull()); - const Script& script = - Script::Handle(Z, kernel_program_info_.ScriptAt(index)); - if (library.is_dart_scheme()) { - FixCoreLibraryScriptUri(library, script); - } else if (import_uri != -1) { - script.set_url(H.DartString(import_uri, Heap::kOld)); - } - return script.raw(); -} - void KernelLoader::GenerateFieldAccessors(const Class& klass, const Field& field, FieldHelper* field_helper) { diff --git a/runtime/vm/kernel_loader.h b/runtime/vm/kernel_loader.h index 4e64fcf63597..ea39aac2bcad 100644 --- a/runtime/vm/kernel_loader.h +++ b/runtime/vm/kernel_loader.h @@ -294,8 +294,6 @@ class KernelLoader : public ValueObject { void ReadInferredType(const Field& field, intptr_t kernel_offset); void CheckForInitializer(const Field& field); - void FixCoreLibraryScriptUri(const Library& library, const Script& script); - void LoadClass(const Library& library, const Class& toplevel_class, intptr_t class_end, @@ -327,9 +325,6 @@ class KernelLoader : public ValueObject { RawScript* ScriptAt(intptr_t source_uri_index) { return kernel_program_info_.ScriptAt(source_uri_index); } - RawScript* ScriptAt(intptr_t source_uri_index, - const Library& lib, - StringIndex import_uri); void GenerateFieldAccessors(const Class& klass, const Field& field,