Process typedefs, referenced from the program.

Process typedefs, referenced from the program.

R=smok@google.com

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

[Roman, 2012-08-09 16:48:03]

lgtm

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

https://chromiumcodereview.appspot.com/10854067/diff/1/lib/compiler/implement...
lib/compiler/implementation/dart_backend/backend.dart:80: compiler,
style nazi: Can you please make it more compact? It does not look nice like
this, it is also easy to skip call to collect() while reading this.

https://chromiumcodereview.appspot.com/10854067/diff/1/lib/compiler/implement...
lib/compiler/implementation/dart_backend/backend.dart:105:
typedefs.forEach(emitter.outputElement);
I think we also want to traverse typedef nodes, because they can have references
to yet not used types.

PS I guess that's why the test failed. Not necessary task for this cl, just to
keep a note.

https://chromiumcodereview.appspot.com/10854067/diff/1/lib/compiler/implement...
lib/compiler/implementation/dart_backend/backend.dart:136: class
AdditionalElementCollector extends AbstractVisitor {
Please think about better name. What is "additional element" is unclear. We are
trying to collect all used types here, so maybe some name telling about type
collection?

https://chromiumcodereview.appspot.com/10854067/diff/1/lib/compiler/implement...
lib/compiler/implementation/dart_backend/backend.dart:150: Element element =
treeElements[typeAnnotation];
I think canonically types should be resolved through treeElements.getType() . Is
there any difference if you try that?

[ahe, 2012-08-09 16:51:55]

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

https://chromiumcodereview.appspot.com/10854067/diff/1/lib/compiler/implement...
lib/compiler/implementation/dart_backend/backend.dart:152: if
(element.isTypedef()) typedefs.add(element as TypedefElement);
The cast is unnecessary. Dart is a dynamically typed language with optional
types. Respect the dynamic nature :-)

[Anton Muhin, 2012-08-09 17:01:08]

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

https://chromiumcodereview.appspot.com/10854067/diff/1/lib/compiler/implement...
lib/compiler/implementation/dart_backend/backend.dart:80: compiler,
On 2012/08/09 16:48:03, Roman wrote:
> style nazi: Can you please make it more compact? It does not look nice like
> this, it is also easy to skip call to collect() while reading this.

Done.

https://chromiumcodereview.appspot.com/10854067/diff/1/lib/compiler/implement...
lib/compiler/implementation/dart_backend/backend.dart:105:
typedefs.forEach(emitter.outputElement);
I believe we traverse those typedefs as a part of collector (it does
visitChildren).  But we just do not emit empty classes back.

On 2012/08/09 16:48:03, Roman wrote:
> I think we also want to traverse typedef nodes, because they can have
references
> to yet not used types.
> 
> PS I guess that's why the test failed. Not necessary task for this cl, just to
> keep a note.

https://chromiumcodereview.appspot.com/10854067/diff/1/lib/compiler/implement...
lib/compiler/implementation/dart_backend/backend.dart:136: class
AdditionalElementCollector extends AbstractVisitor {
Ok, is it any better?

On 2012/08/09 16:48:03, Roman wrote:
> Please think about better name. What is "additional element" is unclear. We
are
> trying to collect all used types here, so maybe some name telling about type
> collection?

https://chromiumcodereview.appspot.com/10854067/diff/1/lib/compiler/implement...
lib/compiler/implementation/dart_backend/backend.dart:150: Element element =
treeElements[typeAnnotation];
Right now I don't want to resolve types, meaning I don't want get Type instance
back, at least for typedefs.  I might be forced to do that for classes though. 
Let's see.

On 2012/08/09 16:48:03, Roman wrote:
> I think canonically types should be resolved through treeElements.getType() .
Is
> there any difference if you try that?

https://chromiumcodereview.appspot.com/10854067/diff/1/lib/compiler/implement...
lib/compiler/implementation/dart_backend/backend.dart:152: if
(element.isTypedef()) typedefs.add(element as TypedefElement);
Sure, I was just concerned if some tool will give me a warning here as a put
Element into Set<TypedefElement>.

On 2012/08/09 16:51:55, ahe wrote:
> The cast is unnecessary. Dart is a dynamically typed language with optional
> types. Respect the dynamic nature :-)