[stable] [cfe] Ensure default values in synthesized function nodes

Closes #55529

Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/364325
Cherry-pick-request: #55714
Change-Id: If532820d53680a754c9e79a5a66367b1d44c6505
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/366300
Commit-Queue: Martin Kustermann <[email protected]>
Reviewed-by: Slava Egorov <[email protected]>

CHANGELOG.md

@@ -1,3 +1,10 @@
+## 3.4.1
+
+This is a patch release that
+
+- Fixes a bug in CFE which an manifest in compilation errors of flutter web app
+  when compiled with dart2wasm.
+
 ## 3.4.0
 
 ### Language

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

@@ -250,13 +250,15 @@ class FormalParameterBuilder extends ModifierBuilderImpl
     // and named parameters in several cases:
     // * for const constructors to enable constant evaluation,
     // * for instance methods because these might be needed to generated
-    //   noSuchMethod forwarders, and
+    //   noSuchMethod forwarders,
     // * for generative constructors to support forwarding constructors
-    //   in mixin applications.
+    //   in mixin applications, and
+    // * for factories, to uphold the invariant that optional parameters always
+    //   have default values, even during modular compilation.
     if (parent is ConstructorBuilder) {
       return true;
     } else if (parent is SourceFactoryBuilder) {
-      return parent!.isFactory && parent!.isConst;
+      return parent!.isFactory;
     } else {
       return parent!.isClassInstanceMember;
     }

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

@@ -79,7 +79,7 @@ Procedure createTypedefTearOffProcedure(
 /// The [declarationConstructor] is the origin constructor and
 /// [implementationConstructor] is the augmentation constructor, if augmented,
 /// otherwise it is the [declarationConstructor].
-void buildConstructorTearOffProcedure(
+DelayedDefaultValueCloner buildConstructorTearOffProcedure(
     {required Procedure tearOff,
     required Member declarationConstructor,
     required Member implementationConstructor,
@@ -118,12 +118,17 @@ void buildConstructorTearOffProcedure(
 
   List<DartType> typeArguments = freshTypeParameters.freshTypeArguments;
   Substitution substitution = freshTypeParameters.substitution;
-  _createParameters(tearOff, implementationConstructor, function, substitution,
+  DelayedDefaultValueCloner delayedDefaultValueCloner = _createParameters(
+      tearOff,
+      implementationConstructor,
+      function,
+      substitution,
       libraryBuilder);
   Arguments arguments = _createArguments(tearOff, typeArguments, fileOffset);
   _createTearOffBody(tearOff, declarationConstructor, arguments);
   tearOff.function.fileOffset = tearOff.fileOffset;
   tearOff.function.fileEndOffset = tearOff.fileOffset;
+  return delayedDefaultValueCloner;
 }
 
 /// Creates the parameters and body for [tearOff] for a typedef tearoff of
@@ -134,7 +139,7 @@ void buildConstructorTearOffProcedure(
 /// The [declarationConstructor] is the origin constructor and
 /// [implementationConstructor] is the augmentation constructor, if augmented,
 /// otherwise it is the [declarationConstructor].
-void buildTypedefTearOffProcedure(
+DelayedDefaultValueCloner buildTypedefTearOffProcedure(
     {required Procedure tearOff,
     required Member declarationConstructor,
     required Member implementationConstructor,
@@ -185,7 +190,7 @@ void buildTypedefTearOffProcedure(
           (int index) => substitution.substituteType(typeArguments[index]));
     }
   }
-  _createParameters(
+  DelayedDefaultValueCloner delayedDefaultValueCloner = _createParameters(
       tearOff,
       implementationConstructor,
       function,
@@ -195,6 +200,7 @@ void buildTypedefTearOffProcedure(
   _createTearOffBody(tearOff, declarationConstructor, arguments);
   tearOff.function.fileOffset = tearOff.fileOffset;
   tearOff.function.fileEndOffset = tearOff.fileOffset;
+  return delayedDefaultValueCloner;
 }
 
 /// Creates the parameters for the redirecting factory [tearOff] based on the
@@ -207,13 +213,19 @@ void buildTypedefTearOffProcedure(
 FreshTypeParameters buildRedirectingFactoryTearOffProcedureParameters(
     {required Procedure tearOff,
     required Procedure implementationConstructor,
-    required SourceLibraryBuilder libraryBuilder}) {
+    required SourceLibraryBuilder libraryBuilder,
+    List<DelayedDefaultValueCloner>? delayedDefaultValueCloners}) {
   FunctionNode function = implementationConstructor.function;
   FreshTypeParameters freshTypeParameters =
       _createFreshTypeParameters(function.typeParameters, tearOff.function);
   Substitution substitution = freshTypeParameters.substitution;
-  _createParameters(tearOff, implementationConstructor, function, substitution,
+  DelayedDefaultValueCloner delayedDefaultValueCloner = _createParameters(
+      tearOff,
+      implementationConstructor,
+      function,
+      substitution,
       libraryBuilder);
+  delayedDefaultValueCloners?.add(delayedDefaultValueCloner);
   tearOff.function.fileOffset = tearOff.fileOffset;
   tearOff.function.fileEndOffset = tearOff.fileOffset;
   return freshTypeParameters;
@@ -295,7 +307,7 @@ FreshTypeParameters _createFreshTypeParameters(
 /// Creates the parameters for the [tearOff] lowering based of the parameters
 /// in [constructor] and using the [substitution] to compute the parameter and
 /// return types.
-void _createParameters(
+DelayedDefaultValueCloner _createParameters(
     Procedure tearOff,
     Member constructor,
     FunctionNode function,
@@ -326,6 +338,8 @@ void _createParameters(
       tearOff,
       new TypeDependency(tearOff, constructor, substitution,
           copyReturnType: true));
+  return new DelayedDefaultValueCloner(constructor, tearOff,
+      identicalSignatures: true, libraryBuilder: libraryBuilder);
 }
 
 /// Creates the [Arguments] for passing the parameters from [tearOff] to its

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

@@ -131,7 +131,9 @@ class DelayedDefaultValueCloner {
             VariableDeclaration synthesizedParameter =
                 _synthesized.positionalParameters[superParameterIndex];
             _cloneDefaultValueForSuperParameters(
-                originalParameter, synthesizedParameter, typeEnvironment);
+                originalParameter, synthesizedParameter, typeEnvironment,
+                isOptional:
+                    superParameterIndex >= _synthesized.requiredParameterCount);
           }
         }
       }
@@ -162,7 +164,8 @@ class DelayedDefaultValueCloner {
             VariableDeclaration synthesizedParameter =
                 _synthesized.namedParameters[i];
             _cloneDefaultValueForSuperParameters(
-                originalParameter, synthesizedParameter, typeEnvironment);
+                originalParameter, synthesizedParameter, typeEnvironment,
+                isOptional: !synthesizedParameter.isRequired);
           } else {
             // TODO(cstefantsova): Handle the erroneous case of missing names.
           }
@@ -218,7 +221,8 @@ class DelayedDefaultValueCloner {
   void _cloneDefaultValueForSuperParameters(
       VariableDeclaration originalParameter,
       VariableDeclaration synthesizedParameter,
-      TypeEnvironment typeEnvironment) {
+      TypeEnvironment typeEnvironment,
+      {required bool isOptional}) {
     Expression? originalParameterInitializer = originalParameter.initializer;
     DartType? originalParameterInitializerType = originalParameterInitializer
         ?.getStaticType(new StaticTypeContext(synthesized, typeEnvironment));
@@ -227,6 +231,9 @@ class DelayedDefaultValueCloner {
         typeEnvironment.isSubtypeOf(originalParameterInitializerType,
             synthesizedParameterType, SubtypeCheckMode.withNullabilities)) {
       _cloneInitializer(originalParameter, synthesizedParameter);
+    } else if (originalParameterInitializer == null && isOptional) {
+      synthesizedParameter.initializer = new NullLiteral()
+        ..parent = synthesizedParameter;
     } else {
       synthesizedParameter.hasDeclaredInitializer = false;
       if (synthesizedParameterType.isPotentiallyNonNullable) {

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

@@ -521,7 +521,11 @@ class KernelTarget extends TargetImplementation {
       loader.computeShowHideElements();
 
       benchmarker?.enterPhase(BenchmarkPhases.outline_installTypedefTearOffs);
-      loader.installTypedefTearOffs();
+      List<DelayedDefaultValueCloner>?
+          typedefTearOffsDelayedDefaultValueCloners =
+          loader.installTypedefTearOffs();
+      typedefTearOffsDelayedDefaultValueCloners
+          ?.forEach(registerDelayedDefaultValueCloner);
 
       benchmarker
           ?.enterPhase(BenchmarkPhases.outline_computeFieldPromotability);
@@ -1111,12 +1115,15 @@ class KernelTarget extends TargetImplementation {
         forAbstractClassOrEnumOrMixin: classBuilder.isAbstract);
 
     if (constructorTearOff != null) {
-      buildConstructorTearOffProcedure(
-          tearOff: constructorTearOff,
-          declarationConstructor: constructor,
-          implementationConstructor: constructor,
-          enclosingDeclarationTypeParameters: classBuilder.cls.typeParameters,
-          libraryBuilder: libraryBuilder);
+      DelayedDefaultValueCloner delayedDefaultValueCloner =
+          buildConstructorTearOffProcedure(
+              tearOff: constructorTearOff,
+              declarationConstructor: constructor,
+              implementationConstructor: constructor,
+              enclosingDeclarationTypeParameters:
+                  classBuilder.cls.typeParameters,
+              libraryBuilder: libraryBuilder);
+      registerDelayedDefaultValueCloner(delayedDefaultValueCloner);
     }
     SyntheticSourceConstructorBuilder constructorBuilder =
         new SyntheticSourceConstructorBuilder(
@@ -1135,9 +1142,10 @@ class KernelTarget extends TargetImplementation {
   }
 
   void registerDelayedDefaultValueCloner(DelayedDefaultValueCloner cloner) {
-    assert(!_delayedDefaultValueCloners.containsKey(cloner.synthesized),
-        "Default cloner already registered for ${cloner.synthesized}.");
-    _delayedDefaultValueCloners[cloner.synthesized] = cloner;
+    // TODO(cstefantsova): Investigate the reason for the assumption breakage
+    // and uncomment the following line.
+    // assert(!_delayedDefaultValueCloners.containsKey(cloner.synthesized));
+    _delayedDefaultValueCloners[cloner.synthesized] ??= cloner;
   }
 
   void finishSynthesizedParameters({bool forOutline = false}) {
@@ -1190,12 +1198,15 @@ class KernelTarget extends TargetImplementation {
         forAbstractClassOrEnumOrMixin:
             enclosingClass.isAbstract || enclosingClass.isEnum);
     if (constructorTearOff != null) {
-      buildConstructorTearOffProcedure(
-          tearOff: constructorTearOff,
-          declarationConstructor: constructor,
-          implementationConstructor: constructor,
-          enclosingDeclarationTypeParameters: classBuilder.cls.typeParameters,
-          libraryBuilder: libraryBuilder);
+      DelayedDefaultValueCloner delayedDefaultValueCloner =
+          buildConstructorTearOffProcedure(
+              tearOff: constructorTearOff,
+              declarationConstructor: constructor,
+              implementationConstructor: constructor,
+              enclosingDeclarationTypeParameters:
+                  classBuilder.cls.typeParameters,
+              libraryBuilder: libraryBuilder);
+      registerDelayedDefaultValueCloner(delayedDefaultValueCloner);
     }
     return new SyntheticSourceConstructorBuilder(
         classBuilder, constructor, constructorTearOff);

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

@@ -544,12 +544,15 @@ class DeclaredSourceConstructorBuilder
       _constructor.isExternal = isExternal;
 
       if (_constructorTearOff != null) {
-        buildConstructorTearOffProcedure(
-            tearOff: _constructorTearOff,
-            declarationConstructor: constructor,
-            implementationConstructor: _constructor,
-            enclosingDeclarationTypeParameters: classBuilder.cls.typeParameters,
-            libraryBuilder: libraryBuilder);
+        DelayedDefaultValueCloner delayedDefaultValueCloners =
+            buildConstructorTearOffProcedure(
+                tearOff: _constructorTearOff,
+                declarationConstructor: constructor,
+                implementationConstructor: _constructor,
+                enclosingDeclarationTypeParameters:
+                    classBuilder.cls.typeParameters,
+                libraryBuilder: libraryBuilder);
+        _delayedDefaultValueCloners.add(delayedDefaultValueCloners);
       }
 
       _hasBeenBuilt = true;
@@ -616,7 +619,7 @@ class DeclaredSourceConstructorBuilder
 
   final bool _hasSuperInitializingFormals;
 
-  final List<DelayedDefaultValueCloner> _superParameterDefaultValueCloners =
+  final List<DelayedDefaultValueCloner> _delayedDefaultValueCloners =
       <DelayedDefaultValueCloner>[];
 
   @override
@@ -634,7 +637,7 @@ class DeclaredSourceConstructorBuilder
             doFinishConstructor: false);
       }
       finalizeSuperInitializingFormals(
-          hierarchy, _superParameterDefaultValueCloners, initializers);
+          hierarchy, _delayedDefaultValueCloners, initializers);
     }
   }
 
@@ -824,8 +827,8 @@ class DeclaredSourceConstructorBuilder
           .addSuperParameterDefaultValueCloners(delayedDefaultValueCloners);
     }
 
-    delayedDefaultValueCloners.addAll(_superParameterDefaultValueCloners);
-    _superParameterDefaultValueCloners.clear();
+    delayedDefaultValueCloners.addAll(_delayedDefaultValueCloners);
+    _delayedDefaultValueCloners.clear();
   }
 
   @override
@@ -1085,6 +1088,8 @@ class SourceExtensionTypeConstructorBuilder
 
   final MemberName _memberName;
 
+  DelayedDefaultValueCloner? _delayedDefaultValueCloner;
+
   SourceExtensionTypeConstructorBuilder(
       List<MetadataBuilder>? metadata,
       int modifiers,
@@ -1194,6 +1199,10 @@ class SourceExtensionTypeConstructorBuilder
       _buildBody();
     }
     beginInitializers = null;
+
+    if (_delayedDefaultValueCloner != null) {
+      delayedDefaultValueCloners.add(_delayedDefaultValueCloner!);
+    }
   }
 
   bool _hasBuiltBody = false;
@@ -1268,7 +1277,7 @@ class SourceExtensionTypeConstructorBuilder
       _constructor.isExtensionTypeMember = true;
 
       if (_constructorTearOff != null) {
-        buildConstructorTearOffProcedure(
+        _delayedDefaultValueCloner = buildConstructorTearOffProcedure(
             tearOff: _constructorTearOff,
             declarationConstructor: _constructor,
             implementationConstructor: _constructor,

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

@@ -64,6 +64,8 @@ class SourceFactoryBuilder extends SourceFunctionBuilderImpl {
 
   final MemberName _memberName;
 
+  DelayedDefaultValueCloner? _delayedDefaultValueCloner;
+
   SourceFactoryBuilder(
       List<MetadataBuilder>? metadata,
       int modifiers,
@@ -185,7 +187,7 @@ class SourceFactoryBuilder extends SourceFunctionBuilderImpl {
     _procedureInternal.isStatic = isStatic;
 
     if (_factoryTearOff != null) {
-      buildConstructorTearOffProcedure(
+      _delayedDefaultValueCloner = buildConstructorTearOffProcedure(
           tearOff: _factoryTearOff,
           declarationConstructor: _procedure,
           implementationConstructor: _procedureInternal,
@@ -201,6 +203,9 @@ class SourceFactoryBuilder extends SourceFunctionBuilderImpl {
       List<DelayedActionPerformer> delayedActionPerformers,
       List<DelayedDefaultValueCloner> delayedDefaultValueCloners) {
     if (_hasBuiltOutlines) return;
+    if (_delayedDefaultValueCloner != null) {
+      delayedDefaultValueCloners.add(_delayedDefaultValueCloner!);
+    }
     super.buildOutlineExpressions(
         classHierarchy, delayedActionPerformers, delayedDefaultValueCloners);
     _hasBuiltOutlines = true;

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

@@ -5489,25 +5489,39 @@ class SourceLibraryBuilder extends LibraryBuilderImpl {
     }
   }
 
-  void installTypedefTearOffs() {
+  List<DelayedDefaultValueCloner>? installTypedefTearOffs() {
+    List<DelayedDefaultValueCloner>? delayedDefaultValueCloners;
+
     Iterable<SourceLibraryBuilder>? augmentationLibraries =
         this.augmentationLibraries;
     if (augmentationLibraries != null) {
       for (SourceLibraryBuilder augmentationLibrary in augmentationLibraries) {
-        augmentationLibrary.installTypedefTearOffs();
+        List<DelayedDefaultValueCloner>?
+            augmentationLibraryDelayedDefaultValueCloners =
+            augmentationLibrary.installTypedefTearOffs();
+        if (augmentationLibraryDelayedDefaultValueCloners != null) {
+          (delayedDefaultValueCloners ??= [])
+              .addAll(augmentationLibraryDelayedDefaultValueCloners);
+        }
       }
     }
 
     Iterator<SourceTypeAliasBuilder> iterator = localMembersIteratorOfType();
     while (iterator.moveNext()) {
       SourceTypeAliasBuilder declaration = iterator.current;
-      declaration.buildTypedefTearOffs(this, (Procedure procedure) {
+      DelayedDefaultValueCloner? delayedDefaultValueCloner =
+          declaration.buildTypedefTearOffs(this, (Procedure procedure) {
         procedure.isStatic = true;
         if (!declaration.isAugmenting && !declaration.isDuplicate) {
           library.addProcedure(procedure);
         }
       });
+      if (delayedDefaultValueCloner != null) {
+        (delayedDefaultValueCloners ??= []).add(delayedDefaultValueCloner);
+      }
     }
+
+    return delayedDefaultValueCloners;
   }
 }