fixes #130, accept anything with iterator in for loops

[email protected]

Review URL: https://codereview.chromium.org/1126713002

Author: John Messerly

pkg/dev_compiler/lib/runtime/dart/collection.js

@@ -109,6 +109,9 @@ var collection;
   let _newSet = Symbol('_newSet');
   let SetMixin$ = dart.generic(function(E) {
     class SetMixin extends core.Object {
+      [Symbol.iterator]() {
+        return new dart.JsIterator(this[core.$iterator]);
+      }
       get [core.$isEmpty]() {
         return this[core.$length] == 0;
       }
@@ -454,6 +457,9 @@ var collection;
           result.add(e);
         return result;
       }
+      [Symbol.iterator]() {
+        return new dart.JsIterator(this[core.$iterator]);
+      }
     }
     HashSet[dart.implements] = () => [core.Set$(E)];
     dart.defineNamedConstructor(HashSet, 'identity');
@@ -677,6 +683,9 @@ var collection;
       toString() {
         return IterableBase.iterableToShortString(this, '(', ')');
       }
+      [Symbol.iterator]() {
+        return new dart.JsIterator(this[core.$iterator]);
+      }
     }
     IterableMixin[dart.implements] = () => [core.Iterable$(E)];
     return IterableMixin;
@@ -1020,6 +1029,9 @@ var collection;
         parts[core.$add](penultimateString);
         parts[core.$add](ultimateString);
       }
+      [Symbol.iterator]() {
+        return new dart.JsIterator(this[core.$iterator]);
+      }
     }
     IterableBase[dart.implements] = () => [core.Iterable$(E)];
     dart.defineLazyProperties(IterableBase, {
@@ -1185,6 +1197,9 @@ var collection;
         }
         return result;
       }
+      [Symbol.iterator]() {
+        return new dart.JsIterator(this[core.$iterator]);
+      }
     }
     LinkedHashSet[dart.implements] = () => [HashSet$(E)];
     dart.defineNamedConstructor(LinkedHashSet, 'identity');
@@ -1396,6 +1411,9 @@ var collection;
       get [core.$iterator]() {
         return new (_internal.ListIterator$(E))(this);
       }
+      [Symbol.iterator]() {
+        return new dart.JsIterator(this[core.$iterator]);
+      }
       [core.$elementAt](index) {
         return this[core.$get](index);
       }
@@ -2212,6 +2230,9 @@ var collection;
       from(elements) {
         return new ListQueue$(E).from(elements);
       }
+      [Symbol.iterator]() {
+        return new dart.JsIterator(this[core.$iterator]);
+      }
     }
     Queue[dart.implements] = () => [core.Iterable$(E), _internal.EfficientLength];
     dart.defineNamedConstructor(Queue, 'from');

pkg/dev_compiler/lib/runtime/dart/core.js

@@ -1085,6 +1085,7 @@ var core;
       return dart.notNull(this.isGetter) || dart.notNull(this.isSetter);
     }
   }
+  let $iterator = dart.JsSymbol('$iterator');
   let $join = dart.JsSymbol('$join');
   let Iterable$ = dart.generic(function(E) {
     class Iterable extends Object {
@@ -1097,22 +1098,16 @@ var core;
           return new (_internal.EmptyIterable$(E))();
         return new (exports._GeneratorIterable$(E))(count, generator);
       }
+      [dart.JsSymbol.iterator]() {
+        return new dart.JsIterator(this[$iterator]);
+      }
       [$join](separator) {
         if (separator === void 0)
           separator = "";
         let buffer = new StringBuffer();
         buffer.writeAll(this, separator);
         return dart.toString(buffer);
       }
-      [dart.JsSymbol.iterator]() {
-        var iterator = this.iterator;
-        return {
-          next() {
-            var done = iterator.moveNext();
-            return {done: done, current: done ? void 0 : iterator.current};
-          }
-        };
-      }
     }
     dart.defineNamedConstructor(Iterable, 'generate');
     return Iterable;
@@ -1126,7 +1121,6 @@ var core;
   let _end = dart.JsSymbol('_end');
   let _start = dart.JsSymbol('_start');
   let _generator = dart.JsSymbol('_generator');
-  let $iterator = dart.JsSymbol('$iterator');
   let $skip = dart.JsSymbol('$skip');
   let $take = dart.JsSymbol('$take');
   let _GeneratorIterable$ = dart.generic(function(E) {
@@ -3412,10 +3406,10 @@ var core;
   exports.identityHashCode = identityHashCode;
   exports.int = int;
   exports.Invocation = Invocation;
+  exports.$iterator = $iterator;
   exports.$join = $join;
   exports.Iterable$ = Iterable$;
   exports.Iterable = Iterable;
-  exports.$iterator = $iterator;
   exports.$skip = $skip;
   exports.$take = $take;
   exports.BidirectionalIterator$ = BidirectionalIterator$;

pkg/dev_compiler/lib/runtime/dart_runtime.js

@@ -945,6 +945,18 @@ var dart, _js_helper, _js_primitives;
   }
   dart.noSuchMethod = noSuchMethod;
 
+  class JsIterator {
+    constructor(dartIterator) {
+      this.dartIterator = dartIterator;
+    }
+    next() {
+      let i = this.dartIterator;
+      var done = !i.moveNext();
+      return { done: done, value: done ? void 0 : i.current };
+    }
+  }
+  dart.JsIterator = JsIterator;
+
   // TODO(jmesserly): right now this is a sentinel. It should be a type object
   // of some sort, assuming we keep around `dynamic` at runtime.
   dart.dynamic = { toString() { return 'dynamic'; } };

pkg/dev_compiler/lib/src/codegen/js_codegen.dart

@@ -573,32 +573,64 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor {
       jsMethods.add(_emitImplicitConstructor(node, name, fields));
     }
 
-    for (var member in node.members) {
-      if (member is ConstructorDeclaration) {
-        jsMethods.add(_emitConstructor(member, name, fields, isObject));
-      } else if (member is MethodDeclaration) {
-        jsMethods.add(_emitMethodDeclaration(type, member));
+    bool hasJsPeer = getAnnotationValue(node, _isJsPeerInterface) != null;
+
+    bool hasIterator = false;
+    for (var m in node.members) {
+      if (m is ConstructorDeclaration) {
+        jsMethods.add(_emitConstructor(m, name, fields, isObject));
+      } else if (m is MethodDeclaration) {
+        jsMethods.add(_emitMethodDeclaration(type, m));
+
+        if (!hasJsPeer && m.isGetter && m.name.name == 'iterator') {
+          hasIterator = true;
+          jsMethods.add(_emitIterable(type));
+        }
       }
     }
 
-    // Support for adapting dart:core Iterator/Iterable to ES6 versions.
-    // This lets them use for-of loops transparently.
-    // https://github.com/lukehoban/es6features#iterators--forof
-    if (element.library.isDartCore && element.name == 'Iterable') {
-      JS.Fun body = js.call('''function() {
-        var iterator = this.iterator;
-        return {
-          next() {
-            var done = iterator.moveNext();
-            return { done: done, current: done ? void 0 : iterator.current };
-          }
-        };
-      }''');
-      jsMethods.add(new JS.Method(js.call('$_SYMBOL.iterator'), body));
+    // If the type doesn't have an `iterator`, but claims to implement Iterable,
+    // we inject the adaptor method here, as it's less code size to put the
+    // helper on a parent class. This pattern is common in the core libraries
+    // (e.g. IterableMixin<E> and IterableBase<E>).
+    //
+    // (We could do this same optimization for any interface with an `iterator`
+    // method, but that's more expensive to check for, so it doesn't seem worth
+    // it. The above case for an explicit `iterator` method will catch those.)
+    if (!hasJsPeer && !hasIterator && _implementsIterable(type)) {
+      jsMethods.add(_emitIterable(type));
     }
+
     return jsMethods.where((m) => m != null).toList(growable: false);
   }
 
+  bool _implementsIterable(InterfaceType t) =>
+      t.interfaces.any((i) => i.element.type == types.iterableType);
+
+  /// Support for adapting dart:core Iterable to ES6 versions.
+  ///
+  /// This lets them use for-of loops transparently:
+  /// <https://github.com/lukehoban/es6features#iterators--forof>
+  ///
+  /// This will return `null` if the adapter was already added on a super type,
+  /// otherwise it returns the adapter code.
+  // TODO(jmesserly): should we adapt `Iterator` too?
+  JS.Method _emitIterable(InterfaceType t) {
+    // If a parent had an `iterator` (concrete or abstract) or implements
+    // Iterable, we know the adapter is already there, so we can skip it as a
+    // simple code size optimization.
+    var parent = t.lookUpGetterInSuperclass('iterator', t.element.library);
+    if (parent != null) return null;
+    parent = findSupertype(t, _implementsIterable);
+    if (parent != null) return null;
+
+    // Otherwise, emit the adapter method, which wraps the Dart iterator in
+    // an ES6 iterator.
+    return new JS.Method(js.call('$_SYMBOL.iterator'), js.call(
+        'function() { return new dart.JsIterator(this.#); }',
+        [_emitMemberName('iterator', type: t)]));
+  }
+
   /// Emit class members that need to come after the class declaration, such
   /// as static fields. See [_emitClassMethods] for things that are emitted
   /// inside the ES6 `class { ... }` node.

pkg/dev_compiler/lib/src/utils.dart

@@ -424,3 +424,16 @@ Map<String, DartType> getObjectMemberMap(TypeProvider typeProvider) {
   }
   return map;
 }
+
+/// Searches all supertype, in order of most derived members, to see if any
+/// [match] a condition. If so, returns the first match, otherwise returns null.
+InterfaceType findSupertype(InterfaceType type, bool match(InterfaceType t)) {
+  for (var m in type.mixins.reversed) {
+    if (match(m)) return m;
+  }
+  var s = type.superclass;
+  if (s == null) return null;
+
+  if (match(s)) return type;
+  return findSupertype(s, match);
+}