Only one source of truth for uriToSource
Previously we had several places where different uriToSource was used.
This was both weird and led to errors when something refering to
some state was used to look up in another state.
This CL makes it so we only have one source of truth.
To not include sdk sources when serializing a component that only mixes
something from the sdk in, (or extends it or...), the serialization is
changed slightly to keep track of which uris come from actual
implementation. Before the sdk sources was explicitly removed in the
incremental compiler, but we want to limit those kinds of things,
which is why we're doing it differently here.
Fixes #35215.
Bug: 35215
Change-Id: Iaa5618fcb0ea42b13aba7720f34a87a85144e047
Reviewed-on: https://dart-review.googlesource.com/c/85175
Reviewed-by: Peter von der Ahé <ahe@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
diff --git a/pkg/front_end/lib/src/fasta/dill/dill_loader.dart b/pkg/front_end/lib/src/fasta/dill/dill_loader.dart
index d033d7d..2a4eb8d 100644
--- a/pkg/front_end/lib/src/fasta/dill/dill_loader.dart
+++ b/pkg/front_end/lib/src/fasta/dill/dill_loader.dart
@@ -11,6 +11,8 @@
import '../fasta_codes.dart'
show SummaryTemplate, Template, templateDillOutlineSummary;
+import '../compiler_context.dart' show CompilerContext;
+
import '../kernel/kernel_builder.dart' show LibraryBuilder;
import '../loader.dart' show Loader;
@@ -26,9 +28,11 @@
final libraries = <Library>[];
/// Sources for all appended components.
- final Map<Uri, Source> uriToSource = <Uri, Source>{};
+ final Map<Uri, Source> uriToSource;
- DillLoader(TargetImplementation target) : super(target);
+ DillLoader(TargetImplementation target)
+ : uriToSource = CompilerContext.current.uriToSource,
+ super(target);
Template<SummaryTemplate> get outlineSummaryTemplate =>
templateDillOutlineSummary;
diff --git a/pkg/front_end/lib/src/fasta/get_dependencies.dart b/pkg/front_end/lib/src/fasta/get_dependencies.dart
index e4aacb2..abd9879 100644
--- a/pkg/front_end/lib/src/fasta/get_dependencies.dart
+++ b/pkg/front_end/lib/src/fasta/get_dependencies.dart
@@ -49,9 +49,8 @@
var platformComponent = loadComponentFromBytes(bytes);
dillTarget.loader.appendLibraries(platformComponent);
}
- KernelTarget kernelTarget = new KernelTarget(
- fileSystem, false, dillTarget, uriTranslator,
- uriToSource: c.uriToSource);
+ KernelTarget kernelTarget =
+ new KernelTarget(fileSystem, false, dillTarget, uriTranslator);
kernelTarget.setEntryPoints(<Uri>[script]);
await dillTarget.buildOutlines();
diff --git a/pkg/front_end/lib/src/fasta/incremental_compiler.dart b/pkg/front_end/lib/src/fasta/incremental_compiler.dart
index c4d089a..2ba7eb7 100644
--- a/pkg/front_end/lib/src/fasta/incremental_compiler.dart
+++ b/pkg/front_end/lib/src/fasta/incremental_compiler.dart
@@ -81,7 +81,6 @@
Set<Uri> invalidatedUris = new Set<Uri>();
DillTarget dillLoadedData;
- Map<Uri, Source> dillLoadedDataUriToSource = <Uri, Source>{};
List<LibraryBuilder> platformBuilders;
Map<Uri, LibraryBuilder> userBuilders;
final Uri initializeFromDillUri;
@@ -206,7 +205,6 @@
for (Uri uri in new Set<Uri>.from(dillLoadedData.loader.builders.keys)
..removeAll(reusedLibraryUris)) {
dillLoadedData.loader.builders.remove(uri);
- dillLoadedDataUriToSource.remove(uri);
userBuilders?.remove(uri);
}
@@ -234,8 +232,7 @@
c.fileSystem),
false,
dillLoadedData,
- uriTranslator,
- uriToSource: c.uriToSource);
+ uriTranslator);
userCode.loader.hierarchy = hierarchy;
for (LibraryBuilder library in reusedLibraries) {
@@ -265,9 +262,6 @@
List<Library> compiledLibraries =
new List<Library>.from(userCode.loader.libraries);
- Map<Uri, Source> uriToSource =
- new Map<Uri, Source>.from(dillLoadedDataUriToSource);
- uriToSource.addAll(userCode.uriToSource);
Procedure mainMethod = componentWithDill == null
? data.userLoadedUriMain
: componentWithDill.mainMethod;
@@ -287,13 +281,10 @@
userCode = userCodeOld;
}
- Map<Uri, Source> optionalUriToSource = context.options.embedSourceText
- ? uriToSource
- : uriToSource.map((uri, source) => MapEntry<Uri, Source>(
- uri, new Source(source.lineStarts, const <int>[])));
// This is the incremental component.
return context.options.target.configureComponent(new Component(
- libraries: outputLibraries, uriToSource: optionalUriToSource))
+ libraries: outputLibraries,
+ uriToSource: componentWithDill.uriToSource))
..mainMethod = mainMethod;
});
}
@@ -347,7 +338,6 @@
Library lib = builder.target;
removedLibraries.add(lib);
dillLoadedData.loader.builders.remove(uri);
- dillLoadedDataUriToSource.remove(uri);
userBuilders?.remove(uri);
}
}
@@ -387,8 +377,6 @@
if (initializationBytes != null) {
ticker.logMs("Read $initializeFromDillUri");
- Set<Uri> sdkUris = data.component.uriToSource.keys.toSet();
-
// We're going to output all we read here so lazy loading it
// doesn't make sense.
new BinaryBuilder(initializationBytes, disableLazyReading: true)
@@ -412,10 +400,6 @@
bytesLength += initializationBytes.length;
data.userLoadedUriMain = data.component.mainMethod;
data.includeUserLoadedLibraries = true;
- for (Uri uri in data.component.uriToSource.keys) {
- if (sdkUris.contains(uri)) continue;
- dillLoadedDataUriToSource[uri] = data.component.uriToSource[uri];
- }
}
}
return bytesLength;
diff --git a/pkg/front_end/lib/src/fasta/kernel/kernel_target.dart b/pkg/front_end/lib/src/fasta/kernel/kernel_target.dart
index 1342714..ee564d1 100644
--- a/pkg/front_end/lib/src/fasta/kernel/kernel_target.dart
+++ b/pkg/front_end/lib/src/fasta/kernel/kernel_target.dart
@@ -133,9 +133,9 @@
KernelTarget(this.fileSystem, this.includeComments, DillTarget dillTarget,
UriTranslator uriTranslator,
- {Map<Uri, Source> uriToSource, MetadataCollector metadataCollector})
+ {MetadataCollector metadataCollector})
: dillTarget = dillTarget,
- uriToSource = uriToSource ?? CompilerContext.current.uriToSource,
+ uriToSource = CompilerContext.current.uriToSource,
metadataCollector = metadataCollector,
super(dillTarget.ticker, uriTranslator, dillTarget.backendTarget) {
loader = createLoader();
@@ -350,7 +350,6 @@
}
this.uriToSource.forEach(copySource);
- dillTarget.loader.uriToSource.forEach(copySource);
Component component = CompilerContext.current.options.target
.configureComponent(new Component(
diff --git a/pkg/front_end/lib/src/fasta/testing/kernel_chain.dart b/pkg/front_end/lib/src/fasta/testing/kernel_chain.dart
index 5e2944d..fbfe897 100644
--- a/pkg/front_end/lib/src/fasta/testing/kernel_chain.dart
+++ b/pkg/front_end/lib/src/fasta/testing/kernel_chain.dart
@@ -128,7 +128,8 @@
Future<Result<Component>> run(Component component, dynamic context) async {
StringBuffer messages = context.componentToDiagnostics[component];
- Uri uri = component.uriToSource.keys.first;
+ Uri uri = component.uriToSource.keys
+ .firstWhere((uri) => uri != null && uri.scheme == "file");
Library library = component.libraries
.firstWhere((Library library) => library.importUri.scheme != "dart");
Uri base = uri.resolve(".");
diff --git a/pkg/front_end/test/incremental_dart2js_load_from_dill_test.dart b/pkg/front_end/test/incremental_dart2js_load_from_dill_test.dart
index ea0190d..9ae979a 100644
--- a/pkg/front_end/test/incremental_dart2js_load_from_dill_test.dart
+++ b/pkg/front_end/test/incremental_dart2js_load_from_dill_test.dart
@@ -6,6 +6,10 @@
import 'dart:io' show Directory, File;
import 'package:expect/expect.dart' show Expect;
+import 'package:front_end/src/compute_platform_binaries_location.dart'
+ show computePlatformBinariesLocation;
+import 'package:kernel/binary/ast_from_binary.dart' show BinaryBuilder;
+import 'package:kernel/kernel.dart' show Component;
import 'incremental_load_from_dill_test.dart'
show normalCompile, initializedCompile, checkIsEqual;
@@ -35,6 +39,30 @@
Stopwatch stopwatch = new Stopwatch()..start();
await normalCompile(dart2jsUrl, normalDill);
print("Normal compile took ${stopwatch.elapsedMilliseconds} ms");
+ {
+ // Check that we don't include the source from files from the sdk.
+ final Uri sdkRoot = computePlatformBinariesLocation(forceBuildDir: true);
+ Uri platformUri = sdkRoot.resolve("vm_platform.dill");
+ Component cSdk = new Component();
+ new BinaryBuilder(new File.fromUri(platformUri).readAsBytesSync(),
+ disableLazyReading: false)
+ .readComponent(cSdk);
+
+ Component c = new Component();
+ new BinaryBuilder(new File.fromUri(normalDill).readAsBytesSync(),
+ disableLazyReading: false)
+ .readComponent(c);
+ for (Uri uri in c.uriToSource.keys) {
+ if (cSdk.uriToSource.containsKey(uri)) {
+ if ((c.uriToSource[uri].source?.length ?? 0) != 0) {
+ throw "Compile contained sources for the sdk $uri";
+ }
+ if ((c.uriToSource[uri].lineStarts?.length ?? 0) != 0) {
+ throw "Compile contained line starts for the sdk $uri";
+ }
+ }
+ }
+ }
// Compile dart2js, initializing from the just-compiled dill,
// a nonexisting file and a dill file that isn't valid.
diff --git a/pkg/front_end/testcases/incremental_initialize_from_dill/regress_35215.yaml b/pkg/front_end/testcases/incremental_initialize_from_dill/regress_35215.yaml
new file mode 100644
index 0000000..5fd397a
--- /dev/null
+++ b/pkg/front_end/testcases/incremental_initialize_from_dill/regress_35215.yaml
@@ -0,0 +1,44 @@
+# Copyright (c) 2018, the Dart project authors. Please see the AUTHORS file
+# for details. All rights reserved. Use of this source code is governed by a
+# BSD-style license that can be found in the LICENSE.md file.
+
+# Load from a dill file, update a file in the dill that is the context of an
+# error in such a way that the position in the newly compiled procedure doesn't
+# exist in the old library.
+
+type: newworld
+strong: true
+worlds:
+ - entry: main.dart
+ sources:
+ main.dart: |
+ import "b.dart";
+ main() {
+ b(42);
+ }
+ b.dart: |
+ b({int named}) {
+ print("b");
+ }
+ expectedLibraryCount: 2
+ errors: true
+ - entry: main.dart
+ worldTypex: updated
+ invalidate:
+ - b.dart
+ sources:
+ main.dart: |
+ import "b.dart";
+ main() {
+ b(42);
+ }
+ b.dart: |
+ // lots of comments
+ // forcing offsets down
+ // and also adding more lines
+ // and whatnot
+ b({int named}) {
+ print("b");
+ }
+ expectedLibraryCount: 2
+ errors: true
diff --git a/pkg/front_end/tool/_fasta/entry_points.dart b/pkg/front_end/tool/_fasta/entry_points.dart
index bd473b2..22d2418 100644
--- a/pkg/front_end/tool/_fasta/entry_points.dart
+++ b/pkg/front_end/tool/_fasta/entry_points.dart
@@ -227,8 +227,7 @@
KernelTarget createKernelTarget(
DillTarget dillTarget, UriTranslator uriTranslator) {
- return new KernelTarget(c.fileSystem, false, dillTarget, uriTranslator,
- uriToSource: c.uriToSource);
+ return new KernelTarget(c.fileSystem, false, dillTarget, uriTranslator);
}
Future<KernelTarget> buildOutline([Uri output]) async {
diff --git a/pkg/kernel/lib/ast.dart b/pkg/kernel/lib/ast.dart
index 7555396..3006953 100644
--- a/pkg/kernel/lib/ast.dart
+++ b/pkg/kernel/lib/ast.dart
@@ -5695,7 +5695,7 @@
/// number. The returned line contains no line separators.
String getTextLine(int line) {
RangeError.checkValueInInterval(line, 1, lineStarts.length, 'line');
- if (source == null) return null;
+ if (source == null || source.isEmpty) return null;
cachedText ??= utf8.decode(source, allowMalformed: true);
// -1 as line numbers start at 1.
diff --git a/pkg/kernel/lib/binary/ast_to_binary.dart b/pkg/kernel/lib/binary/ast_to_binary.dart
index b46a768..33d8da4 100644
--- a/pkg/kernel/lib/binary/ast_to_binary.dart
+++ b/pkg/kernel/lib/binary/ast_to_binary.dart
@@ -23,6 +23,8 @@
final StringIndexer stringIndexer;
ConstantIndexer _constantIndexer;
final UriIndexer _sourceUriIndexer = new UriIndexer();
+ bool _currentlyInNonimplementation = false;
+ final List<bool> _sourcesFromRealImplementation = new List<bool>();
final Set<Uri> _knownSourceUri = new Set<Uri>();
Map<LibraryDependency, int> _libraryDependencyIndex =
<LibraryDependency, int>{};
@@ -226,6 +228,12 @@
Uri writeUriReference(Uri uri) {
final int index = _sourceUriIndexer.put(uri);
writeUInt30(index);
+ if (!_currentlyInNonimplementation) {
+ if (_sourcesFromRealImplementation.length <= index) {
+ _sourcesFromRealImplementation.length = index + 1;
+ }
+ _sourcesFromRealImplementation[index] = true;
+ }
return uri;
}
@@ -503,9 +511,12 @@
Utf8Encoder utf8Encoder = const Utf8Encoder();
for (Uri uri in _sourceUriIndexer.index.keys) {
index[i] = getBufferOffset();
- Source source =
- (_knownSourceUri.contains(uri) ? uriToSource[uri] : null) ??
- new Source(<int>[], const <int>[]);
+ Source source = ((_knownSourceUri.contains(uri) &&
+ _sourcesFromRealImplementation.length > i &&
+ _sourcesFromRealImplementation[i] == true)
+ ? uriToSource[uri]
+ : null) ??
+ new Source(<int>[], const <int>[]);
writeByteList(utf8Encoder.convert(uri == null ? "" : "$uri"));
writeByteList(source.source);
@@ -753,6 +764,8 @@
void visitClass(Class node) {
classOffsets.add(getBufferOffset());
+ if (node.isAnonymousMixin) _currentlyInNonimplementation = true;
+
int flags = _encodeClassFlags(node.flags, node.level);
if (node.canonicalName == null) {
throw 'Missing canonical name for $node';
@@ -791,6 +804,7 @@
writeUInt32(offset);
}
writeUInt32(procedureOffsets.length - 1);
+ _currentlyInNonimplementation = false;
}
static final Name _emptyName = new Name('');
@@ -834,6 +848,13 @@
if (node.canonicalName == null) {
throw 'Missing canonical name for $node';
}
+
+ final bool currentlyInNonimplementationSaved =
+ _currentlyInNonimplementation;
+ if (node.isNoSuchMethodForwarder || node.isSyntheticForwarder) {
+ _currentlyInNonimplementation = true;
+ }
+
enterScope(memberScope: true);
writeByte(Tag.Procedure);
writeCanonicalNameReference(getCanonicalNameOfMember(node));
@@ -856,6 +877,7 @@
leaveScope(memberScope: true);
+ _currentlyInNonimplementation = currentlyInNonimplementationSaved;
assert((node.forwardingStubSuperTarget != null) ||
!(node.isForwardingStub && node.function.body != null));
}
