Fix type variables in closures.

sdk/lib/_internal/compiler/implementation/ssa/builder.dart

Index: sdk/lib/_internal/compiler/implementation/ssa/builder.dart
diff --git a/sdk/lib/_internal/compiler/implementation/ssa/builder.dart b/sdk/lib/_internal/compiler/implementation/ssa/builder.dart
index 75d644355eece6246ba3cb338c882e5e44d4f904..fdc081116cddb1d58f52fa577942932eaa5bf666 100644
--- a/sdk/lib/_internal/compiler/implementation/ssa/builder.dart
+++ b/sdk/lib/_internal/compiler/implementation/ssa/builder.dart
@@ -3246,13 +3246,23 @@ class SsaBuilder extends ResolvedVisitor implements Visitor {
       member = closureClass.methodElement;
       member = member.getOutermostEnclosingMemberOrTopLevel();
     }
-    if (isClosure && member.isFactoryConstructor()) {
-      // The type variable is used from a closure in a factory constructor.  The
-      // value of the type argument is stored as a local on the closure itself.
-      return localsHandler.readLocal(type.element);
-    } else if (member.isConstructor() ||
-               member.isGenerativeConstructorBody() ||
-               member.isField()) {
+    bool isConstructor =

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

How about calling this variable isInConstructorContext and add member.isField to
it?

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

Done.

+        member.isConstructor() || member.isGenerativeConstructorBody();
+    if (isClosure) {
+      if (member.isFactoryConstructor()) {
+        // The type variable is used from a closure in a factory constructor.
+        // The value of the type argument is stored as a local on the closure
+        // itself.
+        return localsHandler.readLocal(type.element);
+      } else if (!member.isField()) {

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

Maybe use the positive form instead and shuffle the code below.

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

I am not sure it will be more readable.

[ngeoffray, 2013/06/19 20:03:57]

What I don't like about negative forms is that everything else goes. In this
example everything else covers the !member.isField, but it's not explicit which
elements you actually cover.

But maybe that's just me, feel free to keep it as is.

[karlklose, 2013/06/20 10:17:59]

I changed it to use the positive form and added an assert.

+        // The type variable is stored on the "enclosing object" and needs to be
+        // accessed using the this-reference in the closure.
+        return readTypeVariable(member.getEnclosingClass(), type.element);
+      } else {
+        // The type variable is stored in a parameter of the method.
+        return localsHandler.readLocal(type.element);
+      }
+    } else if (isConstructor || member.isField()) {
       // The type variable is stored in a parameter of the method.
       return localsHandler.readLocal(type.element);
     } else if (member.isInstanceMember()) {