‘Yield* e' should not access e.current eagerly

runtime/lib/core_patch.dart

Index: runtime/lib/core_patch.dart
diff --git a/runtime/lib/core_patch.dart b/runtime/lib/core_patch.dart
index 224d532c59181cf04341ec7674cd84a3fd158493..11e307af2fe9782aaf8f9f5063f5a55211f91fce 100644
--- a/runtime/lib/core_patch.dart
+++ b/runtime/lib/core_patch.dart
@@ -40,9 +40,13 @@ class _SyncIterable extends IterableBase {
 class _SyncIterator implements Iterator {
   bool isYieldEach;  // Set by generated code for the yield* statement.
   Iterator yieldEachIterator;
-  var current;  // Set by generated code for the yield and yield* statement.
+  var _current;  // Set by generated code for the yield and yield* statement.
   SyncGeneratorCallback moveNextFn;
 
+  get current => yieldEachIterator != null
+      ? yieldEachIterator.current
+      : _current;
+
   _SyncIterator(this.moveNextFn);
 
   bool moveNext() {
@@ -52,7 +56,6 @@ class _SyncIterator implements Iterator {
     while(true) {
       if (yieldEachIterator != null) {
         if (yieldEachIterator.moveNext()) {
-          current = yieldEachIterator.current;
           return true;
         }
         yieldEachIterator = null;
@@ -60,13 +63,14 @@ class _SyncIterator implements Iterator {
       isYieldEach = false;
       if (!moveNextFn(this)) {
         moveNextFn = null;
-        current = null;
+        _current = null;
         return false;
       }
       if (isYieldEach) {

[Ivan Posva, 2016/02/13 07:03:03]

Can you please explain in a comment how this code here is ever reached? Unless
of course the moveNextFn updates the isYieldEach value of this iterator when it
is being called, but such a side-effect should definitely be pointed out.

Thanks!

[hausner, 2016/02/13 07:13:06]

Yes, the generator sets this value when it reaches a yield* statement. See
comment in line 41.

[Ivan Posva, 2016/02/13 07:27:09]

I understood the above comment as the field being set once on construction,
which apparently is not the case.

How about a comment like this before line 64:
// The moveNextFn will update the values of the isYieldEach and _current fields.

Note on the side: Why are we publicly exposing the internal fields of this
implementation class?

         // Spec mandates: it is a dynamic error if the class of [the object
         // returned by yield*] does not implement Iterable.
-        yieldEachIterator = (current as Iterable).iterator;
+        yieldEachIterator = (_current as Iterable).iterator;
+        _current = null;
         continue;
       }
       return true;