From ec48e8f323d68717cd3fed94abd85d4f82ea1ce7 Mon Sep 17 00:00:00 2001 From: Nicholas Shahan Date: Mon, 3 May 2021 17:54:19 +0000 Subject: [PATCH] [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 Reviewed-by: Sigmund Cherem Reviewed-by: Mark Zhou --- pkg/dev_compiler/lib/src/kernel/compiler.dart | 62 +++++++++++++------ .../language/generic/regress_45767_test.dart | 25 ++++++++ 2 files changed, 68 insertions(+), 19 deletions(-) create mode 100644 tests/language/generic/regress_45767_test.dart diff --git a/pkg/dev_compiler/lib/src/kernel/compiler.dart b/pkg/dev_compiler/lib/src/kernel/compiler.dart index 791ada248dc..b687d8cf507 100644 --- a/pkg/dev_compiler/lib/src/kernel/compiler.dart +++ b/pkg/dev_compiler/lib/src/kernel/compiler.dart @@ -94,6 +94,9 @@ class ProgramCompiler extends ComputeOnceConstantVisitor /// 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 @@ class ProgramCompiler extends ComputeOnceConstantVisitor 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 @@ class ProgramCompiler extends ComputeOnceConstantVisitor 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 @@ class ProgramCompiler extends ComputeOnceConstantVisitor 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 @@ class ProgramCompiler extends ComputeOnceConstantVisitor 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 @@ class ProgramCompiler extends ComputeOnceConstantVisitor 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 @@ class ProgramCompiler extends ComputeOnceConstantVisitor _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 00000000000..953c966404d --- /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 {} + +class F> extends I {} + +// Extending F forces DDC to defer the superclass and trigger the bug +class A extends F> {} + +class B extends F> {} + +void main() { + Expect.isTrue(A() is I>); + Expect.equals(!hasSoundNullSafety, A() is I>); + Expect.isTrue(B() is I>); + Expect.equals(!hasSoundNullSafety, B() is I>); +}