Fix order of visitChildren in Constructors.

Fix order of visitChildren in Constructors.

BUG=
R=ahe@google.com, kmillikin@google.com

Committed: https://github.com/dart-lang/sdk/commit/4250923832db68504cdafaebbb6128e0ec8fffd8

[Emily Fortuna, 2017-06-27 22:59:47]

efortuna@google.com changed reviewers:
+ ahe@google.com, kmillikin@google.com

[Emily Fortuna, 2017-06-27 22:59:48]

It's very possible I'm missing something, but this seems like an ordering error.
I believe we want to traverse the initializers before the body of the
constructor.
lmk if I'm mistaken. (... it does seem odd that this would be found now if it is
indeed a bug)

[ahe, 2017-06-28 07:31:57]

lgtm

I think the reason why nobody has noticed this, is that they might not have used
visitChildren if the order mattered.

[Kevin Millikin (Google), 2017-06-28 08:41:55]

LGTM if you change transformChildren as well.

It's OK to change it, but we don't intend that anyone ever relies on the order
of visitChildren.  The contract is that it visits the children in some
unspecified order.  If you find you need a specific order, it's better to
implement visitConstructor and visit the children in an order that you control.

(We have ideas for a better story around transformations.  Writing them in a
more functional style when possible, where they don't depend on the order that
subtrees are visited, is helpful.  And making it explicit by overriding
visitConstructor when they do depend on the order is also helpful.)

[Emily Fortuna, 2017-06-29 01:12:50]

Description was changed from

==========
Fix order of visitChildren in Constructors.

BUG=
==========

to

==========
Fix order of visitChildren in Constructors.

BUG=
R=ahe@google.com, kmillikin@google.com

Committed:
https://github.com/dart-lang/sdk/commit/4250923832db68504cdafaebbb6128e0ec8fffd8
==========

[Emily Fortuna, 2017-06-29 01:12:51]

Committed patchset #2 (id:20001) manually as
4250923832db68504cdafaebbb6128e0ec8fffd8 (presubmit successful).

[Dmitry Stefantsov, 2017-06-29 14:37:48]

Hi Emily, Peter, and Kevin,

I found a place that relies on the order of visiting children of constructors. 
It's the information gathering pass of closure conversion.  Apparently, it
wanted to visit declarations of constructor parameters (that are children of the
function associated with the constructor) before it encounters usages of these
variables in initializers.  So, it may be the reason behind the original
visiting order: to visit variable declarations before they are used.

As Kevin wrote, the code should define the visiting order explicitly if it needs
one.  So, I'll update this pass of closure conversion accordingly.

But thanks for this finding!  Without this change it was hard to see that the
code had an implicit requirement.