for loop variable capture is inconsistent between 'for in' and 'for' #333

Closed
DartBot opened this Issue Nov 4, 2011 · 9 comments

Comments

Projects
None yet
4 participants
@DartBot

DartBot commented Nov 4, 2011

This issue was originally filed by mattsh@google.com


I notice that the loop variable in a 'for' loop behaves differently than the loop variable in a 'for in' loop, when the loop variable is captured by a closure. I think the 'for in' behavior is what we want, and we should change the 'for' loop behavior to do what 'for in' does.

For example, here is output from the program below that demonstrates this. Here 'a' is the loop variable, and 'b' is a local block variable. Notice 'b' gets captured in both cases, but 'a' is only captured with 'for in' loops.

start
using normal for loop:
a is 5, b is 0
a is 5, b is 1
a is 5, b is 2
a is 5, b is 3
a is 5, b is 4
using for in loop:
a is 0, b is 0
a is 1, b is 1
a is 2, b is 2
a is 3, b is 3
a is 4, b is 4
done

From this program:

class Capture {

  static void main() {
    print("start");
    doForLoop();
    doForIn();
    print ("done");
  }

  static void doForLoop() {
    print("using normal for loop:");
    List closures = new List();
    for (int a = 0; a < 5; a++) {
      int b = a;
      closures.add( (){print("a is ${a}, b is ${b}");} );
    }
    for (var c in closures) {
      c();
    }
  }

  static void doForIn() {

    print("using for in loop:");
    List numbers = new List();
    for (int i = 0; i < 5; i++) {
      numbers.add(i);
    }
    List closures = new List();
    for (int a in numbers) {
      int b = a;
      closures.add( (){print("a is ${a}, b is ${b}");} );
    }
    for (var c in closures) {
      c();
    }
  }
}

void main() {
  Capture.main();
}

@kasperl

This comment has been minimized.

Show comment Hide comment
@kasperl

kasperl Nov 4, 2011

Contributor

This is already covered by section 11.5.1 ("For Loop") in the specification. It just needs to be implemented in the VM and the compilers.

Contributor

kasperl commented Nov 4, 2011

This is already covered by section 11.5.1 ("For Loop") in the specification. It just needs to be implemented in the VM and the compilers.

@DartBot

This comment has been minimized.

Show comment Hide comment
@DartBot

DartBot Nov 4, 2011

This comment was originally written by drfibonacci@google.com


Removed Type-Defect label.
Added Type-Enhancement, Area-VM, Triaged labels.

DartBot commented Nov 4, 2011

This comment was originally written by drfibonacci@google.com


Removed Type-Defect label.
Added Type-Enhancement, Area-VM, Triaged labels.

@DartBot

This comment has been minimized.

Show comment Hide comment
@DartBot

DartBot Nov 16, 2011

This comment was originally written by mattsh@google.com


I'm changing the area to frog, just to make sure frog side sees this, and there are some closely related bugs in frog that might make fixing this one easy at the same time. (Let me know if we want separate bugs for vm and frog on this, but I think it's probably easiest to track in one place.)


Set owner to jimhug@google.com.
Removed Area-VM label.
Added Area-Frog label.

DartBot commented Nov 16, 2011

This comment was originally written by mattsh@google.com


I'm changing the area to frog, just to make sure frog side sees this, and there are some closely related bugs in frog that might make fixing this one easy at the same time. (Let me know if we want separate bugs for vm and frog on this, but I think it's probably easiest to track in one place.)


Set owner to jimhug@google.com.
Removed Area-VM label.
Added Area-Frog label.

@DartBot

This comment has been minimized.

Show comment Hide comment
@DartBot

DartBot Nov 22, 2011

This comment was originally written by jimhug@google.com


Marking as low pri as this would be okay to let slip until next quarter. Please edit the priority if you think this is critical to using frog in the near term.


Removed Priority-Medium label.
Added Priority-Low label.

DartBot commented Nov 22, 2011

This comment was originally written by jimhug@google.com


Marking as low pri as this would be okay to let slip until next quarter. Please edit the priority if you think this is critical to using frog in the near term.


Removed Priority-Medium label.
Added Priority-Low label.

@DartBot

This comment has been minimized.

Show comment Hide comment
@DartBot

DartBot Nov 28, 2011

This comment was originally written by jimhug@google.com


Removed the owner.

DartBot commented Nov 28, 2011

This comment was originally written by jimhug@google.com


Removed the owner.

@DartBot

This comment has been minimized.

Show comment Hide comment
@DartBot

DartBot Jan 11, 2012

This comment was originally written by @sethladd


This appears to be working in the VM now, but not in DartC (try.dartlang.org) or Frog.

Code tested:

void main() {
  var callbacks = [];
  for (var i = 0; i < 10; i++) {
    callbacks.add(() => print(i));
  }
  callbacks.forEach((c) => c());
}

DartBot commented Jan 11, 2012

This comment was originally written by @sethladd


This appears to be working in the VM now, but not in DartC (try.dartlang.org) or Frog.

Code tested:

void main() {
  var callbacks = [];
  for (var i = 0; i < 10; i++) {
    callbacks.add(() => print(i));
  }
  callbacks.forEach((c) => c());
}

@DartBot

This comment has been minimized.

Show comment Hide comment
@DartBot

DartBot Mar 26, 2012

This comment was originally written by sj...@google.com


Also observed this not working at try.dartlang.org... until it is fixed, would it make sense to add a warning to the language tour page under the example bragging about how Dart fixes this relative to JavaScript? :)

DartBot commented Mar 26, 2012

This comment was originally written by sj...@google.com


Also observed this not working at try.dartlang.org... until it is fixed, would it make sense to add a warning to the language tour page under the example bragging about how Dart fixes this relative to JavaScript? :)

@anders-sandholm

This comment has been minimized.

Show comment Hide comment
@anders-sandholm

anders-sandholm Jun 8, 2012

Member

Removed Area-Frog label.
Added Area-Dart2JS, FromAreaFrog labels.

Member

anders-sandholm commented Jun 8, 2012

Removed Area-Frog label.
Added Area-Dart2JS, FromAreaFrog labels.

@kasperl

This comment has been minimized.

Show comment Hide comment
@kasperl

kasperl Jun 12, 2012

Contributor

Added WontFix label.

Contributor

kasperl commented Jun 12, 2012

Added WontFix label.

nex3 pushed a commit that referenced this issue Aug 31, 2016

Merge pull request #333 from dart-lang/self_references
normalize lib/ self-refs to path references

This issue was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment