Properly type infer for-in loops when the iterator type is a type parameter.

pkg/front_end/lib/src/fasta/kernel/kernel_shadow_ast.dart

 // 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 file.
 
 /// This file declares a "shadow hierarchy" of concrete classes which extend
 /// the kernel class hierarchy, adding methods and fields needed by the
 /// BodyBuilder.
 ///
 /// Instances of these classes may be created using the factory methods in
 /// `ast_factory.dart`.
@@ skipping 669 matching lines @@
       }
     } else {
       // TODO(paulberry): In this case, should the context be based on the
       // declared type of the loop variable?
       // TODO(paulberry): Note that when [_declaresVariable] is `false`, the
       // body starts with an assignment from the synthetic loop variable to
       // another variable.  We need to make sure any type inference diagnostics
       // that occur related to this assignment are reported at the correct
       // locations.
     }
-    var inferredExpressionType =
-        inferrer.inferExpression(iterable, context, typeNeeded);
+    var inferredExpressionType = inferrer.resolveTypeParameter(
+        inferrer.inferExpression(iterable, context, typeNeeded));
     if (typeNeeded) {
       var inferredType = const DynamicType();
       if (inferredExpressionType is InterfaceType) {
         InterfaceType supertype = inferrer.classHierarchy
             .getTypeAsInstanceOf(inferredExpressionType, iterableClass);
         if (supertype != null) {
           inferredType = supertype.typeArguments[0];
         }
       }
       inferrer.instrumentation?.record(
@@ skipping 1325 matching lines @@
       // so that the type hierarchy will be simpler (which may speed up "is"
       // checks).
       return statement._inferStatement(this);
     } else {
       // Encountered a statement type for which type inference is not yet
       // implemented, so just skip it for now.
       // TODO(paulberry): once the BodyBuilder uses shadow classes for
       // everything, this case should no longer be needed.
     }
   }
+
+  /// If the given [type] is a [TypeParameterType], resolve it to its bound.
+  DartType resolveTypeParameter(DartType type) {
+    DartType resolveOneStep(DartType type) {
+      if (type is TypeParameterType) {
+        return type.bound ?? type.parameter.bound;
+      } else {
+        return null;
+      }
+    }
+
+    var resolved = resolveOneStep(type);
+    if (resolved == null) return type;
+
+    // Detect circularities using the tortoise-and-hare algorithm.
+    type = resolved;
+    DartType hare = resolveOneStep(type);
+    if (hare == null) return type;
+    while (true) {
+      if (identical(type, hare)) {
+        // We found a circularity.  Give up and return `dynamic`.
+        return const DynamicType();
+      }
+      resolved = resolveOneStep(hare);

[Siggi Cherem (dart-lang), 2017/06/21 21:12:10]

random optional idea here - it took me some time to see that this is doing a
2-step and 1-step walk side by side (I wasn't as familiar with the algorithm), I
wonder if it helps to rename "resolved" here, maybe add a couple locals that
don't escape the loop body.

For example:

while (true) {
  if (identical(...)) stop;

  // hare takes 2 steps
  var step1 = resolveOneStep(hare);
  if (step1 == null) return hare;
  var step2 = resolveOneStep(step1);
  if (step2 == null) return step1;
  hare = step2;

  // toirtoise takes one step:
  type = resolveOneStep(type);

[Paul Berry, 2017/06/21 21:44:16]

I like it!  Thanks

+      if (resolved == null) return hare;
+      hare = resolved;
+      resolved = resolveOneStep(hare);
+      if (resolved == null) return hare;
+      hare = resolved;
+      type = resolveOneStep(type);
+    }
+  }
 }
 
 /// Shadow object for [TypeLiteral].
 class KernelTypeLiteral extends TypeLiteral implements KernelExpression {
   KernelTypeLiteral(DartType type) : super(type);
 
   @override
   void _collectDependencies(KernelDependencyCollector collector) {
     // No inference dependencies.
   }
@@ skipping 131 matching lines @@
   KernelVariableDeclaration.forValue(
       Expression initializer, this._functionNestingLevel)
       : _implicitlyTyped = true,
         _isLocalFunction = false,
         super.forValue(initializer);
 
   @override
   void _inferStatement(KernelTypeInferrer inferrer) {
     inferrer.listener.variableDeclarationEnter(this);
     var declaredType = _implicitlyTyped ? null : type;
+    DartType inferredType;
     if (initializer != null) {
-      var inferredType = inferrer.inferDeclarationType(inferrer.inferExpression(
+      inferredType = inferrer.inferDeclarationType(inferrer.inferExpression(
           initializer, declaredType, _implicitlyTyped));
-      if (inferrer.strongMode && _implicitlyTyped) {
-        inferrer.instrumentation?.record(Uri.parse(inferrer.uri), fileOffset,
-            'type', new InstrumentationValueForType(inferredType));
-        type = inferredType;
-      }
+    } else {
+      inferredType = const DynamicType();
+    }
+    if (inferrer.strongMode && _implicitlyTyped) {
+      inferrer.instrumentation?.record(Uri.parse(inferrer.uri), fileOffset,
+          'type', new InstrumentationValueForType(inferredType));
+      type = inferredType;
     }
     inferrer.listener.variableDeclarationExit(this);
   }
 
   /// Determine whether the given [KernelVariableDeclaration] had an implicit
   /// type.
   ///
   /// This is static to avoid introducing a method that would be visible to
   /// the kernel.
   static bool isImplicitlyTyped(KernelVariableDeclaration variable) =>
@@ skipping 74 matching lines @@
   }
 
   transformChildren(v) {
     return internalError("Internal error: Unsupported operation.");
   }
 
   visitChildren(v) {
     return internalError("Internal error: Unsupported operation.");
   }
 }