Capture this.

Capture this.

Committed: https://code.google.com/p/dart/source/detail?r=4082

[floitsch, 2012-02-08 12:26:23]

adding test now (and updating status files, if necessary).

[ngeoffray, 2012-02-08 12:37:58]

LGTM

https://chromiumcodereview.appspot.com/9359011/diff/1/frog/leg/ssa/closure.dart
File frog/leg/ssa/closure.dart (right):

https://chromiumcodereview.appspot.com/9359011/diff/1/frog/leg/ssa/closure.da...
frog/leg/ssa/closure.dart:198: (element === null ||
Please add a:

Elements.isUnbound(Send node, TreeElement elements).

https://chromiumcodereview.appspot.com/9359011/diff/1/frog/leg/ssa/closure.da...
frog/leg/ssa/closure.dart:286: // TODO(floitsch): we should not need to look for
generative constructors.
So why are we doing it?

https://chromiumcodereview.appspot.com/9359011/diff/1/frog/leg/ssa/closure.da...
frog/leg/ssa/closure.dart:289: thisElement = new VariableElement(const
SourceString("this"),
new Element

[floitsch, 2012-02-08 13:17:07]

PTAL.
status files are not yet updated. (if there are any to update).

https://chromiumcodereview.appspot.com/9359011/diff/1/frog/leg/ssa/closure.dart
File frog/leg/ssa/closure.dart (right):

https://chromiumcodereview.appspot.com/9359011/diff/1/frog/leg/ssa/closure.da...
frog/leg/ssa/closure.dart:198: (element === null ||
On 2012/02/08 12:37:58, ngeoffray wrote:
> Please add a:
> 
> Elements.isUnbound(Send node, TreeElement elements).
Now uses isInstanceSend.

https://chromiumcodereview.appspot.com/9359011/diff/1/frog/leg/ssa/closure.da...
frog/leg/ssa/closure.dart:286: // TODO(floitsch): we should not need to look for
generative constructors.
On 2012/02/08 12:37:58, ngeoffray wrote:
> So why are we doing it?

updated comment.

https://chromiumcodereview.appspot.com/9359011/diff/1/frog/leg/ssa/closure.da...
frog/leg/ssa/closure.dart:289: thisElement = new VariableElement(const
SourceString("this"),
On 2012/02/08 12:37:58, ngeoffray wrote:
> new Element

Done.

[ngeoffray, 2012-02-08 13:31:24]

LGTM

https://chromiumcodereview.appspot.com/9359011/diff/4001/frog/leg/ssa/closure...
File frog/leg/ssa/closure.dart (right):

https://chromiumcodereview.appspot.com/9359011/diff/4001/frog/leg/ssa/closure...
frog/leg/ssa/closure.dart:198: Elements.isInstanceSend(node, elements)) {
How about isThisSend in that case? It could also check for 'this' as receiver. I
know visitChildren will do it, but you could just visit the arguments if
isThisSend returns true, or just keep the code as it is, as calling useLocal
multiple times is harmless.

https://chromiumcodereview.appspot.com/9359011/diff/4001/frog/leg/ssa/closure...
frog/leg/ssa/closure.dart:284: // TODO(floitsch): we should not need to look for
generative constructors.
Improve comment?

[floitsch, 2012-02-08 14:36:53]

PTAL.

https://chromiumcodereview.appspot.com/9359011/diff/4001/frog/leg/ssa/closure...
File frog/leg/ssa/closure.dart (right):

https://chromiumcodereview.appspot.com/9359011/diff/4001/frog/leg/ssa/closure...
frog/leg/ssa/closure.dart:198: Elements.isInstanceSend(node, elements)) {
On 2012/02/08 13:31:24, ngeoffray wrote:
> How about isThisSend in that case? It could also check for 'this' as receiver.
I
> know visitChildren will do it, but you could just visit the arguments if
> isThisSend returns true, or just keep the code as it is, as calling useLocal
> multiple times is harmless.

I prefer keeping it the way it is.

https://chromiumcodereview.appspot.com/9359011/diff/4001/frog/leg/ssa/closure...
frog/leg/ssa/closure.dart:284: // TODO(floitsch): we should not need to look for
generative constructors.
On 2012/02/08 13:31:24, ngeoffray wrote:
> Improve comment?

Done.

[ngeoffray, 2012-02-08 16:36:40]

LGTM!

[floitsch, 2012-02-09 14:00:40]

PTAL: I had to add lines 294-301 in closure.dart.

[ngeoffray, 2012-02-09 14:12:29]

Still LGTM