Defer analysis of closure arguments when inference-update-1 is enab…

…led.

In order to address dart-lang/language#731
(improved type inference for `fold` etc.) we're going to need to
sometimes defer analysis of invocation arguments that are closures, so
that closure parameters can have their types inferred based on other
parameters.  To avoid annoying the user with inconsistent behaviors,
we defer analysis of closures in all circumstances, even if it's not
necessary to do so for type inference purposes.

This has a minor user-visible effect: if an invocation contains some
closures and some non-closures, any demotions that happen due to write
captures in the closures are postponed until the end of the
invocation; this means that the write-captured variables remain
promoted for other invocation arguments, even if those arguments
appear after the closure.  This is safe because there is no way for
the closure to be called until after all of the other invocation
arguments are evaluated.  See the language tests in
tests/language/inference_update_1/write_capture_deferral_enabled_test.dart
for details.

Note that this change only has an effect when the experimental feature
`inference-update-1` is enabled.

Change-Id: If7bb38e361755180c033ecb2108fc4fffa7570b1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/241864
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>

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

@@ -463,6 +463,11 @@ class SourceLibraryBuilder extends LibraryBuilderImpl {
     return type;
   }
 
+  bool get isInferenceUpdate1Enabled =>
+      libraryFeatures.inferenceUpdate1.isSupported &&
+      languageVersion.version >=
+          libraryFeatures.inferenceUpdate1.enabledVersion;
+
   bool? _isNonNullableByDefault;
 
   @override

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

@@ -250,6 +250,9 @@ class TypeInferrerImpl implements TypeInferrer {
 
   CoreTypes get coreTypes => engine.coreTypes;
 
+  bool get isInferenceUpdate1Enabled =>
+      libraryBuilder.isInferenceUpdate1Enabled;
+
   bool get isNonNullableByDefault => libraryBuilder.isNonNullableByDefault;
 
   NnbdMode get nnbdMode => libraryBuilder.loader.nnbdMode;
@@ -2395,10 +2398,41 @@ class TypeInferrerImpl implements TypeInferrer {
       hoistingEndIndex = 0;
     }
 
+    ExpressionInferenceResult inferArgument(
+        DartType formalType, Expression argumentExpression,
+        {required bool isNamed}) {
+      DartType inferredFormalType = substitution != null
+          ? substitution.substituteType(formalType)
+          : formalType;
+      if (!isNamed) {
+        if (isSpecialCasedBinaryOperator) {
+          inferredFormalType =
+              typeSchemaEnvironment.getContextTypeOfSpecialCasedBinaryOperator(
+                  typeContext, receiverType!, inferredFormalType,
+                  isNonNullableByDefault: isNonNullableByDefault);
+        } else if (isSpecialCasedTernaryOperator) {
+          inferredFormalType =
+              typeSchemaEnvironment.getContextTypeOfSpecialCasedTernaryOperator(
+                  typeContext, receiverType!, inferredFormalType,
+                  isNonNullableByDefault: isNonNullableByDefault);
+        }
+      }
+      return inferExpression(
+          argumentExpression,
+          isNonNullableByDefault
+              ? inferredFormalType
+              : legacyErasure(inferredFormalType),
+          inferenceNeeded ||
+              isSpecialCasedBinaryOperator ||
+              isSpecialCasedTernaryOperator ||
+              typeChecksNeeded);
+    }
+
     List<EqualityInfo<VariableDeclaration, DartType>?>? identicalInfo =
         isIdentical && arguments.positional.length == 2 ? [] : null;
     int positionalIndex = 0;
     int namedIndex = 0;
+    List<_DeferredParamInfo>? deferredFunctionLiterals;
     for (int evaluationOrderIndex = 0;
         evaluationOrderIndex < argumentsEvaluationOrder.length;
         evaluationOrderIndex++) {
@@ -2421,60 +2455,77 @@ class TypeInferrerImpl implements TypeInferrer {
         formalType = getNamedParameterType(calleeType, namedArgument.name);
         argumentExpression = namedArgument.value;
       }
-      DartType inferredFormalType = substitution != null
-          ? substitution.substituteType(formalType)
-          : formalType;
-      if (isExpression) {
-        if (isImplicitExtensionMember && index == 0) {
-          assert(
-              receiverType != null,
-              "No receiver type provided for implicit extension member "
-              "invocation.");
-          continue;
-        }
-        if (isSpecialCasedBinaryOperator) {
-          inferredFormalType =
-              typeSchemaEnvironment.getContextTypeOfSpecialCasedBinaryOperator(
-                  typeContext, receiverType!, inferredFormalType,
-                  isNonNullableByDefault: isNonNullableByDefault);
-        } else if (isSpecialCasedTernaryOperator) {
-          inferredFormalType =
-              typeSchemaEnvironment.getContextTypeOfSpecialCasedTernaryOperator(
-                  typeContext, receiverType!, inferredFormalType,
-                  isNonNullableByDefault: isNonNullableByDefault);
-        }
-      }
-      ExpressionInferenceResult result = inferExpression(
-          argumentExpression,
-          isNonNullableByDefault
-              ? inferredFormalType
-              : legacyErasure(inferredFormalType),
-          inferenceNeeded ||
-              isSpecialCasedBinaryOperator ||
-              isSpecialCasedTernaryOperator ||
-              typeChecksNeeded);
-      DartType inferredType = identical(result.inferredType, noInferredType) ||
-              isNonNullableByDefault
-          ? result.inferredType
-          : legacyErasure(result.inferredType);
-      if (localHoistedExpressions != null &&
-          evaluationOrderIndex >= hoistingEndIndex) {
-        hoistedExpressions = null;
+      if (isExpression && isImplicitExtensionMember && index == 0) {
+        assert(
+            receiverType != null,
+            "No receiver type provided for implicit extension member "
+            "invocation.");
+        continue;
       }
-      Expression expression =
-          _hoist(result.expression, inferredType, hoistedExpressions);
-      identicalInfo
-          ?.add(flowAnalysis.equalityOperand_end(expression, inferredType));
-      if (isExpression) {
-        arguments.positional[index] = expression..parent = arguments;
+      if (isInferenceUpdate1Enabled &&
+          argumentExpression is FunctionExpression) {
+        (deferredFunctionLiterals ??= []).add(new _DeferredParamInfo(
+            formalType: formalType,
+            argumentExpression: argumentExpression,
+            isNamed: !isExpression,
+            evaluationOrderIndex: evaluationOrderIndex,
+            index: index));
+        // We don't have `identical` info yet, so fill it in with `null` for
+        // now.  Later, when we visit the function literal, we'll replace it.
+        identicalInfo?.add(null);
+        if (useFormalAndActualTypes) {
+          formalTypes!.add(formalType);
+          // We don't have an inferred type yet, so fill it in with UnknownType
+          // for now.  Later, when we infer a type, we'll replace it.
+          actualTypes!.add(const UnknownType());
+        }
       } else {
-        NamedExpression namedArgument = arguments.named[index];
-        namedArgument.value = expression..parent = namedArgument;
+        ExpressionInferenceResult result = inferArgument(
+            formalType, argumentExpression,
+            isNamed: !isExpression);
+        DartType inferredType = _computeInferredType(result);
+        if (localHoistedExpressions != null &&
+            evaluationOrderIndex >= hoistingEndIndex) {
+          hoistedExpressions = null;
+        }
+        Expression expression =
+            _hoist(result.expression, inferredType, hoistedExpressions);
+        identicalInfo
+            ?.add(flowAnalysis.equalityOperand_end(expression, inferredType));
+        if (isExpression) {
+          arguments.positional[index] = expression..parent = arguments;
+        } else {
+          NamedExpression namedArgument = arguments.named[index];
+          namedArgument.value = expression..parent = namedArgument;
+        }
+        gatherer?.tryConstrainLower(formalType, inferredType);
+        if (useFormalAndActualTypes) {
+          formalTypes!.add(formalType);
+          actualTypes!.add(inferredType);
+        }
       }
-      gatherer?.tryConstrainLower(formalType, inferredType);
-      if (useFormalAndActualTypes) {
-        formalTypes!.add(formalType);
-        actualTypes!.add(inferredType);
+    }
+    if (deferredFunctionLiterals != null) {
+      for (_DeferredParamInfo deferredArgument in deferredFunctionLiterals) {
+        ExpressionInferenceResult result = inferArgument(
+            deferredArgument.formalType, deferredArgument.argumentExpression,
+            isNamed: deferredArgument.isNamed);
+        DartType inferredType = _computeInferredType(result);
+        Expression expression = result.expression;
+        identicalInfo?[deferredArgument.evaluationOrderIndex] =
+            flowAnalysis.equalityOperand_end(expression, inferredType);
+        if (deferredArgument.isNamed) {
+          NamedExpression namedArgument =
+              arguments.named[deferredArgument.index];
+          namedArgument.value = expression..parent = namedArgument;
+        } else {
+          arguments.positional[deferredArgument.index] = expression
+            ..parent = arguments;
+        }
+        gatherer?.tryConstrainLower(deferredArgument.formalType, inferredType);
+        if (useFormalAndActualTypes) {
+          actualTypes![deferredArgument.evaluationOrderIndex] = inferredType;
+        }
       }
     }
     if (identicalInfo != null) {
@@ -4718,6 +4769,11 @@ class TypeInferrerImpl implements TypeInferrer {
   Expression createEqualsNull(int fileOffset, Expression left) {
     return new EqualsNull(left)..fileOffset = fileOffset;
   }
+
+  DartType _computeInferredType(ExpressionInferenceResult result) =>
+      identical(result.inferredType, noInferredType) || isNonNullableByDefault
+          ? result.inferredType
+          : legacyErasure(result.inferredType);
 }
 
 class TypeInferrerImplBenchmarked implements TypeInferrer {
@@ -5877,3 +5933,33 @@ class ImplicitInstantiation {
   ImplicitInstantiation(
       this.typeArguments, this.functionType, this.instantiatedType);
 }
+
+/// Information about an invocation argument that needs to be resolved later due
+/// to the fact that it's a function literal and the `inference-update-1`
+/// feature is enabled.
+class _DeferredParamInfo {
+  /// The (unsubstituted) type of the formal parameter corresponding to this
+  /// argument.
+  final DartType formalType;
+
+  /// The function literal expression.
+  final FunctionExpression argumentExpression;
+
+  /// Indicates whether this is a named argument.
+  final bool isNamed;
+
+  /// The index into the full argument list (considering both named and unnamed
+  /// arguments) of the function literal expression.
+  final int evaluationOrderIndex;
+
+  /// The index into either [Arguments.named] or [Arguments.positional] of the
+  /// function literal expression (depending upon the value of [isNamed]).
+  final int index;
+
+  _DeferredParamInfo(
+      {required this.formalType,
+      required this.argumentExpression,
+      required this.isNamed,
+      required this.evaluationOrderIndex,
+      required this.index});
+}

pkg/front_end/test/spell_checking_list_code.txt

@@ -248,6 +248,7 @@ configs
 configured
 connectivity
 consideration
+considering
 constness
 constraining
 consult
@@ -1441,6 +1442,7 @@ unset
 unshadowed
 unsortable
 unsound
+unsubstituted
 untouched
 unwrapper
 unwraps

pkg/front_end/test/spell_checking_list_tests.txt

@@ -341,6 +341,7 @@ implemen
 implementor
 implementors
 imprecision
+improvement
 inclosure
 inconsistencies
 increasing

pkg/front_end/testcases/inference_update_1/folder.options

@@ -0,0 +1 @@
+--enable-experiment=inference-update-1

pkg/front_end/testcases/inference_update_1/write_capture_deferral.dart

@@ -0,0 +1,66 @@
+// Copyright (c) 2022, 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.
+
+// Tests that when the feature is enabled, if an invocation argument is a
+// closure, write captures made by that closure do not take effect until after
+// the invocation.  This is a minor improvement to flow analysis that falls
+// naturally out of the fact that closures are analyzed last (so that their
+// types can depend on the types of other arguments).
+
+withUnnamedArguments(int? i, void Function(void Function(), Object?) f) {
+  if (i != null) {
+    f(() {
+      i = null;
+    }, i);
+    i;
+  }
+}
+
+withNamedArguments(
+    int? i, void Function({required void Function() g, Object? x}) f) {
+  if (i != null) {
+    f(
+        g: () {
+          i = null;
+        },
+        x: i);
+    i;
+  }
+}
+
+withIdentical_lhs(int? i) {
+  if (i != null) {
+    i;
+    identical(() {
+      i = null;
+    }, i);
+    i;
+  }
+}
+
+withIdentical_rhs(int? i) {
+  if (i != null) {
+    identical(i, () {
+      i = null;
+    });
+    i;
+  }
+}
+
+class B {
+  B(Object? x, void Function() g, Object? y);
+  B.redirectingConstructorInvocation(int? i)
+      : this(i!, () {
+          i = null;
+        }, i);
+}
+
+class C extends B {
+  C.superConstructorInvocation(int? i)
+      : super(i!, () {
+          i = null;
+        }, i);
+}
+
+main() {}

pkg/front_end/testcases/inference_update_1/write_capture_deferral.dart.textual_outline.expect

@@ -0,0 +1,22 @@
+withUnnamedArguments(int? i, void Function(void Function(), Object?) f) {}
+withNamedArguments(
+    int? i, void Function({required void Function() g, Object? x}) f) {}
+withIdentical_lhs(int? i) {}
+withIdentical_rhs(int? i) {}
+
+class B {
+  B(Object? x, void Function() g, Object? y);
+  B.redirectingConstructorInvocation(int? i)
+      : this(i!, () {
+          i = null;
+        }, i);
+}
+
+class C extends B {
+  C.superConstructorInvocation(int? i)
+      : super(i!, () {
+          i = null;
+        }, i);
+}
+
+main() {}

pkg/front_end/testcases/inference_update_1/write_capture_deferral.dart.textual_outline_modelled.expect

@@ -0,0 +1,21 @@
+class B {
+  B(Object? x, void Function() g, Object? y);
+  B.redirectingConstructorInvocation(int? i)
+      : this(i!, () {
+          i = null;
+        }, i);
+}
+
+class C extends B {
+  C.superConstructorInvocation(int? i)
+      : super(i!, () {
+          i = null;
+        }, i);
+}
+
+main() {}
+withIdentical_lhs(int? i) {}
+withIdentical_rhs(int? i) {}
+withNamedArguments(
+    int? i, void Function({required void Function() g, Object? x}) f) {}
+withUnnamedArguments(int? i, void Function(void Function(), Object?) f) {}