Add name and library to selectors.

Add name and library to selectors.

This is the first step towards improving the Selector as an
abstraction used by the resolver, builder, and enqueur. The
intent is to have the resolver construct selectors that can
be used for method lookup, tree shaking, and for simplifying
the logic we use to build the correct SSA instructions for
sends (some of which are quite complex).

Next step is to get rid of more of the adhoc selector building
that's done outside of the resolver and clean up how we resolve
sends. After that, I'll start removing the need for passing 
around both a name and a selector in a lot of places. 

R=ahe@google.com,sgjesse@google.com
BUG=

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

[kasperl, 2012-08-14 08:15:43]

Adding Anton on the CC list.

[kasperl, 2012-08-14 08:18:46]

https://chromiumcodereview.appspot.com/10825337/diff/1/lib/compiler/implement...
File lib/compiler/implementation/universe.dart (right):

https://chromiumcodereview.appspot.com/10825337/diff/1/lib/compiler/implement...
lib/compiler/implementation/universe.dart:88: final LibraryElement library;  //
XXX: Should be null for non-private names?
I think I need to make sure this is actually null for non-private names in order
for the tree shaker to do its job properly.

https://chromiumcodereview.appspot.com/10825337/diff/1/lib/compiler/implement...
lib/compiler/implementation/universe.dart:98: this.library,
I'll make the library null for non-private names here.

[Søren Gjesse, 2012-08-14 12:01:38]

lgtm

https://chromiumcodereview.appspot.com/10825337/diff/1/lib/compiler/implement...
File lib/compiler/implementation/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/10825337/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/builder.dart:1846: void
pushInvokeHelper0(Element helper) {
How about "pushInvokeStatic0" and rename argument "helper" to "element"?

https://chromiumcodereview.appspot.com/10825337/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/builder.dart:1854: void
pushInvokeHelper1(Element helper, HInstruction a0) {
Ditto.

https://chromiumcodereview.appspot.com/10825337/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/builder.dart:1862: void
pushInvokeHelper2(Element helper, HInstruction a0, HInstruction a1) {
Ditto.

https://chromiumcodereview.appspot.com/10825337/diff/1/lib/compiler/implement...
File lib/compiler/implementation/universe.dart (right):

https://chromiumcodereview.appspot.com/10825337/diff/1/lib/compiler/implement...
lib/compiler/implementation/universe.dart:78: static final SelectorKind
INVOCATION = const SelectorKind('invocation');
Rename INVOCATION and 'invocation' to CALL and 'call'?

https://chromiumcodereview.appspot.com/10825337/diff/6001/lib/compiler/implem...
File lib/compiler/implementation/resolver.dart (right):

https://chromiumcodereview.appspot.com/10825337/diff/6001/lib/compiler/implem...
lib/compiler/implementation/resolver.dart:1212: } else if (isSet) {
Maybe get rid of the else here.

[kasperl, 2012-08-14 12:15:46]

Thanks for the comments! Landing as soon as all tests pass again after merging.

https://chromiumcodereview.appspot.com/10825337/diff/1/lib/compiler/implement...
File lib/compiler/implementation/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/10825337/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/builder.dart:1846: void
pushInvokeHelper0(Element helper) {
On 2012/08/14 12:01:38, Søren Gjesse wrote:
> How about "pushInvokeStatic0" and rename argument "helper" to "element"?

I'll fix this in another CL. Still trying to merge in the latest changes and get
everything working again ;-)

https://chromiumcodereview.appspot.com/10825337/diff/1/lib/compiler/implement...
File lib/compiler/implementation/universe.dart (right):

https://chromiumcodereview.appspot.com/10825337/diff/1/lib/compiler/implement...
lib/compiler/implementation/universe.dart:78: static final SelectorKind
INVOCATION = const SelectorKind('invocation');
On 2012/08/14 12:01:38, Søren Gjesse wrote:
> Rename INVOCATION and 'invocation' to CALL and 'call'?

Will do in next CL.

[ahe, 2012-08-15 07:37:59]

LGTM

https://chromiumcodereview.appspot.com/10825337/diff/7016/lib/compiler/implem...
File lib/compiler/implementation/enqueue.dart (right):

https://chromiumcodereview.appspot.com/10825337/diff/7016/lib/compiler/implem...
lib/compiler/implementation/enqueue.dart:237: compiler.internalError("Wrong
selector name: $message.");
Style-wise, I think I'd prefer:

compiler.internalError("Wrong selector name: "
                       "$name != ${selector.name} (${selector.kind})");

https://chromiumcodereview.appspot.com/10825337/diff/7016/lib/compiler/implem...
File lib/compiler/implementation/resolver.dart (right):

https://chromiumcodereview.appspot.com/10825337/diff/7016/lib/compiler/implem...
lib/compiler/implementation/resolver.dart:1139: name = selector.name;
Yay!

https://chromiumcodereview.appspot.com/10825337/diff/7016/lib/compiler/implem...
lib/compiler/implementation/resolver.dart:1186: static Selector
computeSendSelector(Send node, LibraryElement library) {
Should this be moved somewhere less arbitrary?

https://chromiumcodereview.appspot.com/10825337/diff/7016/lib/compiler/implem...
lib/compiler/implementation/resolver.dart:1188: bool isSet = node is SendSet;
Please use:

node.asSendSet() !== null

https://chromiumcodereview.appspot.com/10825337/diff/7016/lib/compiler/implem...
lib/compiler/implementation/resolver.dart:1196: switch (source.stringValue) {
This is faster:

String value = source.stringValue;
if (value === '!' || ...) {
...

I have taken care to ensure you can use identity checks on these strings.

https://chromiumcodereview.appspot.com/10825337/diff/7016/lib/compiler/implem...
lib/compiler/implementation/resolver.dart:1203: return node.arguments.isEmpty()
I like!

https://chromiumcodereview.appspot.com/10825337/diff/7016/lib/compiler/implem...
lib/compiler/implementation/resolver.dart:1210: assert(!isSet);
Internal error?

https://chromiumcodereview.appspot.com/10825337/diff/7016/lib/compiler/implem...
lib/compiler/implementation/resolver.dart:1218: List<SourceString> named =
<SourceString>[];
Why not use a Link?

https://chromiumcodereview.appspot.com/10825337/diff/7016/lib/compiler/implem...
lib/compiler/implementation/resolver.dart:1224: named.add((argument as
NamedArgument).name.source);
Please don't use casts :-(((((

You could write it like this instead:

NamedArgument namedArgument = argument.asNamedArgument();
if (namedArgument !== null) {
  named.add(namedArgument.name.source);
}

https://chromiumcodereview.appspot.com/10825337/diff/7016/lib/compiler/implem...
lib/compiler/implementation/resolver.dart:1244: void visitArguments(NodeList
list) {
I find it confusing to have a visitFoo which isn't a visitor method.

https://chromiumcodereview.appspot.com/10825337/diff/7016/lib/compiler/implem...
lib/compiler/implementation/resolver.dart:1326: if (compoundAssignment) {
Nice simplification.

https://chromiumcodereview.appspot.com/10825337/diff/7016/lib/compiler/implem...
lib/compiler/implementation/resolver.dart:1340: if (node.isIndex && node is
SendSet) {
node.asSendSet !== null

https://chromiumcodereview.appspot.com/10825337/diff/7016/lib/compiler/implem...
lib/compiler/implementation/resolver.dart:1366: // Also register the getter for
compound assignments.
FYI: Clearly this is a hack. The getter should only be registered if we know
this is a compound assignment.

I should have added a TODO here when I wrote this code.

https://chromiumcodereview.appspot.com/10825337/diff/7016/lib/compiler/implem...
lib/compiler/implementation/resolver.dart:1536: Selector selector = new
Selector.call(name, library, 0);
Seems like a helper method might be nice for handling this.

https://chromiumcodereview.appspot.com/10825337/diff/7016/lib/compiler/implem...
File lib/compiler/implementation/universe.dart (right):

https://chromiumcodereview.appspot.com/10825337/diff/7016/lib/compiler/implem...
lib/compiler/implementation/universe.dart:88: final SourceString name;       //
Name is null for call-any selectors.
Wouldn't it better using a documentation comment?