Skip to content

Commit 10bbfbe

Browse files
johnniwintherCommit Bot
authored and
Commit Bot
committed
[cfe] Handle type dependencies through SynthesizedSourceConstructorBuilder
The types of forwarding constructor parameters are required for super parameters. In order to handle the dependency correct and computed the right substitutions through the inheritance chain, SynthesizedSourceConstructorBuilder now use inferFormalTypes to trigger copying of inferred parameter types. Closes #48708 Change-Id: I365ea5ea1fe5de0ac2422d01878b7f1eac421d24 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/239427 Reviewed-by: Chloe Stefantsova <[email protected]> Commit-Queue: Chloe Stefantsova <[email protected]>
1 parent 3feede1 commit 10bbfbe

15 files changed

+333
-33
lines changed

pkg/front_end/lib/src/fasta/kernel/kernel_helper.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,11 +250,13 @@ class TypeDependency {
250250
final Member original;
251251
final Substitution substitution;
252252
final bool copyReturnType;
253+
bool _hasBeenInferred = false;
253254

254255
TypeDependency(this.synthesized, this.original, this.substitution,
255256
{required this.copyReturnType});
256257

257258
void copyInferred() {
259+
if (_hasBeenInferred) return;
258260
for (int i = 0; i < original.function!.positionalParameters.length; i++) {
259261
VariableDeclaration synthesizedParameter =
260262
synthesized.function!.positionalParameters[i];
@@ -275,5 +277,6 @@ class TypeDependency {
275277
synthesized.function!.returnType =
276278
substitution.substituteType(original.function!.returnType);
277279
}
280+
_hasBeenInferred = true;
278281
}
279282
}

pkg/front_end/lib/src/fasta/kernel/kernel_target.dart

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1088,11 +1088,11 @@ class KernelTarget extends TargetImplementation {
10881088
//..fileEndOffset = cls.fileOffset
10891089
..isNonNullableByDefault = cls.enclosingLibrary.isNonNullableByDefault;
10901090

1091+
TypeDependency? typeDependency;
10911092
if (hasTypeDependency) {
1092-
loader.registerTypeDependency(
1093-
constructor,
1094-
new TypeDependency(constructor, superConstructor, substitution,
1095-
copyReturnType: false));
1093+
typeDependency = new TypeDependency(
1094+
constructor, superConstructor, substitution,
1095+
copyReturnType: false);
10961096
}
10971097

10981098
Procedure? constructorTearOff = createConstructorTearOffProcedure(
@@ -1107,16 +1107,20 @@ class KernelTarget extends TargetImplementation {
11071107
buildConstructorTearOffProcedure(constructorTearOff, constructor,
11081108
classBuilder.cls, classBuilder.libraryBuilder);
11091109
}
1110-
return new SyntheticSourceConstructorBuilder(
1111-
classBuilder, constructor, constructorTearOff,
1112-
// We pass on the original constructor and the cloned function nodes to
1113-
// ensure that the default values are computed and cloned for the
1114-
// outline. It is needed to make the default values a part of the
1115-
// outline for const constructors, and additionally it is required for
1116-
// a potential subclass using super initializing parameters that will
1117-
// required the cloning of the default values.
1118-
definingConstructor: superConstructorBuilder,
1119-
delayedDefaultValueCloner: delayedDefaultValueCloner);
1110+
SyntheticSourceConstructorBuilder constructorBuilder =
1111+
new SyntheticSourceConstructorBuilder(
1112+
classBuilder, constructor, constructorTearOff,
1113+
// We pass on the original constructor and the cloned function nodes
1114+
// to ensure that the default values are computed and cloned for the
1115+
// outline. It is needed to make the default values a part of the
1116+
// outline for const constructors, and additionally it is required
1117+
// for a potential subclass using super initializing parameters that
1118+
// will required the cloning of the default values.
1119+
definingConstructor: superConstructorBuilder,
1120+
delayedDefaultValueCloner: delayedDefaultValueCloner,
1121+
typeDependency: typeDependency);
1122+
loader.registerConstructorToBeInferred(constructor, constructorBuilder);
1123+
return constructorBuilder;
11201124
}
11211125

11221126
void finishSynthesizedParameters({bool forOutline = false}) {

pkg/front_end/lib/src/fasta/source/source_constructor_builder.dart

Lines changed: 45 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ import '../kernel/body_builder.dart' show BodyBuilder;
2626
import '../kernel/constructor_tearoff_lowering.dart';
2727
import '../kernel/expression_generator_helper.dart';
2828
import '../kernel/hierarchy/class_member.dart' show ClassMember;
29-
import '../kernel/kernel_helper.dart' show DelayedDefaultValueCloner;
29+
import '../kernel/kernel_helper.dart'
30+
show DelayedDefaultValueCloner, TypeDependency;
3031
import '../kernel/utils.dart'
3132
show isRedirectingGenerativeConstructorImplementation;
3233
import '../messages.dart'
@@ -52,6 +53,9 @@ import 'source_function_builder.dart';
5253

5354
abstract class SourceConstructorBuilder
5455
implements ConstructorBuilder, SourceMemberBuilder {
56+
/// Infers the types of any untyped initializing formals.
57+
void inferFormalTypes(TypeEnvironment typeEnvironment);
58+
5559
void addSuperParameterDefaultValueCloners(
5660
List<DelayedDefaultValueCloner> delayedDefaultValueCloners);
5761
}
@@ -224,7 +228,7 @@ class DeclaredSourceConstructorBuilder extends SourceFunctionBuilderImpl
224228
return _constructor;
225229
}
226230

227-
/// Infers the types of any untyped initializing formals.
231+
@override
228232
void inferFormalTypes(TypeEnvironment typeEnvironment) {
229233
if (_hasFormalsInferred) return;
230234
if (formals != null) {
@@ -321,14 +325,8 @@ class DeclaredSourceConstructorBuilder extends SourceFunctionBuilderImpl
321325
ConstructorBuilder? superTargetBuilder =
322326
_computeSuperTargetBuilder(initializers);
323327

324-
if (superTargetBuilder is DeclaredSourceConstructorBuilder) {
328+
if (superTargetBuilder is SourceConstructorBuilder) {
325329
superTargetBuilder.inferFormalTypes(typeEnvironment);
326-
} else if (superTargetBuilder is SyntheticSourceConstructorBuilder) {
327-
MemberBuilder? superTargetOriginBuilder =
328-
superTargetBuilder._effectivelyDefiningConstructor;
329-
if (superTargetOriginBuilder is DeclaredSourceConstructorBuilder) {
330-
superTargetOriginBuilder.inferFormalTypes(typeEnvironment);
331-
}
332330
}
333331

334332
Constructor superTarget;
@@ -339,10 +337,9 @@ class DeclaredSourceConstructorBuilder extends SourceFunctionBuilderImpl
339337
superFormals = superTargetBuilder.formals!;
340338
} else if (superTargetBuilder is DillConstructorBuilder) {
341339
superTarget = superTargetBuilder.constructor;
340+
superConstructorFunction = superTargetBuilder.function;
342341
if (superTargetBuilder is SyntheticSourceConstructorBuilder) {
343342
superFormals = superTargetBuilder.formals;
344-
} else {
345-
superConstructorFunction = superTargetBuilder.function;
346343
}
347344
} else {
348345
// The error in this case should be reported elsewhere. Here we perform a
@@ -354,7 +351,27 @@ class DeclaredSourceConstructorBuilder extends SourceFunctionBuilderImpl
354351
List<bool> positionalSuperFormalHasInitializer = [];
355352
Map<String, DartType?> namedSuperFormalType = {};
356353
Map<String, bool> namedSuperFormalHasInitializer = {};
357-
if (superFormals != null) {
354+
// TODO(johnniwinther): Clean this up when [VariableDeclaration] has a
355+
// `hasDeclaredInitializer` flag.
356+
if (superFormals != null && superConstructorFunction != null) {
357+
for (VariableDeclaration formal
358+
in superConstructorFunction.positionalParameters) {
359+
positionalSuperFormalType.add(formal.type);
360+
}
361+
for (VariableDeclaration formal
362+
in superConstructorFunction.namedParameters) {
363+
namedSuperFormalType[formal.name!] = formal.type;
364+
}
365+
for (FormalParameterBuilder formal in superFormals) {
366+
if (formal.isPositional) {
367+
positionalSuperFormalHasInitializer
368+
.add(formal.hasDeclaredInitializer);
369+
} else {
370+
namedSuperFormalHasInitializer[formal.name] =
371+
formal.hasDeclaredInitializer;
372+
}
373+
}
374+
} else if (superFormals != null) {
358375
for (FormalParameterBuilder formal in superFormals) {
359376
if (formal.isPositional) {
360377
positionalSuperFormalType.add(formal.variable?.type);
@@ -802,19 +819,34 @@ class SyntheticSourceConstructorBuilder extends DillConstructorBuilder
802819
/// the constructor that effectively defines this constructor.
803820
MemberBuilder? _immediatelyDefiningConstructor;
804821
DelayedDefaultValueCloner? _delayedDefaultValueCloner;
822+
TypeDependency? _typeDependency;
805823

806824
SyntheticSourceConstructorBuilder(SourceClassBuilder parent,
807825
Constructor constructor, Procedure? constructorTearOff,
808826
{MemberBuilder? definingConstructor,
809-
DelayedDefaultValueCloner? delayedDefaultValueCloner})
827+
DelayedDefaultValueCloner? delayedDefaultValueCloner,
828+
TypeDependency? typeDependency})
810829
: _immediatelyDefiningConstructor = definingConstructor,
811830
_delayedDefaultValueCloner = delayedDefaultValueCloner,
831+
_typeDependency = typeDependency,
812832
super(constructor, constructorTearOff, parent);
813833

814834
@override
815835
SourceLibraryBuilder get libraryBuilder =>
816836
super.libraryBuilder as SourceLibraryBuilder;
817837

838+
@override
839+
void inferFormalTypes(TypeEnvironment typeEnvironment) {
840+
if (_immediatelyDefiningConstructor is SourceConstructorBuilder) {
841+
(_immediatelyDefiningConstructor as SourceConstructorBuilder)
842+
.inferFormalTypes(typeEnvironment);
843+
}
844+
if (_typeDependency != null) {
845+
_typeDependency!.copyInferred();
846+
_typeDependency = null;
847+
}
848+
}
849+
818850
MemberBuilder? get _effectivelyDefiningConstructor {
819851
MemberBuilder? origin = _immediatelyDefiningConstructor;
820852
while (origin is SyntheticSourceConstructorBuilder) {

pkg/front_end/lib/src/fasta/source/source_loader.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -974,7 +974,7 @@ severity: $severity
974974
}
975975

976976
void registerConstructorToBeInferred(
977-
Constructor constructor, DeclaredSourceConstructorBuilder builder) {
977+
Constructor constructor, SourceConstructorBuilder builder) {
978978
_typeInferenceEngine!.toBeInferred[constructor] = builder;
979979
}
980980

pkg/front_end/lib/src/fasta/type_inference/type_inference_engine.dart

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,14 +113,14 @@ abstract class TypeInferenceEngine {
113113
/// This is represented as a map from a constructor to its library
114114
/// builder because the builder is used to report errors due to cyclic
115115
/// inference dependencies.
116-
final Map<Constructor, DeclaredSourceConstructorBuilder> toBeInferred = {};
116+
final Map<Constructor, SourceConstructorBuilder> toBeInferred = {};
117117

118118
/// A map containing constructors in the process of being inferred.
119119
///
120120
/// This is used to detect cyclic inference dependencies. It is represented
121121
/// as a map from a constructor to its library builder because the builder
122122
/// is used to report errors.
123-
final Map<Constructor, DeclaredSourceConstructorBuilder> beingInferred = {};
123+
final Map<Constructor, SourceConstructorBuilder> beingInferred = {};
124124

125125
final Map<Member, TypeDependency> typeDependencies = {};
126126

@@ -144,7 +144,7 @@ abstract class TypeInferenceEngine {
144144
void finishTopLevelInitializingFormals() {
145145
// Field types have all been inferred so we don't need to guard against
146146
// cyclic dependency.
147-
for (DeclaredSourceConstructorBuilder builder in toBeInferred.values) {
147+
for (SourceConstructorBuilder builder in toBeInferred.values) {
148148
builder.inferFormalTypes(typeSchemaEnvironment);
149149
}
150150
toBeInferred.clear();

pkg/front_end/lib/src/fasta/type_inference/type_inferrer.dart

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -388,8 +388,7 @@ class TypeInferrerImpl implements TypeInferrer {
388388

389389
@override
390390
void inferConstructorParameterTypes(Constructor target) {
391-
DeclaredSourceConstructorBuilder? constructor =
392-
engine.beingInferred[target];
391+
SourceConstructorBuilder? constructor = engine.beingInferred[target];
393392
if (constructor != null) {
394393
// There is a cyclic dependency where inferring the types of the
395394
// initializing formals of a constructor required us to infer the
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
class A {}
6+
7+
class Mixin {}
8+
9+
abstract class B<D> {
10+
B({
11+
required this.field,
12+
});
13+
14+
final D field;
15+
}
16+
17+
class C extends B<A> with Mixin {
18+
C({
19+
required super.field,
20+
});
21+
}
22+
23+
main() {}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
library /*isNonNullableByDefault*/;
2+
import self as self;
3+
import "dart:core" as core;
4+
5+
class A extends core::Object {
6+
synthetic constructor •() → self::A
7+
: super core::Object::•()
8+
;
9+
}
10+
class Mixin extends core::Object {
11+
synthetic constructor •() → self::Mixin
12+
: super core::Object::•()
13+
;
14+
}
15+
abstract class B<D extends core::Object? = dynamic> extends core::Object {
16+
final field self::B::D% field;
17+
constructor •({required self::B::D% field = #C1}) → self::B<self::B::D%>
18+
: self::B::field = field, super core::Object::•()
19+
;
20+
}
21+
abstract class _C&B&Mixin = self::B<self::A> with self::Mixin /*isAnonymousMixin*/ {
22+
synthetic constructor •({self::A field = #C1}) → self::_C&B&Mixin
23+
: super self::B::•(field: field)
24+
;
25+
}
26+
class C extends self::_C&B&Mixin {
27+
constructor •({required self::A field = #C1}) → self::C
28+
: super self::_C&B&Mixin::•(field: field)
29+
;
30+
}
31+
static method main() → dynamic {}
32+
33+
constants {
34+
#C1 = null
35+
}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
library /*isNonNullableByDefault*/;
2+
import self as self;
3+
import "dart:core" as core;
4+
5+
class A extends core::Object {
6+
synthetic constructor •() → self::A
7+
: super core::Object::•()
8+
;
9+
}
10+
class Mixin extends core::Object {
11+
synthetic constructor •() → self::Mixin
12+
: super core::Object::•()
13+
;
14+
}
15+
abstract class B<D extends core::Object? = dynamic> extends core::Object {
16+
final field self::B::D% field;
17+
constructor •({required self::B::D% field = #C1}) → self::B<self::B::D%>
18+
: self::B::field = field, super core::Object::•()
19+
;
20+
}
21+
abstract class _C&B&Mixin extends self::B<self::A> implements self::Mixin /*isAnonymousMixin,isEliminatedMixin*/ {
22+
synthetic constructor •({self::A field = #C1}) → self::_C&B&Mixin
23+
: super self::B::•(field: field)
24+
;
25+
}
26+
class C extends self::_C&B&Mixin {
27+
constructor •({required self::A field = #C1}) → self::C
28+
: super self::_C&B&Mixin::•(field: field)
29+
;
30+
}
31+
static method main() → dynamic {}
32+
33+
constants {
34+
#C1 = null
35+
}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
class A {}
2+
3+
class Mixin {}
4+
5+
abstract class B<D> {
6+
B({
7+
required this.field,
8+
});
9+
final D field;
10+
}
11+
12+
class C extends B<A> with Mixin {
13+
C({
14+
required super.field,
15+
});
16+
}
17+
18+
main() {}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
abstract class B<D> {
2+
B({
3+
required this.field,
4+
});
5+
final D field;
6+
}
7+
8+
class A {}
9+
10+
class C extends B<A> with Mixin {
11+
C({
12+
required super.field,
13+
});
14+
}
15+
16+
class Mixin {}
17+
18+
main() {}

0 commit comments

Comments
 (0)