Skip to content

Commit 9887828

Browse files
johnniwintherCommit Queue
authored andcommitted
[cfe] Handle redirecting factories in top level inference.
Previously top level inference was triggered before inference of redirecting factories. This only supported dependencies like: redirecting factory -> field A user encountered a crash when a field type depended on a redirecting factory, resulting in the opposite dependency: field -> redirecting factory Redirecting factories are now included in the top level inference, similar to how generative constructors where already handled, by introducing an InferableMember interface. Tests are added that exercises this dependency as well as: field -> redirecting factory -> field Change-Id: I0bff1012990d536fbf44e84d6f4d6b26566043a2 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/415862 Commit-Queue: Johnni Winther <[email protected]> Reviewed-by: Jens Johansen <[email protected]>
1 parent 151edd1 commit 9887828

File tree

38 files changed

+784
-152
lines changed

38 files changed

+784
-152
lines changed

pkg/front_end/lib/src/builder/formal_parameter_builder.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ class FormalParameterBuilder extends BuilderImpl
230230
String fieldName = isWildcardLoweredFormalParameter(name) ? '_' : name;
231231
Builder? fieldBuilder = declarationBuilder.lookupLocalMember(fieldName);
232232
if (fieldBuilder is SourcePropertyBuilder && fieldBuilder.isField) {
233-
DartType fieldType = fieldBuilder.inferType(hierarchy);
233+
DartType fieldType = fieldBuilder.inferFieldType(hierarchy);
234234
fieldType = constructorDeclaration.substituteFieldType(fieldType);
235235
type.registerInferredType(fieldType);
236236
} else {

pkg/front_end/lib/src/dill/dill_member_builder.dart

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -509,7 +509,8 @@ class DillClassMember extends BuilderClassMember {
509509

510510
@override
511511
// Coverage-ignore(suite): Not run.
512-
void registerOverrideDependency(Set<ClassMember> overriddenMembers) {
512+
void registerOverrideDependency(
513+
ClassMembersBuilder membersBuilder, Set<ClassMember> overriddenMembers) {
513514
// Do nothing; this is only for source members.
514515
}
515516

pkg/front_end/lib/src/fragment/constructor/encoding.dart

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -200,8 +200,8 @@ class RegularConstructorEncoding {
200200
}
201201
}
202202
if (needsInference) {
203-
libraryBuilder.loader
204-
.registerConstructorToBeInferred(_constructor, constructorBuilder);
203+
libraryBuilder.loader.registerConstructorToBeInferred(
204+
new InferableConstructor(_constructor, constructorBuilder));
205205
}
206206
}
207207
}
@@ -500,8 +500,8 @@ class ExtensionTypeConstructorEncoding {
500500
}
501501
}
502502
if (needsInference) {
503-
libraryBuilder.loader
504-
.registerConstructorToBeInferred(_constructor, constructorBuilder);
503+
libraryBuilder.loader.registerConstructorToBeInferred(
504+
new InferableConstructor(_constructor, constructorBuilder));
505505
}
506506
}
507507
}

pkg/front_end/lib/src/fragment/enum_element.dart

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,7 @@ class EnumElementFragment
317317
}
318318

319319
@override
320+
// Coverage-ignore(suite): Not run.
320321
void ensureTypes(
321322
ClassMembersBuilder membersBuilder,
322323
Set<ClassMember>? getterOverrideDependencies,
@@ -434,7 +435,7 @@ class _EnumElementClassMember implements ClassMember {
434435

435436
@override
436437
Member getMember(ClassMembersBuilder membersBuilder) {
437-
_builder.ensureTypes(membersBuilder);
438+
inferType(membersBuilder);
438439
return forSetter
439440
?
440441
// Coverage-ignore(suite): Not run.
@@ -458,9 +459,8 @@ class _EnumElementClassMember implements ClassMember {
458459
bool get hasDeclarations => false;
459460

460461
@override
461-
// Coverage-ignore(suite): Not run.
462462
void inferType(ClassMembersBuilder membersBuilder) {
463-
_builder.ensureTypes(membersBuilder);
463+
_builder.inferFieldType(membersBuilder.hierarchyBuilder);
464464
}
465465

466466
@override
@@ -531,8 +531,10 @@ class _EnumElementClassMember implements ClassMember {
531531

532532
@override
533533
// Coverage-ignore(suite): Not run.
534-
void registerOverrideDependency(Set<ClassMember> overriddenMembers) {
535-
_builder.registerGetterOverrideDependency(overriddenMembers);
534+
void registerOverrideDependency(
535+
ClassMembersBuilder membersBuilder, Set<ClassMember> overriddenMembers) {
536+
_builder.registerGetterOverrideDependency(
537+
membersBuilder, overriddenMembers);
536538
}
537539

538540
@override

pkg/front_end/lib/src/fragment/field.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,7 @@ class FieldFragment
366366
nameLength: name.length,
367367
isAssignable: hasSetter);
368368
} else {
369+
// Coverage-ignore-block(suite): Not run.
369370
type.build(builder.libraryBuilder, TypeUse.fieldType,
370371
hierarchy: membersBuilder.hierarchyBuilder);
371372
}

pkg/front_end/lib/src/fragment/field/class_member.dart

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ class _FieldClassMember implements ClassMember {
4949

5050
@override
5151
Member getMember(ClassMembersBuilder membersBuilder) {
52-
_builder.ensureTypes(membersBuilder);
52+
inferType(membersBuilder);
5353
return forSetter ? _builder.writeTarget! : _builder.readTarget!;
5454
}
5555

@@ -84,7 +84,7 @@ class _FieldClassMember implements ClassMember {
8484

8585
@override
8686
void inferType(ClassMembersBuilder membersBuilder) {
87-
_builder.ensureTypes(membersBuilder);
87+
_builder.inferFieldType(membersBuilder.hierarchyBuilder);
8888
}
8989

9090
@override
@@ -150,11 +150,14 @@ class _FieldClassMember implements ClassMember {
150150
Name get name => _builder.memberName;
151151

152152
@override
153-
void registerOverrideDependency(Set<ClassMember> overriddenMembers) {
153+
void registerOverrideDependency(
154+
ClassMembersBuilder membersBuilder, Set<ClassMember> overriddenMembers) {
154155
if (forSetter) {
155-
_builder.registerSetterOverrideDependency(overriddenMembers);
156+
_builder.registerSetterOverrideDependency(
157+
membersBuilder, overriddenMembers);
156158
} else {
157-
_builder.registerGetterOverrideDependency(overriddenMembers);
159+
_builder.registerGetterOverrideDependency(
160+
membersBuilder, overriddenMembers);
158161
}
159162
}
160163

@@ -183,14 +186,14 @@ class _SynthesizedFieldClassMember implements ClassMember {
183186

184187
@override
185188
Member getMember(ClassMembersBuilder membersBuilder) {
186-
_builder.ensureTypes(membersBuilder);
189+
inferType(membersBuilder);
187190
return _member;
188191
}
189192

190193
@override
191194
Member? getTearOff(ClassMembersBuilder membersBuilder) {
192195
// Ensure field type is computed.
193-
getMember(membersBuilder);
196+
inferType(membersBuilder);
194197
return null;
195198
}
196199

@@ -209,15 +212,18 @@ class _SynthesizedFieldClassMember implements ClassMember {
209212

210213
@override
211214
void inferType(ClassMembersBuilder membersBuilder) {
212-
_builder.ensureTypes(membersBuilder);
215+
_builder.inferFieldType(membersBuilder.hierarchyBuilder);
213216
}
214217

215218
@override
216-
void registerOverrideDependency(Set<ClassMember> overriddenMembers) {
219+
void registerOverrideDependency(
220+
ClassMembersBuilder membersBuilder, Set<ClassMember> overriddenMembers) {
217221
if (forSetter) {
218-
_builder.registerSetterOverrideDependency(overriddenMembers);
222+
_builder.registerSetterOverrideDependency(
223+
membersBuilder, overriddenMembers);
219224
} else {
220-
_builder.registerGetterOverrideDependency(overriddenMembers);
225+
_builder.registerGetterOverrideDependency(
226+
membersBuilder, overriddenMembers);
221227
}
222228
}
223229

pkg/front_end/lib/src/kernel/benchmarker.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,9 @@ enum BenchmarkPhases {
185185
outline_buildClassHierarchyMembers,
186186
outline_computeHierarchy,
187187
outline_installTypedefTearOffs,
188+
outline_prepareTopLevelInference,
188189
outline_performRedirectingFactoryInference,
190+
outline_computeMemberTypes,
189191
outline_performTopLevelInference,
190192
outline_checkOverrides,
191193
outline_checkAbstractMembers,

pkg/front_end/lib/src/kernel/hierarchy/class_member.dart

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -339,7 +339,8 @@ abstract class ClassMember {
339339
/// Registers that this class member overrides [overriddenMembers].
340340
///
341341
/// This is used to infer types from the [overriddenMembers].
342-
void registerOverrideDependency(Set<ClassMember> overriddenMembers);
342+
void registerOverrideDependency(
343+
ClassMembersBuilder membersBuilder, Set<ClassMember> overriddenMembers);
343344

344345
/// Returns `true` if this has the same underlying declaration as [other].
345346
///
@@ -400,7 +401,8 @@ abstract class SynthesizedMember extends ClassMember {
400401

401402
@override
402403
// Coverage-ignore(suite): Not run.
403-
void registerOverrideDependency(Set<ClassMember> overriddenMembers) {}
404+
void registerOverrideDependency(
405+
ClassMembersBuilder membersBuilder, Set<ClassMember> overriddenMembers) {}
404406

405407
@override
406408
MemberResult getMemberResult(ClassMembersBuilder membersBuilder) {

pkg/front_end/lib/src/kernel/hierarchy/members_node.dart

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -967,7 +967,8 @@ class ClassMembersNodeBuilder extends MembersNodeBuilder {
967967
/// Member types can be queried at arbitrary points during top level
968968
/// inference so we need to ensure that types are computed in dependency
969969
/// order.
970-
classMember.registerOverrideDependency(overriddenMembers);
970+
classMember.registerOverrideDependency(
971+
_membersBuilder, overriddenMembers);
971972

972973
/// Not all member type are queried during top level inference so we
973974
/// register delayed computation to ensure that all types have been

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

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -541,14 +541,23 @@ class KernelTarget {
541541
?.enterPhase(BenchmarkPhases.outline_computeFieldPromotability);
542542
loader.computeFieldPromotability();
543543

544+
benchmarker
545+
// Coverage-ignore(suite): Not run.
546+
?.enterPhase(BenchmarkPhases.outline_prepareTopLevelInference);
547+
loader.prepareTopLevelInference();
548+
544549
benchmarker
545550
// Coverage-ignore(suite): Not run.
546551
?.enterPhase(
547552
BenchmarkPhases.outline_performRedirectingFactoryInference);
548553
// TODO(johnniwinther): Add an interface for registering delayed actions.
549554
List<DelayedDefaultValueCloner> delayedDefaultValueCloners = [];
550-
loader.inferRedirectingFactories(
551-
loader.hierarchy, delayedDefaultValueCloners);
555+
loader.inferRedirectingFactories(delayedDefaultValueCloners);
556+
557+
benchmarker
558+
// Coverage-ignore(suite): Not run.
559+
?.enterPhase(BenchmarkPhases.outline_computeMemberTypes);
560+
loader.computeMemberTypes();
552561

553562
benchmarker
554563
// Coverage-ignore(suite): Not run.
@@ -1143,7 +1152,8 @@ class KernelTarget {
11431152
definingConstructor: superConstructorBuilder,
11441153
delayedDefaultValueCloner: delayedDefaultValueCloner,
11451154
typeDependency: typeDependency);
1146-
loader.registerConstructorToBeInferred(constructor, constructorBuilder);
1155+
loader.registerConstructorToBeInferred(
1156+
new InferableConstructor(constructor, constructorBuilder));
11471157
return constructorBuilder;
11481158
}
11491159

0 commit comments

Comments
 (0)