Introduce AST cloning.

Introduce AST cloning.

As a temporary testing measure, clone AST of most elements.

R=smok@google.com

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

[Anton Muhin, 2012-09-06 15:58:48]

That turned out more complicated than I expected originally, so I'd rather run
it by you before start to employ for additional minifications.

[Roman, 2012-09-07 08:10:56]

General question - why do we need cloning?

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

https://chromiumcodereview.appspot.com/10919127/diff/1/lib/compiler/implement...
lib/compiler/implementation/dart_backend/placeholder_collector.dart:201: if
(implementingFactory is !SynthesizedConstructorElement) {
Why this check? If it fixes something, please add a test (or better fix it in a
separate CL). I also remember there was a CL that changed something in
shouldOutput about SynthesizedConstructorElement, is it related?

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

https://chromiumcodereview.appspot.com/10919127/diff/1/lib/compiler/implement...
lib/compiler/implementation/dart_backend/utils.dart:9:
CloningVisitor(this.originalTreeElements)
I suggest removing TreeElements cloning from this visitor to another place. This
would make CloningVisitor more simple from client perspective.

https://chromiumcodereview.appspot.com/10919127/diff/1/tests/compiler/dart2js...
File tests/compiler/dart2js/unparser_test.dart (right):

https://chromiumcodereview.appspot.com/10919127/diff/1/tests/compiler/dart2js...
tests/compiler/dart2js/unparser_test.dart:477: class DynoMap implements
Map<Element, ElementAst> {
I don't understand "DynoMap" name, what does it mean?

[Anton Muhin, 2012-09-07 12:26:07]

Regarding cloning.  Most of the fields in Node hierarchy are final, that's
technical issue.

Overall, I don't see huge problem here even though right now performance may
suffer.  In this case we can always address that later by various means starting
from dropping final modifiers to lazy tree rewrite.

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

https://chromiumcodereview.appspot.com/10919127/diff/1/lib/compiler/implement...
lib/compiler/implementation/dart_backend/placeholder_collector.dart:201: if
(implementingFactory is !SynthesizedConstructorElement) {
This is somewhat related as we started to drop synthesize constructors earlier.

On the other hand, there is no need to create a placeholder for them anyway as
they won't get output.

If you like I can fix it differently by building AST even for synthesize ctors
and move discarding them back to emitter, but I am not sure it's really better.

On 2012/09/07 08:10:56, Roman wrote:
> Why this check? If it fixes something, please add a test (or better fix it in
a
> separate CL). I also remember there was a CL that changed something in
> shouldOutput about SynthesizedConstructorElement, is it related?

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

https://chromiumcodereview.appspot.com/10919127/diff/1/lib/compiler/implement...
lib/compiler/implementation/dart_backend/utils.dart:9:
CloningVisitor(this.originalTreeElements)
I might be missing some simple solution, but I found no easier way to recreate
this mapping after the cloning.  Do you have any particular ideas in mind?

On 2012/09/07 08:10:56, Roman wrote:
> I suggest removing TreeElements cloning from this visitor to another place.
This
> would make CloningVisitor more simple from client perspective.

https://chromiumcodereview.appspot.com/10919127/diff/1/tests/compiler/dart2js...
File tests/compiler/dart2js/unparser_test.dart (right):

https://chromiumcodereview.appspot.com/10919127/diff/1/tests/compiler/dart2js...
tests/compiler/dart2js/unparser_test.dart:477: class DynoMap implements
Map<Element, ElementAst> {
On 2012/09/07 08:10:56, Roman wrote:
> I don't understand "DynoMap" name, what does it mean?

Name is horrible, sorry.  I meant that there is no real data, but they are
rather synthesized on the fly.  Any better ideas?  Something like ParsingMap? 
Sounds horrible too.

Another option would be to forget about map altogether and pass a function to
convert an element into element ast, but you usually are so fond of OOP :)

[Roman, 2012-09-07 13:19:59]

lgtm

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

https://chromiumcodereview.appspot.com/10919127/diff/1/lib/compiler/implement...
lib/compiler/implementation/dart_backend/utils.dart:9:
CloningVisitor(this.originalTreeElements)
On 2012/09/07 12:26:07, Anton Muhin wrote:
> I might be missing some simple solution, but I found no easier way to recreate
> this mapping after the cloning.  Do you have any particular ideas in mind?
> 
> On 2012/09/07 08:10:56, Roman wrote:
> > I suggest removing TreeElements cloning from this visitor to another place.
> This
> > would make CloningVisitor more simple from client perspective.
> 

Nothing particular.