Fix type variables in closures.

sdk/lib/_internal/compiler/implementation/closure.dart

Index: sdk/lib/_internal/compiler/implementation/closure.dart
diff --git a/sdk/lib/_internal/compiler/implementation/closure.dart b/sdk/lib/_internal/compiler/implementation/closure.dart
index 35b227e9862e3c9d690735ce189511ff116007de..2bae512c17bc8ad42d1362dcfcf32e57276ac889 100644
--- a/sdk/lib/_internal/compiler/implementation/closure.dart
+++ b/sdk/lib/_internal/compiler/implementation/closure.dart
@@ -469,22 +469,22 @@ class ClosureTranslator extends Visitor {
     // TODO(karlklose,johnniwinther): if the type is null, the annotation is
     // from a parameter which has been analyzed before the method has been
     // resolved and the result has been thrown away.
-    if (compiler.enableTypeAssertions && type != null &&
-        type.containsTypeVariables) {
-      if (insideClosure && member.isFactoryConstructor()) {
-        // This is a closure in a factory constructor.  Since there is no
-        // [:this:], we have to mark the type arguments as free variables to
-        // capture them in the closure.
-        type.forEachTypeVariable((variable) => useLocal(variable.element));
-      }
+    if (type != null && type.containsTypeVariables) {
       // TODO(karlklose): try to get rid of the isField check; there is a bug
       // with type variable use in field initializer (in both modes).

[ngeoffray, 2013/06/18 11:14:51]

use -> used

Could you file a bug? Do you have a failing test case?

[karlklose, 2013/06/19 12:17:27]

In fact it is not really a bug, we have to special case field initializers
because of the way they are generated.  This comment is obsolete because I
reverted the change, but I changed the comment at the place I copied this from.

-      if (member.isInstanceMember() && !member.isField()) {
+      if (!member.isField()) {
         // In checked mode, using a type variable in a type annotation may lead
         // to a runtime type check that needs to access the type argument and
         // therefore the closure needs a this-element.
         registerNeedsThis();

[ngeoffray, 2013/06/18 11:14:51]

Thinking more about it, it looks my previous comment was wrong: how do you
differentiate between an is check and a type annotation here? A type annotation
only needs to register this in checked mode. It looks like you're
unconditionally registering it here.

[karlklose, 2013/06/19 12:17:27]

Reverted it back.

       }
+      if (compiler.enableTypeAssertions && insideClosure &&
+          member.isFactoryConstructor()) {
+        // This is a closure in a factory constructor.  Since there is no
+        // [:this:], we have to mark the type arguments as free variables to
+        // capture them in the closure.
+        type.forEachTypeVariable((variable) => useLocal(variable.element));
+      }
     }
   }
 
@@ -542,23 +542,14 @@ class ClosureTranslator extends Visitor {
   visitNewExpression(NewExpression node) {
     DartType type = elements.getType(node);
 
-    bool hasTypeVariable(DartType type) {
-      if (type is TypeVariableType) {
-        return true;
-      } else if (type is InterfaceType) {
-        InterfaceType ifcType = type;
-        for (DartType argument in ifcType.typeArguments) {
-          if (hasTypeVariable(argument)) {
-            return true;
-          }
-        }
-      }
-      return false;
-    }
-
     void analyzeTypeVariables(DartType type) {
       if (type is TypeVariableType) {
         useLocal(type.element);
+        // Field initializers are inlined and access the type variable as
+        // normal parameters.
+        if (!outermostElement.isField()) {
+          registerNeedsThis();
+        }
       } else if (type is InterfaceType) {
         InterfaceType ifcType = type;
         for (DartType argument in ifcType.typeArguments) {
@@ -572,7 +563,7 @@ class ClosureTranslator extends Visitor {
       if (outermostElement.isConstructor() || outermostElement.isField()) {
         analyzeTypeVariables(type);
       } else if (outermostElement.isInstanceMember()) {
-        if (hasTypeVariable(type)) {
+        if (type.containsTypeVariables) {
           registerNeedsThis();
         }
       }