Split CanonicalNameError, no warning if CanonicalNameSdkError
Prior to this CL, we would issue a warning whenever a CanonicalNameError
was encountered.
This is in principal a good thing, but because we currently have no way
to detect if the sdk we get is the one we expect
(by any other measure than when it issues a CanonicalNameError) we often
issue these warnings for no "real reason" whenever, for instance,
the flutter sdk changes.
This CL splits the CanonicalNameError in two such that errors with
references to the sdk ("dart:" libraries) issue CanonicalNameSdkError
instead, an we then handle that differently. Namely we silently ignore
the error (i.e. don't issue a warning) and just don't initialize from
dill.
This should remedy the situation and be strictly better than to always
swallow CanonicalNameErrors.
Bug: 36032
Change-Id: Idbae0b5ee5b9843a5dbeb49b3c65ae25f5962e36
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/105240
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Kevin Millikin <kmillikin@google.com>
diff --git a/pkg/front_end/lib/src/fasta/incremental_compiler.dart b/pkg/front_end/lib/src/fasta/incremental_compiler.dart
index 99cba9b..f2bd798 100644
--- a/pkg/front_end/lib/src/fasta/incremental_compiler.dart
+++ b/pkg/front_end/lib/src/fasta/incremental_compiler.dart
@@ -9,7 +9,11 @@
import 'package:kernel/binary/ast_from_binary.dart' show BinaryBuilder;
import 'package:kernel/binary/ast_from_binary.dart'
- show BinaryBuilder, CanonicalNameError, InvalidKernelVersionError;
+ show
+ BinaryBuilder,
+ CanonicalNameError,
+ CanonicalNameSdkError,
+ InvalidKernelVersionError;
import 'package:kernel/class_hierarchy.dart' show ClassHierarchy;
@@ -161,7 +165,9 @@
bytesLength =
prepareSummary(summaryBytes, uriTranslator, c, data);
- if (e is InvalidKernelVersionError || e is PackageChangedError) {
+ if (e is InvalidKernelVersionError ||
+ e is PackageChangedError ||
+ e is CanonicalNameSdkError) {
// Don't report any warning.
} else {
Uri gzInitializedFrom;
diff --git a/pkg/front_end/test/incremental_load_from_invalid_dill_test.dart b/pkg/front_end/test/incremental_load_from_invalid_dill_test.dart
index 9dc2941..0d9a84e 100644
--- a/pkg/front_end/test/incremental_load_from_invalid_dill_test.dart
+++ b/pkg/front_end/test/incremental_load_from_invalid_dill_test.dart
@@ -40,7 +40,7 @@
import 'package:front_end/src/fasta/severity.dart' show Severity;
-import 'package:kernel/kernel.dart' show Component;
+import 'package:kernel/kernel.dart' show Component, Library;
import 'incremental_load_from_dill_test.dart' show getOptions;
@@ -56,24 +56,26 @@
Uri sdkSummary;
Uri initializeFrom;
Uri helperFile;
+ Uri helper2File;
Uri entryPoint;
+ Uri entryPointImportDartFoo;
Uri platformUri;
List<int> sdkSummaryData;
List<DiagnosticMessage> errorMessages;
List<DiagnosticMessage> warningMessages;
MemoryFileSystem fs;
CompilerOptions options;
+ IncrementalCompiler compiler;
- compileExpectInitializeFailAndSpecificWarning(
+ void compileExpectInitializeFailAndSpecificWarning(
Code expectedWarningCode, bool writeFileOnCrashReport) async {
errorMessages.clear();
warningMessages.clear();
options.writeFileOnCrashReport = writeFileOnCrashReport;
- DeleteTempFilesIncrementalCompiler compiler =
- new DeleteTempFilesIncrementalCompiler(
- new CompilerContext(
- new ProcessedOptions(options: options, inputs: [entryPoint])),
- initializeFrom);
+ compiler = new DeleteTempFilesIncrementalCompiler(
+ new CompilerContext(
+ new ProcessedOptions(options: options, inputs: [entryPoint])),
+ initializeFrom);
await compiler.computeDelta();
if (compiler.initializedFromDill) {
Expect.fail("Expected to not be able to initialized from dill, but did.");
@@ -91,13 +93,40 @@
}
}
+ Future<Component> compileExpectOk(
+ bool initializedFromDill, Uri compileThis) async {
+ errorMessages.clear();
+ warningMessages.clear();
+ options.writeFileOnCrashReport = false;
+ compiler = new DeleteTempFilesIncrementalCompiler(
+ new CompilerContext(
+ new ProcessedOptions(options: options, inputs: [compileThis])),
+ initializeFrom);
+ Component component = await compiler.computeDelta();
+
+ if (compiler.initializedFromDill != initializedFromDill) {
+ Expect.fail("Expected initializedFromDill to be $initializedFromDill "
+ "but was ${compiler.initializedFromDill}");
+ }
+ if (errorMessages.isNotEmpty) {
+ Expect.fail("Got unexpected errors: " + joinMessages(errorMessages));
+ }
+ if (warningMessages.isNotEmpty) {
+ Expect.fail("Got unexpected warnings: " + joinMessages(warningMessages));
+ }
+
+ return component;
+ }
+
initialize() async {
sdkRoot = computePlatformBinariesLocation(forceBuildDir: true);
base = Uri.parse("org-dartlang-test:///");
sdkSummary = base.resolve("vm_platform.dill");
initializeFrom = base.resolve("initializeFrom.dill");
helperFile = base.resolve("helper.dart");
+ helper2File = base.resolve("helper2.dart");
entryPoint = base.resolve("small.dart");
+ entryPointImportDartFoo = base.resolve("small_foo.dart");
platformUri = sdkRoot.resolve("vm_platform_strong.dill");
sdkSummaryData = await new File.fromUri(platformUri).readAsBytes();
errorMessages = <DiagnosticMessage>[];
@@ -108,6 +137,7 @@
options.fileSystem = fs;
options.sdkRoot = null;
options.sdkSummary = sdkSummary;
+ options.omitPlatform = true;
options.onDiagnostic = (DiagnosticMessage message) {
if (message.severity == Severity.error) {
errorMessages.add(message);
@@ -122,16 +152,27 @@
print("hello from foo");
}
""");
+ fs.entityForUri(helper2File).writeAsStringSync("""
+foo2() {
+ print("hello from foo2");
+}
+""");
fs.entityForUri(entryPoint).writeAsStringSync("""
import "helper.dart" as helper;
main() {
helper.foo();
}
""");
+ fs.entityForUri(entryPointImportDartFoo).writeAsStringSync("""
+import "dart:foo" as helper;
+main() {
+ helper.foo2();
+}
+""");
}
Future<Null> test() async {
- IncrementalCompiler compiler = new IncrementalCompiler(
+ compiler = new IncrementalCompiler(
new CompilerContext(
new ProcessedOptions(options: options, inputs: [entryPoint])),
initializeFrom);
@@ -140,23 +181,32 @@
List<int> dataGood = serializeComponent(componentGood);
fs.entityForUri(initializeFrom).writeAsBytesSync(dataGood);
- // Initialize from good dill file should be ok.
+ // Create fake "dart:foo" library.
+ options.omitPlatform = false;
compiler = new IncrementalCompiler(
new CompilerContext(
- new ProcessedOptions(options: options, inputs: [entryPoint])),
+ new ProcessedOptions(options: options, inputs: [helper2File])),
initializeFrom);
- compiler.invalidate(entryPoint);
- Component component = await compiler.computeDelta();
- if (!compiler.initializedFromDill) {
- Expect.fail(
- "Expected to have sucessfully initialized from dill, but didn't.");
+ Component componentHelper = await compiler.computeDelta();
+ Library helper2Lib = componentHelper.libraries
+ .firstWhere((lib) => lib.importUri == helper2File);
+ helper2Lib.importUri = new Uri(scheme: "dart", path: "foo");
+ List<int> sdkWithDartFoo = serializeComponent(componentHelper);
+ options.omitPlatform = true;
+
+ // Compile with our fake sdk with dart:foo should be ok.
+ List<int> orgSdkBytes = await fs.entityForUri(sdkSummary).readAsBytes();
+ fs.entityForUri(sdkSummary).writeAsBytesSync(sdkWithDartFoo);
+ Component component = await compileExpectOk(true, entryPointImportDartFoo);
+ fs.entityForUri(sdkSummary).writeAsBytesSync(orgSdkBytes);
+ if (component.libraries.length != 1) {
+ Expect.fail("Expected 1 library, got ${component.libraries.length}: "
+ "${component.libraries}");
}
- if (errorMessages.isNotEmpty) {
- Expect.fail("Got unexpected errors: " + joinMessages(errorMessages));
- }
- if (warningMessages.isNotEmpty) {
- Expect.fail("Got unexpected warnings: " + joinMessages(warningMessages));
- }
+ List<int> dataLinkedToSdkWithFoo = serializeComponent(component);
+
+ // Initialize from good dill file should be ok.
+ await compileExpectOk(true, entryPoint);
// Create a partial dill file.
compiler.invalidate(entryPoint);
@@ -181,6 +231,11 @@
codeInitializeFromDillUnknownProblem, true);
await compileExpectInitializeFailAndSpecificWarning(
codeInitializeFromDillUnknownProblemNoDump, false);
+
+ // Create a dill with a reference to a non-existing sdk thing:
+ // Should be ok (for now), but we shouldn't actually initialize from dill.
+ fs.entityForUri(initializeFrom).writeAsBytesSync(dataLinkedToSdkWithFoo);
+ await compileExpectOk(false, entryPoint);
}
}
diff --git a/pkg/kernel/lib/binary/ast_from_binary.dart b/pkg/kernel/lib/binary/ast_from_binary.dart
index 31c621d..905363a 100644
--- a/pkg/kernel/lib/binary/ast_from_binary.dart
+++ b/pkg/kernel/lib/binary/ast_from_binary.dart
@@ -39,6 +39,10 @@
CanonicalNameError(this.message);
}
+class CanonicalNameSdkError extends CanonicalNameError {
+ CanonicalNameSdkError(String message) : super(message);
+}
+
class _ComponentIndex {
static const numberOfFixedFields = 9;
@@ -528,8 +532,8 @@
// OK then.
checkReferenceNode = false;
} else {
- throw new CanonicalNameError(
- "Null reference (${child.name}) ($child).");
+ throw buildCanonicalNameError(
+ "Null reference (${child.name}) ($child).", child);
}
}
if (checkReferenceNode) {
@@ -538,8 +542,8 @@
"Canonical name and reference doesn't agree.");
}
if (child.reference.node == null) {
- throw new CanonicalNameError(
- "Reference is null (${child.name}) ($child).");
+ throw buildCanonicalNameError(
+ "Reference is null (${child.name}) ($child).", child);
}
}
}
@@ -548,6 +552,17 @@
}
}
+ CanonicalNameError buildCanonicalNameError(
+ String message, CanonicalName problemNode) {
+ // Special-case missing sdk entries as that is probably a change to the
+ // platform - that's something we might want to react differently to.
+ String libraryUri = problemNode?.nonRootTop?.name ?? "";
+ if (libraryUri.startsWith("dart:")) {
+ return new CanonicalNameSdkError(message);
+ }
+ return new CanonicalNameError(message);
+ }
+
_ComponentIndex _readComponentIndex(int componentFileSize) {
int savedByteIndex = _byteOffset;
