Fix deferred type literals and typedefs
Closes https://github.com/dart-lang/sdk/issues/33890
Change-Id: I3a154d4de46747f3ad905de889f14188fdfff50a
Reviewed-on: https://dart-review.googlesource.com/65442
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Sigmund Cherem <sigmund@google.com>
diff --git a/pkg/compiler/lib/src/deferred_load.dart b/pkg/compiler/lib/src/deferred_load.dart
index 8b87744..e72eca9 100644
--- a/pkg/compiler/lib/src/deferred_load.dart
+++ b/pkg/compiler/lib/src/deferred_load.dart
@@ -15,6 +15,7 @@
ConstantValue,
ConstructedConstantValue,
DeferredConstantValue,
+ TypeConstantValue,
DeferredGlobalConstantValue,
InstantiationConstantValue;
import 'elements/types.dart';
@@ -193,6 +194,10 @@
Iterable<ImportEntity> memberImportsTo(
MemberEntity element, LibraryEntity library);
+ /// Returns every [ImportEntity] that imports [element] into [library].
+ Iterable<ImportEntity> typedefImportsTo(
+ TypedefEntity element, LibraryEntity library);
+
/// Collects all direct dependencies of [element].
///
/// The collected dependent elements and constants are are added to
@@ -410,18 +415,9 @@
}
}
constant.getDependencies().forEach((ConstantValue dependency) {
- if (dependency is DeferredConstantValue) {
- /// New deferred-imports are only discovered when we are visiting the
- /// main output unit (size == 0) or code reachable from a deferred
- /// import (size == 1). After that, we are rediscovering the
- /// same nodes we have already seen.
- if (newSet.length <= 1) {
- queue.addConstant(
- dependency, importSets.singleton(dependency.import));
- }
- } else {
- _updateConstantRecursive(dependency, oldSet, newSet, queue);
- }
+ // Constants are not allowed to refer to deferred constants, so
+ // no need to check for a deferred type literal here.
+ _updateConstantRecursive(dependency, oldSet, newSet, queue);
});
} else {
assert(
@@ -505,16 +501,21 @@
// dependencies are already visited as dependencies of the enclosing member.
}
+ /// Whether to enqueue a deferred dependency.
+ ///
+ /// Due to the nature of the algorithm, some dependencies may be visited more
+ /// than once. However, we know that new deferred-imports are only discovered
+ /// when we are visiting the main output unit (size == 0) or code reachable
+ /// from a deferred import (size == 1). After that, we are rediscovering the
+ /// same nodes we have already seen.
+ _shouldAddDeferredDependency(ImportSet newSet) => newSet.length <= 1;
+
void _processDependencies(LibraryEntity library, Dependencies dependencies,
ImportSet oldSet, ImportSet newSet, WorkQueue queue) {
for (ClassEntity cls in dependencies.classes) {
Iterable<ImportEntity> imports = classImportsTo(cls, library);
if (_isExplicitlyDeferred(imports)) {
- /// New deferred-imports are only discovered when we are visiting the
- /// main output unit (size == 0) or code reachable from a deferred
- /// import (size == 1). After that, we are rediscovering the
- /// same nodes we have already seen.
- if (newSet.length <= 1) {
+ if (_shouldAddDeferredDependency(newSet)) {
for (ImportEntity deferredImport in imports) {
queue.addClass(cls, importSets.singleton(deferredImport));
}
@@ -527,11 +528,7 @@
for (MemberEntity member in dependencies.members) {
Iterable<ImportEntity> imports = memberImportsTo(member, library);
if (_isExplicitlyDeferred(imports)) {
- /// New deferred-imports are only discovered when we are visiting the
- /// main output unit (size == 0) or code reachable from a deferred
- /// import (size == 1). After that, we are rediscovering the
- /// same nodes we have already seen.
- if (newSet.length <= 1) {
+ if (_shouldAddDeferredDependency(newSet)) {
for (ImportEntity deferredImport in imports) {
queue.addMember(member, importSets.singleton(deferredImport));
}
@@ -546,14 +543,36 @@
}
for (ConstantValue dependency in dependencies.constants) {
+ // TODO(sigmund): either delete DeferredConstantValue or use it to
+ // represent deferred TypeConstantValues below.
if (dependency is DeferredConstantValue) {
- if (newSet.length <= 1) {
+ if (_shouldAddDeferredDependency(newSet)) {
queue.addConstant(
dependency, importSets.singleton(dependency.import));
}
- } else {
- _updateConstantRecursive(dependency, oldSet, newSet, queue);
+ continue;
}
+
+ if (dependency is TypeConstantValue) {
+ var type = dependency.representedType;
+ var imports = const <ImportEntity>[];
+ if (type is InterfaceType) {
+ imports = classImportsTo(type.element, library);
+ } else if (type is TypedefType) {
+ imports = typedefImportsTo(type.element, library);
+ }
+ if (_isExplicitlyDeferred(imports)) {
+ if (_shouldAddDeferredDependency(newSet)) {
+ for (ImportEntity deferredImport in imports) {
+ queue.addConstant(
+ dependency, importSets.singleton(deferredImport));
+ }
+ }
+ continue;
+ }
+ }
+
+ _updateConstantRecursive(dependency, oldSet, newSet, queue);
}
}
diff --git a/pkg/compiler/lib/src/kernel/deferred_load.dart b/pkg/compiler/lib/src/kernel/deferred_load.dart
index b312027..7b0a987 100644
--- a/pkg/compiler/lib/src/kernel/deferred_load.dart
+++ b/pkg/compiler/lib/src/kernel/deferred_load.dart
@@ -48,6 +48,13 @@
}
@override
+ Iterable<ImportEntity> typedefImportsTo(
+ TypedefEntity element, LibraryEntity library) {
+ ir.Typedef node = _elementMap.getTypedefNode(element);
+ return _findImportsTo(node, node.name, node.enclosingLibrary, library);
+ }
+
+ @override
Iterable<ImportEntity> memberImportsTo(
Entity element, LibraryEntity library) {
ir.Member node = _elementMap.getMemberDefinition(element).node;
diff --git a/pkg/compiler/lib/src/kernel/element_map.dart b/pkg/compiler/lib/src/kernel/element_map.dart
index 7be52d1..23a83dd 100644
--- a/pkg/compiler/lib/src/kernel/element_map.dart
+++ b/pkg/compiler/lib/src/kernel/element_map.dart
@@ -192,6 +192,9 @@
/// Returns the [ir.Library] corresponding to [library].
ir.Library getLibraryNode(LibraryEntity library);
+ /// Returns the node that defines [typedef].
+ ir.Typedef getTypedefNode(covariant TypedefEntity typedef);
+
/// Returns the definition information for [member].
MemberDefinition getMemberDefinition(covariant MemberEntity member);
diff --git a/pkg/compiler/lib/src/kernel/element_map_impl.dart b/pkg/compiler/lib/src/kernel/element_map_impl.dart
index cdbfe78..524c487 100644
--- a/pkg/compiler/lib/src/kernel/element_map_impl.dart
+++ b/pkg/compiler/lib/src/kernel/element_map_impl.dart
@@ -844,6 +844,10 @@
return _classes.getData(cls).definition;
}
+ ir.Typedef _getTypedefNode(covariant IndexedTypedef typedef) {
+ return _typedefs.getData(typedef).node;
+ }
+
@override
ImportEntity getImport(ir.LibraryDependency node) {
ir.Library library = node.parent;
@@ -1439,6 +1443,11 @@
return _getClassDefinition(cls);
}
+ @override
+ ir.Typedef getTypedefNode(TypedefEntity typedef) {
+ return _getTypedefNode(typedef);
+ }
+
/// Returns the element type of a async/sync*/async* function.
@override
DartType getFunctionAsyncOrSyncStarElementType(ir.FunctionNode functionNode) {
diff --git a/tests/compiler/dart2js_extra/deferred/type_literal_lib.dart b/tests/compiler/dart2js_extra/deferred/type_literal_lib.dart
new file mode 100644
index 0000000..867c399
--- /dev/null
+++ b/tests/compiler/dart2js_extra/deferred/type_literal_lib.dart
@@ -0,0 +1,7 @@
+// 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 file.
+
+class A {}
+
+class B {}
diff --git a/tests/compiler/dart2js_extra/deferred/type_literal_test.dart b/tests/compiler/dart2js_extra/deferred/type_literal_test.dart
new file mode 100644
index 0000000..d578d8d
--- /dev/null
+++ b/tests/compiler/dart2js_extra/deferred/type_literal_test.dart
@@ -0,0 +1,21 @@
+// 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 file.
+
+// Regression test for Issue #33890.
+//
+// This fails if type literal constants are not deferred, but their RTI
+// representation is. This test however will not detect if we accidentally start
+// building the RTI representation in the main deferred unit (which is what was
+// happening before the bug was introduced).
+
+import 'type_literal_lib.dart' deferred as a;
+import 'package:expect/expect.dart';
+
+main() async {
+ await a.loadLibrary();
+ Expect.isFalse(confuse(a.A) == confuse(a.B));
+}
+
+@NoInline()
+confuse(x) => x;
diff --git a/tests/compiler/dart2js_extra/deferred/typedef_lib.dart b/tests/compiler/dart2js_extra/deferred/typedef_lib.dart
new file mode 100644
index 0000000..2bccab5
--- /dev/null
+++ b/tests/compiler/dart2js_extra/deferred/typedef_lib.dart
@@ -0,0 +1,10 @@
+// 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 file.
+
+class A {}
+
+class B {}
+
+typedef C(A a);
+typedef D(B a);
diff --git a/tests/compiler/dart2js_extra/deferred/typedef_test.dart b/tests/compiler/dart2js_extra/deferred/typedef_test.dart
new file mode 100644
index 0000000..1ad34a3
--- /dev/null
+++ b/tests/compiler/dart2js_extra/deferred/typedef_test.dart
@@ -0,0 +1,23 @@
+// 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 file.
+
+// Regression test for Issue #33890.
+//
+// This fails if type literal constants are not deferred, but their RTI
+// representation is. This test however will not detect if we accidentally start
+// building the RTI representation in the main deferred unit (which is what was
+// happening before the bug was introduced).
+
+import 'typedef_lib.dart' deferred as a;
+import 'package:expect/expect.dart';
+
+main() async {
+ await a.loadLibrary();
+ print(a.A); // unclear why without this the definition of A and B gets pulled
+ print(a.B); // into the main output unit.
+ Expect.isFalse(confuse(a.C) == confuse(a.D));
+}
+
+@NoInline()
+confuse(x) => x;
