Refactor program traversing logic.

Refactor program traversing logic.

R=smok@google.com

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

[Roman, 2012-08-16 13:53:26]

lgtm

https://chromiumcodereview.appspot.com/10855199/diff/2001/lib/compiler/implem...
File lib/compiler/implementation/dart_backend/backend.dart (right):

https://chromiumcodereview.appspot.com/10855199/diff/2001/lib/compiler/implem...
lib/compiler/implementation/dart_backend/backend.dart:48: final
emptyTreeElements = new TreeElementMapping();
maybe remove this? it is used only twice, it would be better for readability to
explicitly use new TreeElementMapping, and you will actually save 1 loc

https://chromiumcodereview.appspot.com/10855199/diff/2001/lib/compiler/implem...
lib/compiler/implementation/dart_backend/backend.dart:56:
collectReferencedElements(element, treeElements) {
Inline this method? It is used only once. And if you do that, you can define
callbacks inside processElement() without these variable hacks when you define
callbacks later, it is difficult to read this way.

[Anton Muhin, 2012-08-16 14:06:42]

Thanks a lot for review, submitting

https://chromiumcodereview.appspot.com/10855199/diff/2001/lib/compiler/implem...
File lib/compiler/implementation/dart_backend/backend.dart (right):

https://chromiumcodereview.appspot.com/10855199/diff/2001/lib/compiler/implem...
lib/compiler/implementation/dart_backend/backend.dart:48: final
emptyTreeElements = new TreeElementMapping();
I don't want to create a lot of garbage.

On 2012/08/16 13:53:26, Roman wrote:
> maybe remove this? it is used only twice, it would be better for readability
to
> explicitly use new TreeElementMapping, and you will actually save 1 loc

https://chromiumcodereview.appspot.com/10855199/diff/2001/lib/compiler/implem...
lib/compiler/implementation/dart_backend/backend.dart:56:
collectReferencedElements(element, treeElements) {
Alas, variables hacks are a must as Dart requires name to be defined before
usage, but inlining is very good idea, done.

On 2012/08/16 13:53:26, Roman wrote:
> Inline this method? It is used only once. And if you do that, you can define
> callbacks inside processElement() without these variable hacks when you define
> callbacks later, it is difficult to read this way.

[Roman, 2012-08-16 14:23:54]

lgtm

https://chromiumcodereview.appspot.com/10855199/diff/2001/lib/compiler/implem...
File lib/compiler/implementation/dart_backend/backend.dart (right):

https://chromiumcodereview.appspot.com/10855199/diff/2001/lib/compiler/implem...
lib/compiler/implementation/dart_backend/backend.dart:48: final
emptyTreeElements = new TreeElementMapping();
On 2012/08/16 14:06:42, Anton Muhin wrote:
> I don't want to create a lot of garbage.
> 
> On 2012/08/16 13:53:26, Roman wrote:
> > maybe remove this? it is used only twice, it would be better for readability
> to
> > explicitly use new TreeElementMapping, and you will actually save 1 loc
> 

Why not?

https://chromiumcodereview.appspot.com/10855199/diff/6001/lib/compiler/implem...
File lib/compiler/implementation/dart_backend/backend.dart (right):

https://chromiumcodereview.appspot.com/10855199/diff/6001/lib/compiler/implem...
lib/compiler/implementation/dart_backend/backend.dart:57:
processElement(element, treeElements) {
How about refactoring all this stuff to a separate class? Its "processing"
output will be top level elements and classes to output.

[Anton Muhin, 2012-08-16 14:30:44]

https://chromiumcodereview.appspot.com/10855199/diff/2001/lib/compiler/implem...
File lib/compiler/implementation/dart_backend/backend.dart (right):

https://chromiumcodereview.appspot.com/10855199/diff/2001/lib/compiler/implem...
lib/compiler/implementation/dart_backend/backend.dart:48: final
emptyTreeElements = new TreeElementMapping();
Why stress GC unnecessarily?

On 2012/08/16 14:23:54, Roman wrote:
> On 2012/08/16 14:06:42, Anton Muhin wrote:
> > I don't want to create a lot of garbage.
> > 
> > On 2012/08/16 13:53:26, Roman wrote:
> > > maybe remove this? it is used only twice, it would be better for
readability
> > to
> > > explicitly use new TreeElementMapping, and you will actually save 1 loc
> > 
> 
> Why not?

https://chromiumcodereview.appspot.com/10855199/diff/6001/lib/compiler/implem...
File lib/compiler/implementation/dart_backend/backend.dart (right):

https://chromiumcodereview.appspot.com/10855199/diff/6001/lib/compiler/implem...
lib/compiler/implementation/dart_backend/backend.dart:57:
processElement(element, treeElements) {
That sounds really interesting, thanks a lot, let me give it a try.

On 2012/08/16 14:23:54, Roman wrote:
> How about refactoring all this stuff to a separate class? Its "processing"
> output will be top level elements and classes to output.