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

runtime/lib/core_patch.dart

 // Copyright (c) 2013, 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.
 
 import "dart:math";
 import "dart:typed_data";
 
 // Equivalent of calling FATAL from C++ code.
 _fatal(msg) native "DartCore_fatal";
 
@@ skipping 22 matching lines @@
   const _SyncIterable(this.moveNextFn);
 
   get iterator {
     return new _SyncIterator(moveNextFn._clone());
   }
 }
 
 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() {
     if (moveNextFn == null) {
       return false;
     }
     while(true) {
       if (yieldEachIterator != null) {
         if (yieldEachIterator.moveNext()) {
-          current = yieldEachIterator.current;
           return true;
         }
         yieldEachIterator = null;
       }
       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;
     }
   }
 }
 
 patch class StackTrace {
   /* patch */ static StackTrace get current native "StackTrace_current";
 }