Rename elements after sorting.

Rename elements after sorting.

That makes renaming more stable.

R=smok@google.com

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

[Roman, 2012-08-21 07:59:46]

lgtm

https://chromiumcodereview.appspot.com/10834416/diff/1/lib/compiler/implement...
File lib/compiler/implementation/dart_backend/renamer.dart (right):

https://chromiumcodereview.appspot.com/10834416/diff/1/lib/compiler/implement...
lib/compiler/implementation/dart_backend/renamer.dart:56: for (Node node in
placeholderCollector.unresolvedNodes) {
Should we use sortedForEach here too? Otherwise it is undefined which unresolved
we meet first. We can sort nodes by its hashcodes.

https://chromiumcodereview.appspot.com/10834416/diff/1/lib/compiler/implement...
lib/compiler/implementation/dart_backend/renamer.dart:60: sortedForEach(mapping,
f) {
Great method, but can you please make it more readable? Now it looks like
one-lined strcpy.
1) Add types to parameters. It is important that we have a map which keys are
elements.
2) this may be controversial, but I think 'final' makes little sense if used on
variables (as opposed to common usage with fields), as a reader I care about
variable type instead of whether it is final or not.
3) Please don't one-line for-s, while-s etc, it is difficult to read:
http://www.dartlang.org/articles/style-guide/#do-use-curly-braces-for-all-flo...

[Anton Muhin, 2012-08-21 10:15:26]

https://chromiumcodereview.appspot.com/10834416/diff/1/lib/compiler/implement...
File lib/compiler/implementation/dart_backend/renamer.dart (right):

https://chromiumcodereview.appspot.com/10834416/diff/1/lib/compiler/implement...
lib/compiler/implementation/dart_backend/renamer.dart:56: for (Node node in
placeholderCollector.unresolvedNodes) {
Okay.  I'll implement somewhat suboptimal solution sorting by tokens.  I don't
want to sort by hash codes as they depend on order of node construction.

Fortunately enough, I don't think we'll have tons of cases for unresolved
classes in realistic programs/tests.

On 2012/08/21 07:59:46, Roman wrote:
> Should we use sortedForEach here too? Otherwise it is undefined which
unresolved
> we meet first. We can sort nodes by its hashcodes.

https://chromiumcodereview.appspot.com/10834416/diff/1/lib/compiler/implement...
lib/compiler/implementation/dart_backend/renamer.dart:60: sortedForEach(mapping,
f) {
On 2012/08/21 07:59:46, Roman wrote:
> Great method, but can you please make it more readable? Now it looks like
> one-lined strcpy.
> 1) Add types to parameters. It is important that we have a map which keys are
> elements.
> 2) this may be controversial, but I think 'final' makes little sense if used
on
> variables (as opposed to common usage with fields), as a reader I care about
> variable type instead of whether it is final or not.
> 3) Please don't one-line for-s, while-s etc, it is difficult to read:
>
http://www.dartlang.org/articles/style-guide/#do-use-curly-braces-for-all-flo...

Done.

[Anton Muhin, 2012-08-21 12:19:04]

PTAL.

Yes, I remember your LGTM, but I reworked it notably, so I would appreciate
another look.

[Roman, 2012-08-21 12:37:39]

lgtm

https://chromiumcodereview.appspot.com/10834416/diff/9/lib/compiler/implement...
File lib/compiler/implementation/dart_backend/backend.dart (right):

https://chromiumcodereview.appspot.com/10834416/diff/9/lib/compiler/implement...
lib/compiler/implementation/dart_backend/backend.dart:186: List sorted(List l,
comparison) {
'sortList'?

[Anton Muhin, 2012-08-21 12:57:42]

https://chromiumcodereview.appspot.com/10834416/diff/9/lib/compiler/implement...
File lib/compiler/implementation/dart_backend/backend.dart (right):

https://chromiumcodereview.appspot.com/10834416/diff/9/lib/compiler/implement...
lib/compiler/implementation/dart_backend/backend.dart:186: List sorted(List l,
comparison) {
On 2012/08/21 12:37:39, Roman wrote:
> 'sortList'? 

sorted matches nicely Python, mind if I keep it this way?