Skip to content

Commit 54031e2

Browse files
chloestefantsovaCommit Queue
chloestefantsova
authored and
Commit Queue
committed
[cfe] Ensure default values in synthesized function nodes
Closes #55529 Change-Id: Ic4738e9b8abc333a39ec52b642bbf844128ef61b Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/364325 Commit-Queue: Chloe Stefantsova <cstefantsova@google.com> Reviewed-by: Johnni Winther <johnniwinther@google.com> Auto-Submit: Chloe Stefantsova <cstefantsova@google.com>
1 parent dd3fbff commit 54031e2

File tree

75 files changed

+501
-241
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

75 files changed

+501
-241
lines changed

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -250,13 +250,15 @@ class FormalParameterBuilder extends ModifierBuilderImpl
250250
// and named parameters in several cases:
251251
// * for const constructors to enable constant evaluation,
252252
// * for instance methods because these might be needed to generated
253-
// noSuchMethod forwarders, and
253+
// noSuchMethod forwarders,
254254
// * for generative constructors to support forwarding constructors
255-
// in mixin applications.
255+
// in mixin applications, and
256+
// * for factories, to uphold the invariant that optional parameters always
257+
// have default values, even during modular compilation.
256258
if (parent is ConstructorBuilder) {
257259
return true;
258260
} else if (parent is SourceFactoryBuilder) {
259-
return parent!.isFactory && parent!.isConst;
261+
return parent!.isFactory;
260262
} else {
261263
return parent!.isClassInstanceMember;
262264
}

‎pkg/front_end/lib/src/fasta/kernel/constructor_tearoff_lowering.dart‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ Procedure createTypedefTearOffProcedure(
7979
/// The [declarationConstructor] is the origin constructor and
8080
/// [implementationConstructor] is the augmentation constructor, if augmented,
8181
/// otherwise it is the [declarationConstructor].
82-
void buildConstructorTearOffProcedure(
82+
DelayedDefaultValueCloner buildConstructorTearOffProcedure(
8383
{required Procedure tearOff,
8484
required Member declarationConstructor,
8585
required Member implementationConstructor,
@@ -118,12 +118,17 @@ void buildConstructorTearOffProcedure(
118118

119119
List<DartType> typeArguments = freshTypeParameters.freshTypeArguments;
120120
Substitution substitution = freshTypeParameters.substitution;
121-
_createParameters(tearOff, implementationConstructor, function, substitution,
121+
DelayedDefaultValueCloner delayedDefaultValueCloner = _createParameters(
122+
tearOff,
123+
implementationConstructor,
124+
function,
125+
substitution,
122126
libraryBuilder);
123127
Arguments arguments = _createArguments(tearOff, typeArguments, fileOffset);
124128
_createTearOffBody(tearOff, declarationConstructor, arguments);
125129
tearOff.function.fileOffset = tearOff.fileOffset;
126130
tearOff.function.fileEndOffset = tearOff.fileOffset;
131+
return delayedDefaultValueCloner;
127132
}
128133

129134
/// Creates the parameters and body for [tearOff] for a typedef tearoff of
@@ -134,7 +139,7 @@ void buildConstructorTearOffProcedure(
134139
/// The [declarationConstructor] is the origin constructor and
135140
/// [implementationConstructor] is the augmentation constructor, if augmented,
136141
/// otherwise it is the [declarationConstructor].
137-
void buildTypedefTearOffProcedure(
142+
DelayedDefaultValueCloner buildTypedefTearOffProcedure(
138143
{required Procedure tearOff,
139144
required Member declarationConstructor,
140145
required Member implementationConstructor,
@@ -185,7 +190,7 @@ void buildTypedefTearOffProcedure(
185190
(int index) => substitution.substituteType(typeArguments[index]));
186191
}
187192
}
188-
_createParameters(
193+
DelayedDefaultValueCloner delayedDefaultValueCloner = _createParameters(
189194
tearOff,
190195
implementationConstructor,
191196
function,
@@ -195,6 +200,7 @@ void buildTypedefTearOffProcedure(
195200
_createTearOffBody(tearOff, declarationConstructor, arguments);
196201
tearOff.function.fileOffset = tearOff.fileOffset;
197202
tearOff.function.fileEndOffset = tearOff.fileOffset;
203+
return delayedDefaultValueCloner;
198204
}
199205

200206
/// Creates the parameters for the redirecting factory [tearOff] based on the
@@ -207,13 +213,19 @@ void buildTypedefTearOffProcedure(
207213
FreshTypeParameters buildRedirectingFactoryTearOffProcedureParameters(
208214
{required Procedure tearOff,
209215
required Procedure implementationConstructor,
210-
required SourceLibraryBuilder libraryBuilder}) {
216+
required SourceLibraryBuilder libraryBuilder,
217+
List<DelayedDefaultValueCloner>? delayedDefaultValueCloners}) {
211218
FunctionNode function = implementationConstructor.function;
212219
FreshTypeParameters freshTypeParameters =
213220
_createFreshTypeParameters(function.typeParameters, tearOff.function);
214221
Substitution substitution = freshTypeParameters.substitution;
215-
_createParameters(tearOff, implementationConstructor, function, substitution,
222+
DelayedDefaultValueCloner delayedDefaultValueCloner = _createParameters(
223+
tearOff,
224+
implementationConstructor,
225+
function,
226+
substitution,
216227
libraryBuilder);
228+
delayedDefaultValueCloners?.add(delayedDefaultValueCloner);
217229
tearOff.function.fileOffset = tearOff.fileOffset;
218230
tearOff.function.fileEndOffset = tearOff.fileOffset;
219231
return freshTypeParameters;
@@ -295,7 +307,7 @@ FreshTypeParameters _createFreshTypeParameters(
295307
/// Creates the parameters for the [tearOff] lowering based of the parameters
296308
/// in [constructor] and using the [substitution] to compute the parameter and
297309
/// return types.
298-
void _createParameters(
310+
DelayedDefaultValueCloner _createParameters(
299311
Procedure tearOff,
300312
Member constructor,
301313
FunctionNode function,
@@ -326,6 +338,8 @@ void _createParameters(
326338
tearOff,
327339
new TypeDependency(tearOff, constructor, substitution,
328340
copyReturnType: true));
341+
return new DelayedDefaultValueCloner(constructor, tearOff,
342+
identicalSignatures: true, libraryBuilder: libraryBuilder);
329343
}
330344

331345
/// Creates the [Arguments] for passing the parameters from [tearOff] to its

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

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,9 @@ class DelayedDefaultValueCloner {
131131
VariableDeclaration synthesizedParameter =
132132
_synthesized.positionalParameters[superParameterIndex];
133133
_cloneDefaultValueForSuperParameters(
134-
originalParameter, synthesizedParameter, typeEnvironment);
134+
originalParameter, synthesizedParameter, typeEnvironment,
135+
isOptional:
136+
superParameterIndex >= _synthesized.requiredParameterCount);
135137
}
136138
}
137139
}
@@ -162,7 +164,8 @@ class DelayedDefaultValueCloner {
162164
VariableDeclaration synthesizedParameter =
163165
_synthesized.namedParameters[i];
164166
_cloneDefaultValueForSuperParameters(
165-
originalParameter, synthesizedParameter, typeEnvironment);
167+
originalParameter, synthesizedParameter, typeEnvironment,
168+
isOptional: !synthesizedParameter.isRequired);
166169
} else {
167170
// TODO(cstefantsova): Handle the erroneous case of missing names.
168171
}
@@ -218,7 +221,8 @@ class DelayedDefaultValueCloner {
218221
void _cloneDefaultValueForSuperParameters(
219222
VariableDeclaration originalParameter,
220223
VariableDeclaration synthesizedParameter,
221-
TypeEnvironment typeEnvironment) {
224+
TypeEnvironment typeEnvironment,
225+
{required bool isOptional}) {
222226
Expression? originalParameterInitializer = originalParameter.initializer;
223227
DartType? originalParameterInitializerType = originalParameterInitializer
224228
?.getStaticType(new StaticTypeContext(synthesized, typeEnvironment));
@@ -227,6 +231,9 @@ class DelayedDefaultValueCloner {
227231
typeEnvironment.isSubtypeOf(originalParameterInitializerType,
228232
synthesizedParameterType, SubtypeCheckMode.withNullabilities)) {
229233
_cloneInitializer(originalParameter, synthesizedParameter);
234+
} else if (originalParameterInitializer == null && isOptional) {
235+
synthesizedParameter.initializer = new NullLiteral()
236+
..parent = synthesizedParameter;
230237
} else {
231238
synthesizedParameter.hasDeclaredInitializer = false;
232239
if (synthesizedParameterType.isPotentiallyNonNullable) {

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

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -530,7 +530,11 @@ class KernelTarget extends TargetImplementation {
530530
loader.computeShowHideElements();
531531

532532
benchmarker?.enterPhase(BenchmarkPhases.outline_installTypedefTearOffs);
533-
loader.installTypedefTearOffs();
533+
List<DelayedDefaultValueCloner>?
534+
typedefTearOffsDelayedDefaultValueCloners =
535+
loader.installTypedefTearOffs();
536+
typedefTearOffsDelayedDefaultValueCloners
537+
?.forEach(registerDelayedDefaultValueCloner);
534538

535539
benchmarker
536540
?.enterPhase(BenchmarkPhases.outline_computeFieldPromotability);
@@ -1123,12 +1127,15 @@ class KernelTarget extends TargetImplementation {
11231127
forAbstractClassOrEnumOrMixin: classBuilder.isAbstract);
11241128

11251129
if (constructorTearOff != null) {
1126-
buildConstructorTearOffProcedure(
1127-
tearOff: constructorTearOff,
1128-
declarationConstructor: constructor,
1129-
implementationConstructor: constructor,
1130-
enclosingDeclarationTypeParameters: classBuilder.cls.typeParameters,
1131-
libraryBuilder: libraryBuilder);
1130+
DelayedDefaultValueCloner delayedDefaultValueCloner =
1131+
buildConstructorTearOffProcedure(
1132+
tearOff: constructorTearOff,
1133+
declarationConstructor: constructor,
1134+
implementationConstructor: constructor,
1135+
enclosingDeclarationTypeParameters:
1136+
classBuilder.cls.typeParameters,
1137+
libraryBuilder: libraryBuilder);
1138+
registerDelayedDefaultValueCloner(delayedDefaultValueCloner);
11321139
}
11331140
SyntheticSourceConstructorBuilder constructorBuilder =
11341141
new SyntheticSourceConstructorBuilder(
@@ -1147,9 +1154,10 @@ class KernelTarget extends TargetImplementation {
11471154
}
11481155

11491156
void registerDelayedDefaultValueCloner(DelayedDefaultValueCloner cloner) {
1150-
assert(!_delayedDefaultValueCloners.containsKey(cloner.synthesized),
1151-
"Default cloner already registered for ${cloner.synthesized}.");
1152-
_delayedDefaultValueCloners[cloner.synthesized] = cloner;
1157+
// TODO(cstefantsova): Investigate the reason for the assumption breakage
1158+
// and uncomment the following line.
1159+
// assert(!_delayedDefaultValueCloners.containsKey(cloner.synthesized));
1160+
_delayedDefaultValueCloners[cloner.synthesized] ??= cloner;
11531161
}
11541162

11551163
void finishSynthesizedParameters({bool forOutline = false}) {
@@ -1202,12 +1210,15 @@ class KernelTarget extends TargetImplementation {
12021210
forAbstractClassOrEnumOrMixin:
12031211
enclosingClass.isAbstract || enclosingClass.isEnum);
12041212
if (constructorTearOff != null) {
1205-
buildConstructorTearOffProcedure(
1206-
tearOff: constructorTearOff,
1207-
declarationConstructor: constructor,
1208-
implementationConstructor: constructor,
1209-
enclosingDeclarationTypeParameters: classBuilder.cls.typeParameters,
1210-
libraryBuilder: libraryBuilder);
1213+
DelayedDefaultValueCloner delayedDefaultValueCloner =
1214+
buildConstructorTearOffProcedure(
1215+
tearOff: constructorTearOff,
1216+
declarationConstructor: constructor,
1217+
implementationConstructor: constructor,
1218+
enclosingDeclarationTypeParameters:
1219+
classBuilder.cls.typeParameters,
1220+
libraryBuilder: libraryBuilder);
1221+
registerDelayedDefaultValueCloner(delayedDefaultValueCloner);
12111222
}
12121223
return new SyntheticSourceConstructorBuilder(
12131224
classBuilder, constructor, constructorTearOff);

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

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -544,12 +544,15 @@ class DeclaredSourceConstructorBuilder
544544
_constructor.isExternal = isExternal;
545545

546546
if (_constructorTearOff != null) {
547-
buildConstructorTearOffProcedure(
548-
tearOff: _constructorTearOff,
549-
declarationConstructor: constructor,
550-
implementationConstructor: _constructor,
551-
enclosingDeclarationTypeParameters: classBuilder.cls.typeParameters,
552-
libraryBuilder: libraryBuilder);
547+
DelayedDefaultValueCloner delayedDefaultValueCloners =
548+
buildConstructorTearOffProcedure(
549+
tearOff: _constructorTearOff,
550+
declarationConstructor: constructor,
551+
implementationConstructor: _constructor,
552+
enclosingDeclarationTypeParameters:
553+
classBuilder.cls.typeParameters,
554+
libraryBuilder: libraryBuilder);
555+
_delayedDefaultValueCloners.add(delayedDefaultValueCloners);
553556
}
554557

555558
_hasBeenBuilt = true;
@@ -616,7 +619,7 @@ class DeclaredSourceConstructorBuilder
616619

617620
final bool _hasSuperInitializingFormals;
618621

619-
final List<DelayedDefaultValueCloner> _superParameterDefaultValueCloners =
622+
final List<DelayedDefaultValueCloner> _delayedDefaultValueCloners =
620623
<DelayedDefaultValueCloner>[];
621624

622625
@override
@@ -634,7 +637,7 @@ class DeclaredSourceConstructorBuilder
634637
doFinishConstructor: false);
635638
}
636639
finalizeSuperInitializingFormals(
637-
hierarchy, _superParameterDefaultValueCloners, initializers);
640+
hierarchy, _delayedDefaultValueCloners, initializers);
638641
}
639642
}
640643

@@ -824,8 +827,8 @@ class DeclaredSourceConstructorBuilder
824827
.addSuperParameterDefaultValueCloners(delayedDefaultValueCloners);
825828
}
826829

827-
delayedDefaultValueCloners.addAll(_superParameterDefaultValueCloners);
828-
_superParameterDefaultValueCloners.clear();
830+
delayedDefaultValueCloners.addAll(_delayedDefaultValueCloners);
831+
_delayedDefaultValueCloners.clear();
829832
}
830833

831834
@override
@@ -1085,6 +1088,8 @@ class SourceExtensionTypeConstructorBuilder
10851088

10861089
final MemberName _memberName;
10871090

1091+
DelayedDefaultValueCloner? _delayedDefaultValueCloner;
1092+
10881093
SourceExtensionTypeConstructorBuilder(
10891094
List<MetadataBuilder>? metadata,
10901095
int modifiers,
@@ -1194,6 +1199,10 @@ class SourceExtensionTypeConstructorBuilder
11941199
_buildBody();
11951200
}
11961201
beginInitializers = null;
1202+
1203+
if (_delayedDefaultValueCloner != null) {
1204+
delayedDefaultValueCloners.add(_delayedDefaultValueCloner!);
1205+
}
11971206
}
11981207

11991208
bool _hasBuiltBody = false;
@@ -1268,7 +1277,7 @@ class SourceExtensionTypeConstructorBuilder
12681277
_constructor.isExtensionTypeMember = true;
12691278

12701279
if (_constructorTearOff != null) {
1271-
buildConstructorTearOffProcedure(
1280+
_delayedDefaultValueCloner = buildConstructorTearOffProcedure(
12721281
tearOff: _constructorTearOff,
12731282
declarationConstructor: _constructor,
12741283
implementationConstructor: _constructor,

‎pkg/front_end/lib/src/fasta/source/source_factory_builder.dart‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,8 @@ class SourceFactoryBuilder extends SourceFunctionBuilderImpl {
6464

6565
final MemberName _memberName;
6666

67+
DelayedDefaultValueCloner? _delayedDefaultValueCloner;
68+
6769
SourceFactoryBuilder(
6870
List<MetadataBuilder>? metadata,
6971
int modifiers,
@@ -185,7 +187,7 @@ class SourceFactoryBuilder extends SourceFunctionBuilderImpl {
185187
_procedureInternal.isStatic = isStatic;
186188

187189
if (_factoryTearOff != null) {
188-
buildConstructorTearOffProcedure(
190+
_delayedDefaultValueCloner = buildConstructorTearOffProcedure(
189191
tearOff: _factoryTearOff,
190192
declarationConstructor: _procedure,
191193
implementationConstructor: _procedureInternal,
@@ -201,6 +203,9 @@ class SourceFactoryBuilder extends SourceFunctionBuilderImpl {
201203
List<DelayedActionPerformer> delayedActionPerformers,
202204
List<DelayedDefaultValueCloner> delayedDefaultValueCloners) {
203205
if (_hasBuiltOutlines) return;
206+
if (_delayedDefaultValueCloner != null) {
207+
delayedDefaultValueCloners.add(_delayedDefaultValueCloner!);
208+
}
204209
super.buildOutlineExpressions(
205210
classHierarchy, delayedActionPerformers, delayedDefaultValueCloners);
206211
_hasBuiltOutlines = true;

‎pkg/front_end/lib/src/fasta/source/source_library_builder.dart‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5489,25 +5489,39 @@ class SourceLibraryBuilder extends LibraryBuilderImpl {
54895489
}
54905490
}
54915491

5492-
void installTypedefTearOffs() {
5492+
List<DelayedDefaultValueCloner>? installTypedefTearOffs() {
5493+
List<DelayedDefaultValueCloner>? delayedDefaultValueCloners;
5494+
54935495
Iterable<SourceLibraryBuilder>? augmentationLibraries =
54945496
this.augmentationLibraries;
54955497
if (augmentationLibraries != null) {
54965498
for (SourceLibraryBuilder augmentationLibrary in augmentationLibraries) {
5497-
augmentationLibrary.installTypedefTearOffs();
5499+
List<DelayedDefaultValueCloner>?
5500+
augmentationLibraryDelayedDefaultValueCloners =
5501+
augmentationLibrary.installTypedefTearOffs();
5502+
if (augmentationLibraryDelayedDefaultValueCloners != null) {
5503+
(delayedDefaultValueCloners ??= [])
5504+
.addAll(augmentationLibraryDelayedDefaultValueCloners);
5505+
}
54985506
}
54995507
}
55005508

55015509
Iterator<SourceTypeAliasBuilder> iterator = localMembersIteratorOfType();
55025510
while (iterator.moveNext()) {
55035511
SourceTypeAliasBuilder declaration = iterator.current;
5504-
declaration.buildTypedefTearOffs(this, (Procedure procedure) {
5512+
DelayedDefaultValueCloner? delayedDefaultValueCloner =
5513+
declaration.buildTypedefTearOffs(this, (Procedure procedure) {
55055514
procedure.isStatic = true;
55065515
if (!declaration.isAugmenting && !declaration.isDuplicate) {
55075516
library.addProcedure(procedure);
55085517
}
55095518
});
5519+
if (delayedDefaultValueCloner != null) {
5520+
(delayedDefaultValueCloners ??= []).add(delayedDefaultValueCloner);
5521+
}
55105522
}
5523+
5524+
return delayedDefaultValueCloners;
55115525
}
55125526
}
55135527

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1779,12 +1779,19 @@ severity: $severity
17791779
ticker.logMs("Resolved $count constructors");
17801780
}
17811781

1782-
void installTypedefTearOffs() {
1782+
List<DelayedDefaultValueCloner>? installTypedefTearOffs() {
1783+
List<DelayedDefaultValueCloner>? delayedDefaultValueCloners;
17831784
if (target.backendTarget.isTypedefTearOffLoweringEnabled) {
17841785
for (SourceLibraryBuilder library in sourceLibraryBuilders) {
1785-
library.installTypedefTearOffs();
1786+
List<DelayedDefaultValueCloner>? libraryDelayedDefaultValueCloners =
1787+
library.installTypedefTearOffs();
1788+
if (libraryDelayedDefaultValueCloners != null) {
1789+
(delayedDefaultValueCloners ??= [])
1790+
.addAll(libraryDelayedDefaultValueCloners);
1791+
}
17861792
}
17871793
}
1794+
return delayedDefaultValueCloners;
17881795
}
17891796

17901797
void finishTypeVariables(Iterable<SourceLibraryBuilder> libraryBuilders,

0 commit comments

Comments
 (0)