Change visitLoop to handleLoop that takes closures as arguments.

Change visitLoop to handleLoop that takes closures as arguments.

This allows easy reuse in for-in loops.

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

[ngeoffray, 2012-03-21 07:58:47]

LGTM!

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

https://chromiumcodereview.appspot.com/9753005/diff/1/frog/leg/ssa/builder.da...
frog/leg/ssa/builder.dart:1099: // The condition function must return a boolean
result.
Since they'are all doing, why not making it uniform and doing a popBoolified in
this method, and typing the return type as void (like the other parameters)?

https://chromiumcodereview.appspot.com/9753005/diff/1/frog/leg/ssa/builder.da...
frog/leg/ssa/builder.dart:1216: node.initializer === null ? null :
buildInitializer,
How about (for this one and all others):

node.initializer === null ? () {} : buildInitializer,

in order to avoid the null checks in handleLoop. There is one extra check for
the condition, but the check is already in handleLoop (and not necessary for a
while and for...in).

https://chromiumcodereview.appspot.com/9753005/diff/1/frog/leg/ssa/builder.da...
frog/leg/ssa/builder.dart:1223: handleLoop(node, null, HInstruction condition()
{ visit(node.condition);
I'd prefer one argument per line, or write the condition function as a statement
before. The formatting of the condition parameter looks horrible here.

https://chromiumcodereview.appspot.com/9753005/diff/1/frog/leg/ssa/builder.da...
frog/leg/ssa/builder.dart:2311: void buildInitialize() {
buildInitializer?

https://chromiumcodereview.appspot.com/9753005/diff/1/frog/leg/ssa/builder.da...
frog/leg/ssa/builder.dart:2324: selector, const SourceString('hasNext'),
[iterator]));
<Instruction>[iterator]

[Lasse Reichstein Nielsen, 2012-03-21 09:00:15]

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

https://chromiumcodereview.appspot.com/9753005/diff/1/frog/leg/ssa/builder.da...
frog/leg/ssa/builder.dart:1099: // The condition function must return a boolean
result.
I expect to use this one in switch-with-continue, where the condition will be
"true". I.e., it won't need the boolify.
For that reason I don't want the handleLoop to always add a boolify.

I could make it pass the condition instruction on the stack instead of as a
result, but I actually find this more readable.

https://chromiumcodereview.appspot.com/9753005/diff/1/frog/leg/ssa/builder.da...
frog/leg/ssa/builder.dart:1216: node.initializer === null ? null :
buildInitializer,
In this case, I could just put the test into the buildInitializer, etc.,
functions and pass them unconditionally.

This is really the only case where parts can be missing, so it's the only case
where we might pass null. As such, it makes sense that this is the place where
we handle that case.

On the other hand, the difference between a while and a for loop is that the for
loop is the only one with an update block. I might want to be able to record
this in the LoopInformation (at some point). But that's being speculative, and
it's easily changable. I'll make the functions all non-null.

https://chromiumcodereview.appspot.com/9753005/diff/1/frog/leg/ssa/builder.da...
frog/leg/ssa/builder.dart:1223: handleLoop(node, null, HInstruction condition()
{ visit(node.condition);
Done.

https://chromiumcodereview.appspot.com/9753005/diff/1/frog/leg/ssa/builder.da...
frog/leg/ssa/builder.dart:2311: void buildInitialize() {
On 2012/03/21 07:58:47, ngeoffray wrote:
> buildInitializer?

Done.

https://chromiumcodereview.appspot.com/9753005/diff/1/frog/leg/ssa/builder.da...
frog/leg/ssa/builder.dart:2324: selector, const SourceString('hasNext'),
[iterator]));
On 2012/03/21 07:58:47, ngeoffray wrote:
> <Instruction>[iterator]

Done.