Revert "Implement Generic Function Instantiation via wrapping node"

This reverts commit 1c8f56f.

Reason for revert: This breaks package:collection at
ListComparableExtensions.binarySearch, at
`comparable ?? compareComparable`:

The argument type 'Function' can't be assigned to the parameter type
'intFunction(E, E)'. #argument_type_not_assignable

Original change's description:
> Implement Generic Function Instantiation via wrapping node
>
> This is a non-breaking change (when analyzing code at language version
> 2.14 and earlier). If constructor-tearoffs is enabled (language version
> 2.15), then FunctionReference nodes are inserted, to represent generic
> function instantiation. Constant evaluation can then use the
> `typeArgumentTypes` to instantiate arbitrary function-typed
> expressions.
>
> If constructor-tearoffs is not enabled, generic function instantiation
> continues to take place at a SimpleIdentifier, PrefixedIdentifier, or
> PropertyAccess only, with constant evaluation using
> SimpleIdentifier.tearOffTypeArgumentTypes.
>
> Bug: #46020
> Change-Id: Ie370c00c8a2ce7a4791ac9cdb7459a01339a79c1
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/217880
> Commit-Queue: Samuel Rawlins <[email protected]>
> Reviewed-by: Brian Wilkerson <[email protected]>
> Reviewed-by: Konstantin Shcheglov <[email protected]>

[email protected],[email protected],[email protected]

Change-Id: Ie76cd703e05cbf65fecaa76b72e829f8272b8307
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: #46020
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/218089
Reviewed-by: Samuel Rawlins <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>

Author: srawlins

pkg/analyzer/lib/src/dart/constant/evaluation.dart

@@ -682,37 +682,27 @@ class ConstantVisitor extends UnifyingAstVisitor<DartObjectImpl> {
     if (functionResult == null) {
       return functionResult;
     }
-
-    // Report an error if any of the _inferred_ type argument types refer to a
-    // type parameter. If, however, `node.typeArguments` is not `null`, then
-    // any type parameters contained therein are reported as non-constant in
-    // [ConstantVerifier].
-    if (node.typeArguments == null &&
-        (node.typeArgumentTypes?.any(hasTypeParameterReference) ?? false)) {
-      _error(node, null);
-    }
-
     var typeArgumentList = node.typeArguments;
     if (typeArgumentList == null) {
-      return _instantiateFunctionType(node, functionResult);
-    }
-
-    var typeArguments = <DartType>[];
-    for (var typeArgument in typeArgumentList.arguments) {
-      var object = typeArgument.accept(this);
-      if (object == null) {
-        return null;
-      }
-      var typeArgumentType = object.toTypeValue();
-      if (typeArgumentType == null) {
-        return null;
+      return functionResult;
+    } else {
+      var typeArguments = <DartType>[];
+      for (var typeArgument in typeArgumentList.arguments) {
+        var object = typeArgument.accept(this);
+        if (object == null) {
+          return null;
+        }
+        var typeArgumentType = object.toTypeValue();
+        if (typeArgumentType == null) {
+          return null;
+        }
+        // TODO(srawlins): Test type alias types (`typedef i = int`) used as
+        // type arguments. Possibly change implementation based on
+        // canonicalization rules.
+        typeArguments.add(typeArgumentType);
       }
-      // TODO(srawlins): Test type alias types (`typedef i = int`) used as
-      // type arguments. Possibly change implementation based on
-      // canonicalization rules.
-      typeArguments.add(typeArgumentType);
+      return _dartObjectComputer.typeInstantiate(functionResult, typeArguments);
     }
-    return _dartObjectComputer.typeInstantiate(functionResult, typeArguments);
   }
 
   @override
@@ -1010,7 +1000,7 @@ class ConstantVisitor extends UnifyingAstVisitor<DartObjectImpl> {
   DartObjectImpl? visitSimpleIdentifier(SimpleIdentifier node) {
     var value = _lexicalEnvironment?[node.name];
     if (value != null) {
-      return _instantiateFunctionTypeForSimpleIdentifier(node, value);
+      return _instantiateFunctionType(node, value);
     }
 
     return _getConstantValue(node, node);
@@ -1222,8 +1212,6 @@ class ConstantVisitor extends UnifyingAstVisitor<DartObjectImpl> {
     var variableElement =
         element is PropertyAccessorElement ? element.variable : element;
 
-    // TODO(srawlins): Remove this check when [FunctionReference]s are inserted
-    // for generic function instantiation for pre-constructor-references code.
     if (node is SimpleIdentifier &&
         (node.tearOffTypeArgumentTypes?.any(hasTypeParameterReference) ??
             false)) {
@@ -1242,7 +1230,7 @@ class ConstantVisitor extends UnifyingAstVisitor<DartObjectImpl> {
         if (value == null) {
           return value;
         }
-        return _instantiateFunctionTypeForSimpleIdentifier(identifier, value);
+        return _instantiateFunctionType(identifier, value);
       }
     } else if (variableElement is ConstructorElement) {
       return DartObjectImpl(
@@ -1258,7 +1246,7 @@ class ConstantVisitor extends UnifyingAstVisitor<DartObjectImpl> {
           function.type,
           FunctionState(function),
         );
-        return _instantiateFunctionTypeForSimpleIdentifier(identifier, rawType);
+        return _instantiateFunctionType(identifier, rawType);
       }
     } else if (variableElement is ClassElement) {
       var type = variableElement.instantiate(
@@ -1317,41 +1305,12 @@ class ConstantVisitor extends UnifyingAstVisitor<DartObjectImpl> {
     return null;
   }
 
-  /// If the type of [value] is a generic [FunctionType], and [node] has type
-  /// argument types, returns [value] type-instantiated with those [node]'s
-  /// type argument types, otherwise returns [value].
-  DartObjectImpl? _instantiateFunctionType(
-      FunctionReference node, DartObjectImpl value) {
-    var functionElement = value.toFunctionValue();
-    if (functionElement is! ExecutableElement) {
-      return value;
-    }
-    var valueType = functionElement.type;
-    if (valueType.typeFormals.isNotEmpty) {
-      var typeArgumentTypes = node.typeArgumentTypes;
-      if (typeArgumentTypes != null && typeArgumentTypes.isNotEmpty) {
-        var instantiatedType =
-            functionElement.type.instantiate(typeArgumentTypes);
-        var substitution = _substitution;
-        if (substitution != null) {
-          instantiatedType =
-              substitution.substituteType(instantiatedType) as FunctionType;
-        }
-        return value.typeInstantiate(
-            typeSystem, instantiatedType, typeArgumentTypes);
-      }
-    }
-    return value;
-  }
-
   /// If the type of [value] is a generic [FunctionType], and [node] is a
   /// [SimpleIdentifier] with tear-off type argument types, returns [value]
   /// type-instantiated with those [node]'s tear-off type argument types,
   /// otherwise returns [value].
-  DartObjectImpl? _instantiateFunctionTypeForSimpleIdentifier(
+  DartObjectImpl? _instantiateFunctionType(
       SimpleIdentifier node, DartObjectImpl value) {
-    // TODO(srawlins): When all code uses [FunctionReference]s generated via
-    // generic function instantiation, remove this method and all call sites.
     var functionElement = value.toFunctionValue();
     if (functionElement is! ExecutableElement) {
       return value;

pkg/analyzer/lib/src/dart/resolver/assignment_expression_resolver.dart

@@ -242,12 +242,9 @@ class AssignmentExpressionResolver {
     }
 
     _inferenceHelper.recordStaticType(node, nodeType);
-    var wrappingExpression =
-        _resolver.insertImplicitCallReference(rightHandSide);
-    wrappingExpression =
-        _resolver.insertGenericFunctionInstantiation(wrappingExpression);
-    if (wrappingExpression != rightHandSide) {
-      assignedType = wrappingExpression.typeOrThrow;
+    var callReference = _resolver.insertImplicitCallReference(rightHandSide);
+    if (callReference != rightHandSide) {
+      assignedType = callReference.typeOrThrow;
     }
 
     // TODO(scheglov) Remove from ErrorVerifier?

pkg/analyzer/lib/src/dart/resolver/prefixed_identifier_resolver.dart

@@ -83,15 +83,8 @@ class PrefixedIdentifierResolver {
       type = result.functionTypeCallType!;
     }
 
-    if (!_resolver.isConstructorTearoffsEnabled) {
-      // Only perform a generic function instantiation on a [PrefixedIdentifier]
-      // in pre-constructor-tearoffs code. In constructor-tearoffs-enabled code,
-      // generic function instantiation is performed at assignability check
-      // sites.
-      // TODO(srawlins): Switch all resolution to use the latter method, in a
-      // breaking change release.
-      type = _inferenceHelper.inferTearOff(node, identifier, type);
-    }
+    type = _inferenceHelper.inferTearOff(node, identifier, type);
+
     _recordStaticType(identifier, type);
     _recordStaticType(node, type);
   }

pkg/analyzer/lib/src/dart/resolver/simple_identifier_resolver.dart

@@ -12,6 +12,7 @@ import 'package:analyzer/src/dart/element/element.dart';
 import 'package:analyzer/src/dart/element/type.dart';
 import 'package:analyzer/src/dart/element/type_provider.dart';
 import 'package:analyzer/src/dart/resolver/flow_analysis_visitor.dart';
+import 'package:analyzer/src/dart/resolver/invocation_inference_helper.dart';
 import 'package:analyzer/src/dart/resolver/property_element_resolver.dart';
 import 'package:analyzer/src/error/codes.dart';
 import 'package:analyzer/src/generated/resolver.dart';
@@ -24,6 +25,8 @@ class SimpleIdentifierResolver {
 
   ErrorReporter get _errorReporter => _resolver.errorReporter;
 
+  InvocationInferenceHelper get _inferenceHelper => _resolver.inferenceHelper;
+
   TypeProviderImpl get _typeProvider => _resolver.typeProvider;
 
   void resolve(SimpleIdentifierImpl node) {
@@ -256,17 +259,7 @@ class SimpleIdentifierResolver {
     } else {
       staticType = DynamicTypeImpl.instance;
     }
-
-    if (!_resolver.isConstructorTearoffsEnabled) {
-      // Only perform a generic function instantiation on a [PrefixedIdentifier]
-      // in pre-constructor-tearoffs code. In constructor-tearoffs-enabled code,
-      // generic function instantiation is performed at assignability check
-      // sites.
-      // TODO(srawlins): Switch all resolution to use the latter method, in a
-      // breaking change release.
-      staticType =
-          _resolver.inferenceHelper.inferTearOff(node, node, staticType);
-    }
+    staticType = _inferenceHelper.inferTearOff(node, node, staticType);
     _recordStaticType(node, staticType);
   }
 

pkg/analyzer/lib/src/dart/resolver/typed_literal_resolver.dart

@@ -601,8 +601,7 @@ class TypedLiteralResolver {
 
   void _insertImplicitCallReference(CollectionElement? node) {
     if (node is Expression) {
-      var wrappingExpression = _resolver.insertImplicitCallReference(node);
-      _resolver.insertGenericFunctionInstantiation(wrappingExpression);
+      _resolver.insertImplicitCallReference(node);
     } else if (node is MapLiteralEntry) {
       _insertImplicitCallReference(node.key);
       _insertImplicitCallReference(node.value);

pkg/analyzer/lib/src/dart/resolver/variable_declaration_resolver.dart

@@ -65,15 +65,17 @@ class VariableDeclarationResolver {
       _resolver.flowAnalysis.flow?.lateInitializer_end();
     }
 
-    initializer = _resolver.insertImplicitCallReference(initializer);
-    initializer = _resolver.insertGenericFunctionInstantiation(initializer);
-
     // Initializers of top-level variables and fields are already included
     // into elements during linking.
     if (element is ConstLocalVariableElementImpl) {
       element.constantInitializer = initializer;
     }
 
+    var callReference = _resolver.insertImplicitCallReference(initializer);
+    if (callReference != initializer) {
+      initializer = callReference;
+    }
+
     _resolver.checkForInvalidAssignment(node.name, initializer,
         whyNotPromoted: whyNotPromoted);
   }

pkg/analyzer/lib/src/generated/resolver.dart

@@ -459,10 +459,8 @@ class ResolverVisitor extends ResolverBase with ErrorDetectionHelpers {
     for (var argument in argumentList.arguments) {
       if (argument is NamedExpression) {
         insertImplicitCallReference(argument.expression);
-        insertGenericFunctionInstantiation(argument.expression);
       } else {
         insertImplicitCallReference(argument);
-        insertGenericFunctionInstantiation(argument);
       }
     }
     var arguments = argumentList.arguments;
@@ -630,62 +628,6 @@ class ResolverVisitor extends ResolverBase with ErrorDetectionHelpers {
     return null;
   }
 
-  /// If generic function instantiation should be performed on `expression`,
-  /// inserts a [FunctionReference] node which wraps [expression].
-  ///
-  /// If an [FunctionReference] is inserted, returns it; otherwise, returns
-  /// [expression].
-  ExpressionImpl insertGenericFunctionInstantiation(Expression expression) {
-    expression as ExpressionImpl;
-    if (!isConstructorTearoffsEnabled) {
-      // Temporarily, only create [ImplicitCallReference] nodes under the
-      // 'constructor-tearoffs' feature.
-      // TODO(srawlins): When we are ready to make a breaking change release to
-      // the analyzer package, remove this exception.
-      return expression;
-    }
-
-    var staticType = expression.staticType;
-    var context = InferenceContext.getContext(expression);
-    if (context == null ||
-        staticType is! FunctionType ||
-        staticType.typeFormals.isEmpty) {
-      return expression;
-    }
-
-    context = typeSystem.flatten(context);
-    if (context is! FunctionType || context.typeFormals.isNotEmpty) {
-      return expression;
-    }
-
-    List<DartType> typeArgumentTypes =
-        typeSystem.inferFunctionTypeInstantiation(
-      context,
-      staticType,
-      errorReporter: errorReporter,
-      errorNode: expression,
-      // If the constructor-tearoffs feature is enabled, then so is
-      // generic-metadata.
-      genericMetadataIsEnabled: true,
-    )!;
-    if (typeArgumentTypes.isNotEmpty) {
-      staticType = staticType.instantiate(typeArgumentTypes);
-    }
-
-    var parent = expression.parent;
-    var genericFunctionInstantiation = astFactory.functionReference(
-      function: expression,
-      typeArguments: null,
-    ) as FunctionReferenceImpl;
-    NodeReplacer.replace(expression, genericFunctionInstantiation,
-        parent: parent);
-
-    genericFunctionInstantiation.typeArgumentTypes = typeArgumentTypes;
-    genericFunctionInstantiation.staticType = staticType;
-
-    return genericFunctionInstantiation;
-  }
-
   /// If `expression` should be treated as `expression.call`, inserts an
   /// [ImplicitCallReferece] node which wraps [expression].
   ///
@@ -725,7 +667,6 @@ class ResolverVisitor extends ResolverBase with ErrorDetectionHelpers {
       typeArgumentTypes = typeSystem.inferFunctionTypeInstantiation(
         context,
         callMethodType,
-        errorReporter: errorReporter,
         errorNode: expression,
         // If the constructor-tearoffs feature is enabled, then so is
         // generic-metadata.
@@ -1364,11 +1305,9 @@ class ResolverVisitor extends ResolverBase with ErrorDetectionHelpers {
     node.accept(typeAnalyzer);
     if (fieldElement != null) {
       if (fieldType != null && expression.staticType != null) {
-        var wrappingExpression = insertImplicitCallReference(expression);
-        wrappingExpression =
-            insertGenericFunctionInstantiation(wrappingExpression);
-        if (expression != wrappingExpression) {
-          checkForInvalidAssignment(node.fieldName, wrappingExpression,
+        var callReference = insertImplicitCallReference(expression);
+        if (expression != callReference) {
+          checkForInvalidAssignment(node.fieldName, callReference,
               whyNotPromoted: whyNotPromoted);
         }
       }
@@ -1406,13 +1345,8 @@ class ResolverVisitor extends ResolverBase with ErrorDetectionHelpers {
   void visitDefaultFormalParameter(DefaultFormalParameter node) {
     InferenceContext.setType(node.defaultValue, node.declaredElement?.type);
     super.visitDefaultFormalParameter(node);
-    var defaultValue = node.defaultValue;
-    if (defaultValue != null) {
-      // TODO(srawlins): Presumably we should also perform implicit tear-off
-      // conversion here, e.g. `f(Function p = C()])`.
-      insertGenericFunctionInstantiation(defaultValue);
-    }
     ParameterElement element = node.declaredElement!;
+
     if (element is DefaultParameterElementImpl && node.isOfLocalFunction) {
       element.constantInitializer = node.defaultValue;
     }
@@ -1485,7 +1419,6 @@ class ResolverVisitor extends ResolverBase with ErrorDetectionHelpers {
 
       super.visitExpressionFunctionBody(node);
       insertImplicitCallReference(node.expression);
-      insertGenericFunctionInstantiation(node.expression);
 
       flowAnalysis.flow?.handleExit();
 
@@ -1956,15 +1889,7 @@ class ResolverVisitor extends ResolverBase with ErrorDetectionHelpers {
       type = DynamicTypeImpl.instance;
     }
 
-    if (!isConstructorTearoffsEnabled) {
-      // Only perform a generic function instantiation on a [PrefixedIdentifier]
-      // in pre-constructor-tearoffs code. In constructor-tearoffs-enabled code,
-      // generic function instantiation is performed at assignability check
-      // sites.
-      // TODO(srawlins): Switch all resolution to use the latter method, in a
-      // breaking change release.
-      type = inferenceHelper.inferTearOff(node, propertyName, type);
-    }
+    type = inferenceHelper.inferTearOff(node, propertyName, type);
 
     inferenceHelper.recordStaticType(propertyName, type);
     inferenceHelper.recordStaticType(node, type);
@@ -2010,8 +1935,7 @@ class ResolverVisitor extends ResolverBase with ErrorDetectionHelpers {
 
     var expression = node.expression;
     if (expression != null) {
-      expression = insertImplicitCallReference(expression);
-      insertGenericFunctionInstantiation(expression);
+      insertImplicitCallReference(expression);
     }
   }