Revert "Revert "Preserve type variables in closure conversion.""

This reverts commit fd4a0c658f.

pkg/kernel/binary.md

@@ -741,6 +741,7 @@ type ClosureCreation extends Expression {
   MemberReference topLevelFunctionReference;
   Expression contextVector;
   FunctionType functionType;
+  List<DartType> typeArguments;
 }
 
 abstract type Statement extends Node {}

pkg/kernel/lib/ast.dart

@@ -3080,14 +3080,15 @@ class ClosureCreation extends Expression {
   Reference topLevelFunctionReference;
   Expression contextVector;
   FunctionType functionType;
+  List<DartType> typeArguments;
 
   ClosureCreation(Member topLevelFunction, Expression contextVector,
-      FunctionType functionType)
-      : this.byReference(
-            getMemberReference(topLevelFunction), contextVector, functionType);
+      FunctionType functionType, List<DartType> typeArguments)
+      : this.byReference(getMemberReference(topLevelFunction), contextVector,
+            functionType, typeArguments);
 
-  ClosureCreation.byReference(
-      this.topLevelFunctionReference, this.contextVector, this.functionType) {
+  ClosureCreation.byReference(this.topLevelFunctionReference,
+      this.contextVector, this.functionType, this.typeArguments) {
     contextVector?.parent = this;
   }
 
@@ -3102,6 +3103,8 @@ class ClosureCreation extends Expression {
 
   visitChildren(Visitor v) {
     contextVector?.accept(v);
+    functionType.accept(v);
+    visitList(typeArguments, v);
   }
 
   transformChildren(Transformer v) {
@@ -3109,6 +3112,8 @@ class ClosureCreation extends Expression {
       contextVector = contextVector.accept(v);
       contextVector?.parent = this;
     }
+    functionType = v.visitDartType(functionType);
+    transformTypeList(typeArguments, v);
   }
 
   DartType getStaticType(TypeEnvironment types) {

pkg/kernel/lib/binary/ast_from_binary.dart

@@ -937,8 +937,9 @@ class BinaryBuilder {
         var topLevelFunctionReference = readMemberReference();
         var contextVector = readExpression();
         var functionType = readDartType();
+        var typeArgs = readDartTypeList();
         return new ClosureCreation.byReference(
-            topLevelFunctionReference, contextVector, functionType);
+            topLevelFunctionReference, contextVector, functionType, typeArgs);
       default:
         throw fail('Invalid expression tag: $tag');
     }

pkg/kernel/lib/binary/ast_to_binary.dart

@@ -872,6 +872,7 @@ class BinaryPrinter extends Visitor {
     writeReference(node.topLevelFunctionReference);
     writeNode(node.contextVector);
     writeNode(node.functionType);
+    writeNodeList(node.typeArguments);
   }
 
   writeStatementOrEmpty(Statement node) {

pkg/kernel/lib/clone.dart

@@ -235,7 +235,8 @@ class CloneVisitor extends TreeVisitor {
     return new ClosureCreation.byReference(
         node.topLevelFunctionReference,
         cloneOptional(node.contextVector),
-        visitOptionalType(node.functionType));
+        visitOptionalType(node.functionType),
+        node.typeArguments.map(visitType).toList());
   }
 
   visitVectorSet(VectorSet node) {

pkg/kernel/lib/text/ast_to_text.dart

@@ -1061,6 +1061,8 @@ class Printer extends Visitor<Null> {
     writeWord('MakeClosure');
     writeSymbol('<');
     writeNode(node.functionType);
+    if (node.typeArguments.length > 0) writeSymbol(', ');
+    writeList(node.typeArguments, writeType);
     writeSymbol('>');
     writeSymbol('(');
     writeMemberReference(node.topLevelFunction);

pkg/kernel/lib/transformations/closure/converter.dart

@@ -13,7 +13,6 @@ import '../../ast.dart'
         ClosureCreation,
         Constructor,
         DartType,
-        DynamicType,
         EmptyStatement,
         Expression,
         ExpressionStatement,
@@ -260,39 +259,12 @@ class ClosureConverter extends Transformer {
     context = context.toNestedContext(
         new VariableAccessor(contextVariable, null, TreeNode.noOffset));
 
-    Set<TypeParameter> captured = capturedTypeVariables[currentFunction];
-    if (captured != null) {
-      typeSubstitution = copyTypeVariables(captured);
-    } else {
-      typeSubstitution = const <TypeParameter, DartType>{};
-    }
+    Set<TypeParameter> captured =
+        capturedTypeVariables[currentFunction] ?? new Set<TypeParameter>();
+    typeSubstitution = copyTypeVariables(captured);
 
-    // TODO(29181): remove replacementTypeSubstitution variable and its usages.
-    // All the type variables used in this function body are replaced with
-    // either dynamic or their bounds. This is to temporarily remove the type
-    // variables from closure conversion. They should be returned after the VM
-    // changes are done to support vectors and closure creation. See #29181.
-    Map<TypeParameter, DartType> replacementTypeSubstitution =
-        <TypeParameter, DartType>{};
-    for (TypeParameter parameter in typeSubstitution.keys) {
-      replacementTypeSubstitution[parameter] = const DynamicType();
-    }
-    for (TypeParameter parameter in typeSubstitution.keys) {
-      if (!isObject(parameter.bound)) {
-        replacementTypeSubstitution[parameter] =
-            substitute(parameter.bound, replacementTypeSubstitution);
-      }
-    }
-    typeSubstitution = replacementTypeSubstitution;
     function.transformChildren(this);
 
-    // TODO(29181): don't replace typeSubstitution with an empty map.
-    // Information about captured type variables is deleted from the closure
-    // class, because the type variables in this function body are already
-    // replaced with either dynamic or their bounds. This change should be
-    // undone after the VM support for vectors and closure creation is
-    // implemented. See #29181.
-    typeSubstitution = <TypeParameter, DartType>{};
     Expression result = addClosure(function, contextVariable, parent.expression,
         typeSubstitution, enclosingTypeSubstitution);
     currentFunction = enclosingFunction;
@@ -322,9 +294,7 @@ class ClosureConverter extends Transformer {
   }
 
   TreeNode visitFunctionExpression(FunctionExpression node) {
-    return saveContext(() {
-      return handleLocalFunction(node.function);
-    });
+    return saveContext(() => handleLocalFunction(node.function));
   }
 
   /// Add a new procedure to the current library that looks like this:
@@ -346,6 +316,16 @@ class ClosureConverter extends Transformer {
       Expression accessContext,
       Map<TypeParameter, DartType> substitution,
       Map<TypeParameter, DartType> enclosingTypeSubstitution) {
+    var fnTypeParams = <TypeParameter>[];
+    var fnTypeArgs = <TypeParameterType>[];
+    for (TypeParameter t in substitution.keys) {
+      var fnTypeParam = (substitution[t] as TypeParameterType).parameter;
+      fnTypeParams.add(fnTypeParam);
+      fnTypeArgs
+          .add(substitute(new TypeParameterType(t), enclosingTypeSubstitution));
+    }
+
+    function.typeParameters.insertAll(0, fnTypeParams);
     function.positionalParameters.insert(0, contextVariable);
     ++function.requiredParameterCount;
     Procedure closedTopLevelFunction = new Procedure(
@@ -356,21 +336,29 @@ class ClosureConverter extends Transformer {
         fileUri: currentFileUri);
     newLibraryMembers.add(closedTopLevelFunction);
 
+    // We need to again make new type parameters for the function's function
+    // type, and substitute them into the function type's arguments' types.
+    var closureTypeParams = <TypeParameter>[];
+    var closureTypeSubstitutionMap = copyTypeVariables(function.typeParameters);
+    for (DartType d in closureTypeSubstitutionMap.values)
+      closureTypeParams.add((d as TypeParameterType).parameter);
+
     FunctionType closureType = new FunctionType(
         function.positionalParameters
             .skip(1)
-            .map((VariableDeclaration decl) => decl.type)
-            .toList(),
-        function.returnType,
-        namedParameters: function.namedParameters
             .map((VariableDeclaration decl) =>
-                new NamedType(decl.name, decl.type))
+                substitute(decl.type, closureTypeSubstitutionMap))
             .toList(),
-        typeParameters: function.typeParameters,
+        substitute(function.returnType, closureTypeSubstitutionMap),
+        namedParameters: function.namedParameters
+            .map((VariableDeclaration decl) => new NamedType(
+                decl.name, substitute(decl.type, closureTypeSubstitutionMap)))
+            .toList(),
+        typeParameters: closureTypeParams,
         requiredParameterCount: function.requiredParameterCount - 1);
 
     return new ClosureCreation(
-        closedTopLevelFunction, accessContext, closureType);
+        closedTopLevelFunction, accessContext, closureType, fnTypeArgs);
   }
 
   TreeNode visitProcedure(Procedure node) {
@@ -664,14 +652,19 @@ class ClosureConverter extends Transformer {
 
   /// Creates copies of the type variables in [original] and returns a
   /// substitution that can be passed to [substitute] to substitute all uses of
-  /// [original] with their copies.
+  /// [original] with their copies. Additionally returns a list of new type
+  /// parameters to prefix to the enclosing function's type parameters and the
+  /// arguments to be passed for those parameters.
+  ///
   Map<TypeParameter, DartType> copyTypeVariables(
       Iterable<TypeParameter> original) {
     if (original.isEmpty) return const <TypeParameter, DartType>{};
+
     Map<TypeParameter, DartType> substitution = <TypeParameter, DartType>{};
     for (TypeParameter t in original) {
       substitution[t] = new TypeParameterType(new TypeParameter(t.name));
     }
+
     substitution.forEach((TypeParameter t, DartType copy) {
       if (copy is TypeParameterType) {
         copy.parameter.bound = substitute(t.bound, substitution);

pkg/kernel/lib/transformations/closure/info.dart

@@ -30,6 +30,7 @@ class ClosureInfo extends RecursiveVisitor {
 
   final Set<VariableDeclaration> variables = new Set<VariableDeclaration>();
 
+  /// Map from functions to set of type variables captured within them.
   final Map<FunctionNode, Set<TypeParameter>> typeVariables =
       <FunctionNode, Set<TypeParameter>>{};
 
@@ -166,7 +167,10 @@ class ClosureInfo extends RecursiveVisitor {
       // Propagate captured type variables to enclosing function.
       typeVariables
           .putIfAbsent(currentFunction, () => new Set<TypeParameter>())
-          .addAll(capturedTypeVariables);
+          .addAll(
+              // 't.parent == currentFunction' will be true if the type variable
+              // is defined by one of our type parameters.
+              capturedTypeVariables.where((t) => t.parent != currentFunction));
     }
   }
 
@@ -190,7 +194,7 @@ class ClosureInfo extends RecursiveVisitor {
   }
 
   visitTypeParameterType(TypeParameterType node) {
-    if (!isOuterMostContext) {
+    if (!isOuterMostContext && node.parameter.parent != currentFunction) {
       typeVariables
           .putIfAbsent(currentFunction, () => new Set<TypeParameter>())
           .add(node.parameter);

pkg/kernel/test/closures/closures.status

@@ -4,3 +4,18 @@
 
 field: RuntimeError
 type_variables: RuntimeError
+capture_closure: RuntimeError
+capture_closure_parameter: RuntimeError
+capture_this: RuntimeError
+catch: RuntimeError
+closure_in_constructor: RuntimeError
+closure_in_initializer: RuntimeError
+closure_in_initializer_closure: RuntimeError
+closures: RuntimeError
+for_in_closure: RuntimeError
+for_loop: RuntimeError
+for_variable_capture_test: RuntimeError
+instance_tear_off: RuntimeError
+named_closure: RuntimeError
+non_void_context: RuntimeError
+uncaptured_for_in_loop: RuntimeError

pkg/kernel/test/closures/suite.dart

@@ -59,8 +59,7 @@ class ClosureConversionContext extends ChainContext implements CompileContext {
         ];
 
   static Future<ClosureConversionContext> create(
-      Chain suite, Map<String, String> environment) async {
-    bool strongMode = environment.containsKey(STRONG_MODE);
+      Chain suite, Map<String, String> environment, bool strongMode) async {
     bool updateExpectations = environment["updateExpectations"] == "true";
     return new ClosureConversionContext(strongMode, updateExpectations);
   }
@@ -68,9 +67,10 @@ class ClosureConversionContext extends ChainContext implements CompileContext {
 
 Future<ClosureConversionContext> createContext(
     Chain suite, Map<String, String> environment) async {
+  bool strongMode = environment.containsKey(STRONG_MODE);
   environment["updateExpectations"] =
       const String.fromEnvironment("updateExpectations");
-  return ClosureConversionContext.create(suite, environment);
+  return ClosureConversionContext.create(suite, environment, strongMode);
 }
 
 class ClosureConversion

pkg/kernel/test/closures_type_vars/closures_type_vars.status

@@ -0,0 +1,5 @@
+# Copyright (c) 2017, 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.md file.
+
+type_variables: RuntimeError

pkg/kernel/test/closures_type_vars/suite.dart

@@ -0,0 +1,19 @@
+// Copyright (c) 2017, 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.md file.
+
+library test.kernel.closures_type_vars.suite;
+
+import 'dart:async' show Future;
+import 'package:testing/testing.dart' show Chain, runMe;
+import '../closures/suite.dart' show ClosureConversionContext;
+
+Future<ClosureConversionContext> createContext(
+    Chain suite, Map<String, String> environment) async {
+  environment["updateExpectations"] =
+      const String.fromEnvironment("updateExpectations");
+  return ClosureConversionContext.create(
+      suite, environment, true /*strongMode*/);
+}
+
+main(List<String> arguments) => runMe(arguments, createContext, "testing.json");

pkg/kernel/test/closures_type_vars/testing.json

@@ -0,0 +1,28 @@
+{
+"":"Copyright (c) 2016, 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.md file.",
+  "packages": "../../../../.packages",
+  "suites": [
+    {
+      "name": "closures_type_vars",
+      "kind": "Chain",
+      "source": "suite.dart",
+      "path": "../../testcases/closures_type_vars/",
+      "status": "closures_type_vars.status",
+      "pattern": [
+        "\\.dart$"
+      ],
+      "exclude": [
+        "/test/closures_type_vars/suite\\.dart$"
+      ]
+    }
+  ],
+  "analyze": {
+    "uris": [
+      "suite.dart"
+    ],
+    "exclude": [
+    ]
+  }
+}

pkg/kernel/testcases/closures/field.dart.expect

@@ -3,7 +3,7 @@ import self as self;
 import "dart:core" as core;
 
 class C<T extends core::Object> extends core::Object {
-  field dynamic v = MakeClosure<(dynamic) → dynamic>(self::closure#C#v#function, null);
+  field dynamic v = MakeClosure<<T extends core::Object>(dynamic) → dynamic, self::C::T>(self::closure#C#v#function, null);
   final field dynamic y = MakeClosure<() → dynamic>(self::closure#C#y#function, null);
   static final field dynamic z = MakeClosure<() → dynamic>(self::closure#C#z#function, null);
   default constructor •() → void
@@ -31,8 +31,8 @@ static method main() → dynamic {
   if(!"z".==(self::C::z.call()))
     throw "z";
 }
-static method closure#C#v#function(Vector #contextParameter, dynamic x) → dynamic {
-  return x is dynamic;
+static method closure#C#v#function<T extends core::Object>(Vector #contextParameter, dynamic x) → dynamic {
+  return x is self::closure#C#v#function::T;
 }
 static method closure#C#y#function(Vector #contextParameter) → dynamic {
   return "y";

pkg/kernel/testcases/closures/instance_tear_off.dart.expect

@@ -50,7 +50,7 @@ class E<T extends core::Object> extends core::Object {
     : super core::Object::•()
     ;
   get g() → dynamic {
-    return MakeClosure<(dynamic) → dynamic>(self::closure#E#g#function, null);
+    return MakeClosure<<T extends core::Object>(T) → dynamic, self::E::T>(self::closure#E#g#function, null);
   }
   method a() → dynamic {
     return "a";
@@ -107,7 +107,7 @@ static method closure#D#g#function(Vector #contextParameter, dynamic x) → dyna
 static method closure#D#f#function(Vector #contextParameter) → dynamic {
   return "f";
 }
-static method closure#E#g#function(Vector #contextParameter, dynamic x) → dynamic {
+static method closure#E#g#function<T extends core::Object>(Vector #contextParameter, self::closure#E#g#function::T x) → dynamic {
   return "g(${x})";
 }
 static method closure#E#f#function(Vector #contextParameter) → dynamic {

pkg/kernel/testcases/closures/type_variables.dart.expect

@@ -1,50 +0,0 @@
-library;
-import self as self;
-import "dart:core" as core;
-
-class C<T extends core::Object, S extends core::Object> extends core::Object {
-  constructor internal() → void
-    : super core::Object::•()
-    ;
-  method foo(self::C::S s) → dynamic {
-    final Vector #context = MakeVector(2);
-    #context[1] = this;
-    return MakeClosure<(dynamic) → dynamic>(self::closure#C#foo#function, #context);
-  }
-  method bar() → dynamic {
-    self::C<self::C::T, self::C::S> self = this;
-  }
-  method baz() → dynamic {
-    return MakeClosure<() → dynamic>(self::closure#C#baz#function, null);
-  }
-  static factory •<T extends core::Object, S extends core::Object>() → self::C<self::C::•::T, self::C::•::S> {
-    final () → dynamic local = MakeClosure<() → dynamic>(self::closure#C#function#local, null);
-    return local.call();
-  }
-}
-static method main(dynamic arguments) → dynamic {
-  core::print(self::C::•<core::String, core::String>().foo(null).call(arguments.first));
-  dynamic c = self::C::•<core::int, core::int>().baz().call().call();
-  if(!(c is self::C<core::int, core::int>))
-    throw "${c} fails type test 'is C<int, int>'";
-  if(c is self::C<core::String, core::String>) {
-    throw "${c{self::C<core::String, core::String>}} passes type test 'is C<String, String>'";
-  }
-  core::print(c);
-}
-static method closure#C#foo#function(Vector #contextParameter, dynamic x) → dynamic {
-  dynamic y = x;
-  core::Object z = y;
-  self::C<dynamic, dynamic> self = #contextParameter[1];
-  return z as dynamic;
-}
-static method closure#C#baz#function#function(Vector #contextParameter) → dynamic {
-  return self::C::•<dynamic, dynamic>();
-}
-static method closure#C#baz#function(Vector #contextParameter) → dynamic {
-  return MakeClosure<() → dynamic>(self::closure#C#baz#function#function, #contextParameter);
-}
-static method closure#C#function#local(Vector #contextParameter) → dynamic {
-  self::C<dynamic, dynamic> self = new self::C::internal<dynamic, dynamic>();
-  return self;
-}

pkg/kernel/testcases/closures_type_vars/type_variables.dart

@@ -29,6 +29,15 @@ class C<T, S> {
   C.internal();
 }
 
+fn<A>(A x) {
+  var fn2 = (A x2) {
+    var l = <A>[];
+    l.add(x2);
+    return l;
+  };
+  return fn2(x);
+}
+
 main(arguments) {
   print(new C<String, String>().foo(null)(arguments.first));
   dynamic c = new C<int, int>().baz()()();
@@ -37,4 +46,5 @@ main(arguments) {
     throw "$c passes type test 'is C<String, String>'";
   }
   print(c);
+  print(fn<int>(3));
 }

pkg/kernel/testcases/closures_type_vars/type_variables.dart.expect

@@ -0,0 +1,60 @@
+library;
+import self as self;
+import "dart:core" as core;
+
+class C<T extends core::Object, S extends core::Object> extends core::Object {
+  constructor internal() → void
+    : super core::Object::•()
+    ;
+  method foo(self::C::S s) → dynamic {
+    final Vector #context = MakeVector(2);
+    #context[1] = this;
+    return MakeClosure<<T extends core::Object, S extends core::Object>(T) → T, self::C::T, self::C::S>(self::closure#C#foo#function, #context);
+  }
+  method bar() → dynamic {
+    self::C<self::C::T, self::C::S> self = this;
+  }
+  method baz() → dynamic {
+    return MakeClosure<<T extends core::Object, S extends core::Object>() → () → self::C<T, S>, self::C::T, self::C::S>(self::closure#C#baz#function, null);
+  }
+  static factory •<T extends core::Object, S extends core::Object>() → self::C<self::C::•::T, self::C::•::S> {
+    final <T extends core::Object, S extends core::Object>() → self::C<self::C::•::T, self::C::•::S> local = MakeClosure<<T extends core::Object, S extends core::Object>() → self::C<T, S>, self::C::•::T, self::C::•::S>(self::closure#C#function#local, null);
+    return local.call();
+  }
+}
+static method fn<A extends core::Object>(self::fn::A x) → dynamic {
+  <A extends core::Object>(self::fn::A) → core::List<self::fn::A> fn2 = MakeClosure<<A extends core::Object>(A) → core::List<A>, self::fn::A>(self::closure#fn#function, null);
+  return fn2.call(x);
+}
+static method main(dynamic arguments) → dynamic {
+  core::print(self::C::•<core::String, core::String>().{self::C::foo}(null).call(arguments.first));
+  dynamic c = self::C::•<core::int, core::int>().{self::C::baz}().call().call();
+  if(!(c is self::C<core::int, core::int>))
+    throw "${c} fails type test 'is C<int, int>'";
+  if(c is self::C<core::String, core::String>) {
+    throw "${c{self::C<core::String, core::String>}} passes type test 'is C<String, String>'";
+  }
+  core::print(c);
+  core::print(self::fn<core::int>(3));
+}
+static method closure#C#foo#function<T extends core::Object, S extends core::Object>(Vector #contextParameter, self::closure#C#foo#function::T x) → self::closure#C#foo#function::T {
+  self::closure#C#foo#function::T y = x;
+  core::Object z = y;
+  self::C<self::closure#C#foo#function::T, self::closure#C#foo#function::S> self = #contextParameter[1];
+  return z as self::closure#C#foo#function::T;
+}
+static method closure#C#baz#function#function<T extends core::Object, S extends core::Object>(Vector #contextParameter) → self::C<self::closure#C#baz#function#function::T, self::closure#C#baz#function#function::S> {
+  return self::C::•<self::closure#C#baz#function#function::T, self::closure#C#baz#function#function::S>();
+}
+static method closure#C#baz#function<T extends core::Object, S extends core::Object>(Vector #contextParameter) → () → self::C<self::closure#C#baz#function::T, self::closure#C#baz#function::S> {
+  return MakeClosure<<T extends core::Object, S extends core::Object>() → self::C<T, S>, self::closure#C#baz#function::T, self::closure#C#baz#function::S>(self::closure#C#baz#function#function, #contextParameter);
+}
+static method closure#C#function#local<T extends core::Object, S extends core::Object>(Vector #contextParameter) → self::C<self::closure#C#function#local::T, self::closure#C#function#local::S> {
+  self::C<self::closure#C#function#local::T, self::closure#C#function#local::S> self = new self::C::internal<self::closure#C#function#local::T, self::closure#C#function#local::S>();
+  return self;
+}
+static method closure#fn#function<A extends core::Object>(Vector #contextParameter, self::closure#fn#function::A x2) → core::List<self::closure#fn#function::A> {
+  core::List<self::closure#fn#function::A> l = <self::closure#fn#function::A>[];
+  l.{core::List::add}(x2);
+  return l;
+}

runtime/vm/object.cc

@@ -6879,7 +6879,7 @@ void Function::DropUncompiledImplicitClosureFunction() const {
 // Converted closure functions are used in VM Closure instances that represent
 // the results of evaluation of [MakeClosure] primitive operations.
 //
-// Internally, converted closure functins are represented with the same Closure
+// Internally, converted closure functions are represented with the same Closure
 // class as implicit closure functions (that are used for dealing with
 // tear-offs).  The Closure class instances have two fields, one for the
 // function, and one for the captured context.  Implicit closure functions have
@@ -6903,7 +6903,7 @@ void Function::DropUncompiledImplicitClosureFunction() const {
 // this way, is invoked, it should receive the [Vector] as the first argument,
 // and take the rest of the arguments from the invocation.
 //
-// Converted cosure functions in VM follow same discipline as implicit closure
+// Converted closure functions in VM follow same discipline as implicit closure
 // functions, because they are similar in many ways. For further deatils, please
 // refer to the following methods:
 //   -> Function::ConvertedClosureFunction