Support some usages of empty classes.

Support some usages of empty classes.

Positions like is, as, return types and argument types
should be now processed.

R=smok@google.com

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

[Anton Muhin, 2012-08-09 19:48:21]

Roma,

I have some more changes in pipeline, but they depend on this one.  Just fyi.

[Roman, 2012-08-10 07:27:39]

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

https://chromiumcodereview.appspot.com/10826234/diff/1/lib/compiler/implement...
lib/compiler/implementation/dart_backend/backend.dart:80: new
ReferencedElementCollector(
This is probably controversial, but I think "typedefs" and "classes" should not
be passed to constructor, better after collect() take them out of
ReferencedElementCollector. Otherwise here it is not clear what you pass to
ReferencedElementCollector as "input" element (like compiler, element and
treeElements) and what - as "output" (typedefs, classes).

https://chromiumcodereview.appspot.com/10826234/diff/1/lib/compiler/implement...
lib/compiler/implementation/dart_backend/backend.dart:108: if
(resolvedClassMembers.containsKey(classElement)) return;
Why not unify "classes" collection and "resolvedClassMembers"? The latter may be
renamed to "resolvedClasses". If referenced class does not have any members we
care about, the set it maps to will just be null. That's one less forEach here.

https://chromiumcodereview.appspot.com/10826234/diff/1/lib/compiler/implement...
lib/compiler/implementation/dart_backend/backend.dart:163: if (element !== null)
{
This IF may be changed to just "compiler.resolveTypeAnnotation()" and then
addElement(type.element).

[Anton Muhin, 2012-08-10 08:33:37]

PTAL

I added a suppression for try_catch_test as it has been migrated to new syntax
this night and this syntax is not supported by dart2js yet.

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

https://chromiumcodereview.appspot.com/10826234/diff/1/lib/compiler/implement...
lib/compiler/implementation/dart_backend/backend.dart:80: new
ReferencedElementCollector(
Indeed.  For example, I really don't like that you set all those
currentElement/treeElements when entering collect method---I personally prefer
immutability of references/state.

On 2012/08/10 07:27:40, Roman wrote:
> This is probably controversial, but I think "typedefs" and "classes" should
not
> be passed to constructor, better after collect() take them out of
> ReferencedElementCollector. Otherwise here it is not clear what you pass to
> ReferencedElementCollector as "input" element (like compiler, element and
> treeElements) and what - as "output" (typedefs, classes).

https://chromiumcodereview.appspot.com/10826234/diff/1/lib/compiler/implement...
lib/compiler/implementation/dart_backend/backend.dart:108: if
(resolvedClassMembers.containsKey(classElement)) return;
I am considering this too.  But for now I'd prefer to have minimal coupling
between components: this code just collects additional stuff and knows nothing
about the rest of system.  That usually pays off.

On 2012/08/10 07:27:40, Roman wrote:
> Why not unify "classes" collection and "resolvedClassMembers"? The latter may
be
> renamed to "resolvedClasses". If referenced class does not have any members we
> care about, the set it maps to will just be null. That's one less forEach
here.

https://chromiumcodereview.appspot.com/10826234/diff/1/lib/compiler/implement...
lib/compiler/implementation/dart_backend/backend.dart:163: if (element !== null)
{
Good point, done.

On 2012/08/10 07:27:40, Roman wrote:
> This IF may be changed to just "compiler.resolveTypeAnnotation()" and then
> addElement(type.element).

[Roman, 2012-08-10 08:46:01]

lgtm

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

https://chromiumcodereview.appspot.com/10826234/diff/1/lib/compiler/implement...
lib/compiler/implementation/dart_backend/backend.dart:108: if
(resolvedClassMembers.containsKey(classElement)) return;
On 2012/08/10 08:33:37, Anton Muhin wrote:
> I am considering this too.  But for now I'd prefer to have minimal coupling
> between components: this code just collects additional stuff and knows nothing
> about the rest of system.  That usually pays off.
> 
> On 2012/08/10 07:27:40, Roman wrote:
> > Why not unify "classes" collection and "resolvedClassMembers"? The latter
may
> be
> > renamed to "resolvedClasses". If referenced class does not have any members
we
> > care about, the set it maps to will just be null. That's one less forEach
> here.
> 

"Not knowing about the rest of system" is not quite true. Here you utput
elements from "classes" you check another contatiner "resolvedClassMembers", so
here is coupling.

Populating one container from different sources (collectors) does not add
anything to coupling, one collector does not know anything about the other. but
if you unify them it would tell that we actually collecting the same kind of
objects - classes that we use. And we even process them in the same way - with
outputClass.

The downsides of current approach:
1) you have to check that you don't accidentally output the same element twice,
hence the check that classElement is not in resolvedClassMembers.
2) duplicating code that outputs the classes: from "classes" and from
"resolvedClassMembers".

[Anton Muhin, 2012-08-10 08:56:48]

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

https://chromiumcodereview.appspot.com/10826234/diff/1/lib/compiler/implement...
lib/compiler/implementation/dart_backend/backend.dart:108: if
(resolvedClassMembers.containsKey(classElement)) return;
I meant pretty simple thing: ReferencedElementCollector knows nothing about the
rest of the system.

I am seriously considering provide callbacks instead of collections, but I'ld
like to see how this part of system grows to take the ultimate decision.

On 2012/08/10 08:46:01, Roman wrote:
> On 2012/08/10 08:33:37, Anton Muhin wrote:
> > I am considering this too.  But for now I'd prefer to have minimal coupling
> > between components: this code just collects additional stuff and knows
nothing
> > about the rest of system.  That usually pays off.
> > 
> > On 2012/08/10 07:27:40, Roman wrote:
> > > Why not unify "classes" collection and "resolvedClassMembers"? The latter
> may
> > be
> > > renamed to "resolvedClasses". If referenced class does not have any
members
> we
> > > care about, the set it maps to will just be null. That's one less forEach
> > here.
> > 
> 
> "Not knowing about the rest of system" is not quite true. Here you utput
> elements from "classes" you check another contatiner "resolvedClassMembers",
so
> here is coupling.
> 
> Populating one container from different sources (collectors) does not add
> anything to coupling, one collector does not know anything about the other.
but
> if you unify them it would tell that we actually collecting the same kind of
> objects - classes that we use. And we even process them in the same way - with
> outputClass.
> 
> The downsides of current approach:
> 1) you have to check that you don't accidentally output the same element
twice,
> hence the check that classElement is not in resolvedClassMembers.
> 2) duplicating code that outputs the classes: from "classes" and from
> "resolvedClassMembers".