[ddc] Fix missing nullability on deferred types
Emits legacy and nullable wrappers to the types that appear in
circular hierarchies.
There is still missing nullability information if FutureOr appears
in the type hierarchy but that fix uncovers a larger issue with the
FutureOr type. See https://github.com/dart-lang/sdk/issues/45870.
Change-Id: If5894eaff632c5a961f1316d8803032fae2a0ec5
Fixes: https://github.com/dart-lang/sdk/issues/45767
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/196600
Commit-Queue: Nicholas Shahan <nshahan@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Reviewed-by: Mark Zhou <markzipan@google.com>
diff --git a/pkg/dev_compiler/lib/src/kernel/compiler.dart b/pkg/dev_compiler/lib/src/kernel/compiler.dart
index a01a3b9..84bfd17 100644
--- a/pkg/dev_compiler/lib/src/kernel/compiler.dart
+++ b/pkg/dev_compiler/lib/src/kernel/compiler.dart
@@ -94,6 +94,9 @@
/// The class that is emitting its signature information, otherwise null.
Class _classEmittingSignatures;
+ /// True when a class is emitting a deferred class hierarchy.
+ bool _emittingDeferredType = false;
+
/// The current element being loaded.
/// We can use this to determine if we're loading top-level code or not:
///
@@ -859,22 +862,38 @@
return;
}
- js_ast.Expression emitDeferredType(DartType t) {
- assert(isKnownDartTypeImplementor(t));
- if (t is InterfaceType) {
- _declareBeforeUse(t.classNode);
- if (t.typeArguments.isNotEmpty) {
- return _emitGenericClassType(
- t, t.typeArguments.map(emitDeferredType));
+ js_ast.Expression emitDeferredType(DartType t,
+ {bool emitNullability = true}) {
+ js_ast.Expression _emitDeferredType(DartType t,
+ {bool emitNullability = true}) {
+ if (t is InterfaceType) {
+ _declareBeforeUse(t.classNode);
+ if (t.typeArguments.isNotEmpty) {
+ var typeRep = _emitGenericClassType(
+ t, t.typeArguments.map(_emitDeferredType));
+ return emitNullability
+ ? _emitNullabilityWrapper(typeRep, t.declaredNullability)
+ : typeRep;
+ }
+ return _emitInterfaceType(t, emitNullability: emitNullability);
+ } else if (t is FutureOrType) {
+ _declareBeforeUse(_coreTypes.deprecatedFutureOrClass);
+ // TODO(45870) Add nullability wrappers to FutureOr.
+ return _emitFutureOrTypeWithArgument(
+ _emitDeferredType(t.typeArgument));
+ } else if (t is TypeParameterType) {
+ return _emitTypeParameterType(t, emitNullability: emitNullability);
}
- return _emitInterfaceType(t, emitNullability: false);
- } else if (t is FutureOrType) {
- _declareBeforeUse(_coreTypes.deprecatedFutureOrClass);
- return _emitFutureOrTypeWithArgument(emitDeferredType(t.typeArgument));
- } else if (t is TypeParameterType) {
- return _emitTypeParameterType(t, emitNullability: false);
+ return _emitType(t);
}
- return _emitType(t);
+
+ assert(isKnownDartTypeImplementor(t));
+ var savedEmittingDeferredType = _emittingDeferredType;
+ _emittingDeferredType = true;
+ var deferredClassRep =
+ _emitDeferredType(t, emitNullability: emitNullability);
+ _emittingDeferredType = savedEmittingDeferredType;
+ return deferredClassRep;
}
bool shouldDefer(InterfaceType t) {
@@ -921,7 +940,8 @@
js_ast.Expression getBaseClass(int count) {
var base = emitDeferredType(
- c.getThisType(_coreTypes, c.enclosingLibrary.nonNullable));
+ c.getThisType(_coreTypes, c.enclosingLibrary.nonNullable),
+ emitNullability: false);
while (--count >= 0) {
base = js.call('#.__proto__', [base]);
}
@@ -995,7 +1015,7 @@
var originalSupertype = supertype;
deferredSupertypes.add(() => runtimeStatement('setBaseClass(#, #)', [
getBaseClass(isMixinAliasClass(c) ? 0 : mixinApplications.length),
- emitDeferredType(originalSupertype),
+ emitDeferredType(originalSupertype, emitNullability: false),
]));
// Refers to 'supertype' without type parameters. We remove these from
// the 'extends' clause for generics for cyclic dependencies and append
@@ -1015,7 +1035,9 @@
var m = c.mixedInType.asInterfaceType;
var deferMixin = shouldDefer(m);
- var mixinClass = deferMixin ? emitDeferredType(m) : emitClassRef(m);
+ var mixinClass = deferMixin
+ ? emitDeferredType(m, emitNullability: false)
+ : emitClassRef(m);
var classExpr = deferMixin ? getBaseClass(0) : className;
var mixinApplication =
@@ -1087,7 +1109,7 @@
if (shouldDefer(mixinType)) {
deferredSupertypes.add(() => runtimeStatement('applyMixin(#, #)', [
getBaseClass(mixinApplications.length - i),
- emitDeferredType(mixinType)
+ emitDeferredType(mixinType, emitNullability: false)
]));
} else {
body.add(runtimeStatement(
@@ -2909,7 +2931,9 @@
_currentClass != null && identical(_currentClass, _classEmittingExtends);
bool get _cacheTypes =>
- !_emittingClassExtends && !_emittingClassSignatures ||
+ !_emittingDeferredType &&
+ !_emittingClassExtends &&
+ !_emittingClassSignatures ||
_currentFunction != null;
js_ast.Expression _emitGenericClassType(
diff --git a/tests/language/generic/regress_45767_test.dart b/tests/language/generic/regress_45767_test.dart
new file mode 100644
index 0000000..953c966
--- /dev/null
+++ b/tests/language/generic/regress_45767_test.dart
@@ -0,0 +1,25 @@
+// Copyright (c) 2021, 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.
+
+import 'dart:async';
+
+import 'package:expect/expect.dart';
+
+// Regression test for https://github.com/dart-lang/sdk/issues/45767.
+
+class I<T> {}
+
+class F<E extends F<E>> extends I<E> {}
+
+// Extending F forces DDC to defer the superclass and trigger the bug
+class A<T> extends F<A<Object?>> {}
+
+class B<T> extends F<B<T?>> {}
+
+void main() {
+ Expect.isTrue(A<bool>() is I<A<Object?>>);
+ Expect.equals(!hasSoundNullSafety, A<bool>() is I<A<Object>>);
+ Expect.isTrue(B<bool>() is I<B<bool?>>);
+ Expect.equals(!hasSoundNullSafety, B<bool>() is I<B<bool>>);
+}
