Make selector registration in the resolver and code generator more explicit.

Make selector registration in the resolver and code generator more explicit.

Get rid of registerFieldClosureInvocations() in the enqueuer by explicit
registering the closure call selectors when we may be calling a getter in
the resolver and the code generator.

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

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

[floitsch, 2012-08-15 14:14:04]

LGTM.

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

https://chromiumcodereview.appspot.com/10854158/diff/3009/lib/compiler/implem...
lib/compiler/implementation/enqueue.dart:327: do {
remove the inner loop.

https://chromiumcodereview.appspot.com/10854158/diff/3009/lib/compiler/implem...
File lib/compiler/implementation/namer.dart (right):

https://chromiumcodereview.appspot.com/10854158/diff/3009/lib/compiler/implem...
lib/compiler/implementation/namer.dart:38: static final SourceString
CLOSURE_INVOCATION_NAME =
we should remove the CLOSURE_INVOCATION_NAME from the Namer and use the
CALL_OPERATOR_NAME directly.
Having statics in the namer is imho a bad thing. We want to be able to switch
it.

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

https://chromiumcodereview.appspot.com/10854158/diff/3009/lib/compiler/implem...
lib/compiler/implementation/resolver.dart:1313: bool isCompound =
identifier.source.stringValue !== '=';
this should be a predicate on the SendSet node.
Not necessarily in this CL.

https://chromiumcodereview.appspot.com/10854158/diff/3009/lib/compiler/implem...
File lib/compiler/implementation/ssa/codegen.dart (right):

https://chromiumcodereview.appspot.com/10854158/diff/3009/lib/compiler/implem...
lib/compiler/implementation/ssa/codegen.dart:1478: // TODO(floitsch): we should
have a separate list for closure invocations.
remove comment.

[ahe, 2012-08-15 14:32:21]

Comments so far.

https://chromiumcodereview.appspot.com/10854158/diff/3009/lib/compiler/implem...
File lib/compiler/implementation/enqueue.dart (left):

https://chromiumcodereview.appspot.com/10854158/diff/3009/lib/compiler/implem...
lib/compiler/implementation/enqueue.dart:356: registerFieldClosureInvocations();
Yay!

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

https://chromiumcodereview.appspot.com/10854158/diff/3009/lib/compiler/implem...
lib/compiler/implementation/enqueue.dart:327: do {
Remove this loop.

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

https://chromiumcodereview.appspot.com/10854158/diff/3009/lib/compiler/implem...
lib/compiler/implementation/resolver.dart:968: return element;
These two lines can be merged.
And yay!

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

https://chromiumcodereview.appspot.com/10854158/diff/3009/lib/compiler/implem...
lib/compiler/implementation/universe.dart:88: final LibraryElement library;  //
Library is null for non-private selectors.
Extra space before //

[kasperl, 2012-08-16 10:35:47]

PTAL. I had to do some changes to the compiler mocking used by the resolver
tests and the type checker tests.

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

https://chromiumcodereview.appspot.com/10854158/diff/3009/lib/compiler/implem...
lib/compiler/implementation/enqueue.dart:327: do {
On 2012/08/15 14:14:04, floitsch wrote:
> remove the inner loop.

Done.

https://chromiumcodereview.appspot.com/10854158/diff/3009/lib/compiler/implem...
lib/compiler/implementation/enqueue.dart:327: do {
On 2012/08/15 14:14:04, floitsch wrote:
> remove the inner loop.

Done.

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

https://chromiumcodereview.appspot.com/10854158/diff/3009/lib/compiler/implem...
lib/compiler/implementation/resolver.dart:968: return element;
On 2012/08/15 14:32:21, ahe wrote:
> These two lines can be merged.
> And yay!

Done.

https://chromiumcodereview.appspot.com/10854158/diff/3009/lib/compiler/implem...
File lib/compiler/implementation/ssa/codegen.dart (right):

https://chromiumcodereview.appspot.com/10854158/diff/3009/lib/compiler/implem...
lib/compiler/implementation/ssa/codegen.dart:1478: // TODO(floitsch): we should
have a separate list for closure invocations.
On 2012/08/15 14:14:04, floitsch wrote:
> remove comment.

Done.

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

https://chromiumcodereview.appspot.com/10854158/diff/3009/lib/compiler/implem...
lib/compiler/implementation/universe.dart:88: final LibraryElement library;  //
Library is null for non-private selectors.
On 2012/08/15 14:32:21, ahe wrote:
> Extra space before //

Done.

[floitsch, 2012-08-16 11:49:53]

Still LGTM.

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

https://chromiumcodereview.appspot.com/10854158/diff/3022/lib/compiler/implem...
lib/compiler/implementation/resolver.dart:967: return mapping[node] = element;
I would prefer the assignment in a separate line.

[ahe, 2012-08-16 12:09:54]

LGTM!

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

https://chromiumcodereview.appspot.com/10854158/diff/3022/lib/compiler/implem...
lib/compiler/implementation/enqueue.dart:327: f(queue.removeLast()); //
TODO(kasperl): Why isn't this removeFirst?
Main is added last and we want to process it first. LIFO (stack) behavior makes
the tests complete faster, and the user gets error messages faster for his code
(rather than waiting fo system classes).

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

https://chromiumcodereview.appspot.com/10854158/diff/3022/lib/compiler/implem...
lib/compiler/implementation/resolver.dart:1248: if (node.isOperator) {
You could write something like this:

if (node.isOperatorWithTypeExpression) {
  resolveTypeTest(node.arguments.head);
} else {
  resolveArguments(node.argumentsNode);
}

You could also change resolveTypeTest to resolveTypeExpression and have it take
node.argumentsNode.

https://chromiumcodereview.appspot.com/10854158/diff/3022/lib/compiler/implem...
lib/compiler/implementation/resolver.dart:1312: bool isCompound =
identifier.source.stringValue !== '=';
isCompound is not entirely correct as this also is true for ++ and --. Perhaps
the word "complex" is better?

https://chromiumcodereview.appspot.com/10854158/diff/3022/lib/compiler/implem...
lib/compiler/implementation/resolver.dart:1320: // register here. We should
consider using some notation of
Great idea.

https://chromiumcodereview.appspot.com/10854158/diff/3022/lib/compiler/implem...
lib/compiler/implementation/resolver.dart:1343: // the use of classes. Wouldn't
it be simpler if we just did?
I'm not so sure about that. There is a difference between using a class for
instantiation, or using a class for accessing a static.