Introduce typed selectors for getters and setters also for better tree shaking.

Introduce typed selectors for getters and setters also for better tree shaking.
FYI: Swarm from 862465 to 848387 (14k saved).
Committed: https://code.google.com/p/dart/source/detail?r=6997

[kasperl, 2012-04-25 13:38:06]

First round of comments:

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

https://chromiumcodereview.appspot.com/10191016/diff/2001/lib/compiler/implem...
lib/compiler/implementation/universe.dart:37: bool hasOneMatchingSelector(
These are all "at least one", right? Maybe just remove "One" from the name?

https://chromiumcodereview.appspot.com/10191016/diff/2001/lib/compiler/implem...
lib/compiler/implementation/universe.dart:42: };
Remove ;

https://chromiumcodereview.appspot.com/10191016/diff/2001/lib/compiler/implem...
lib/compiler/implementation/universe.dart:83: const Selector.cons(this.kind,
this.argumentCount);
What does cons mean? Rename to constant?

https://chromiumcodereview.appspot.com/10191016/diff/2001/lib/compiler/implem...
lib/compiler/implementation/universe.dart:103: static final Selector GETTER =
const Selector.cons(SelectorKind.GETTER, 0);
I would probably break the line before const here too just to make the "flow"
more consistent.

https://chromiumcodereview.appspot.com/10191016/diff/2001/lib/compiler/implem...
lib/compiler/implementation/universe.dart:130: || kind == SelectorKind.SETTER;
===?

[ngeoffray, 2012-04-25 13:58:44]

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

https://chromiumcodereview.appspot.com/10191016/diff/2001/lib/compiler/implem...
lib/compiler/implementation/universe.dart:37: bool hasOneMatchingSelector(
On 2012/04/25 13:38:06, kasperl wrote:
> These are all "at least one", right? Maybe just remove "One" from the name?

Done.

https://chromiumcodereview.appspot.com/10191016/diff/2001/lib/compiler/implem...
lib/compiler/implementation/universe.dart:42: };
On 2012/04/25 13:38:06, kasperl wrote:
> Remove ;

Done.

https://chromiumcodereview.appspot.com/10191016/diff/2001/lib/compiler/implem...
lib/compiler/implementation/universe.dart:83: const Selector.cons(this.kind,
this.argumentCount);
On 2012/04/25 13:38:06, kasperl wrote:
> What does cons mean? Rename to constant?

Done.

https://chromiumcodereview.appspot.com/10191016/diff/2001/lib/compiler/implem...
lib/compiler/implementation/universe.dart:103: static final Selector GETTER =
const Selector.cons(SelectorKind.GETTER, 0);
On 2012/04/25 13:38:06, kasperl wrote:
> I would probably break the line before const here too just to make the "flow"
> more consistent.

Done.

https://chromiumcodereview.appspot.com/10191016/diff/2001/lib/compiler/implem...
lib/compiler/implementation/universe.dart:130: || kind == SelectorKind.SETTER;
On 2012/04/25 13:38:06, kasperl wrote:
> ===?

Done.

[kasperl, 2012-04-25 14:14:54]

LGTM.

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

https://chromiumcodereview.appspot.com/10191016/diff/2002/lib/compiler/implem...
lib/compiler/implementation/enqueue.dart:71: 
Why some many newlines?

https://chromiumcodereview.appspot.com/10191016/diff/2002/lib/compiler/implem...
lib/compiler/implementation/enqueue.dart:133: Map<SourceString, Set<Selector>>
invokedGetters =
This shares a ton of code with registerSetter and registerInvocation. Refactor
ala:

  registerInvocation(...) => 
     registerSelector(compiler.universe.invokedNames,
                      methodName,
                      selector);

Why do you pass both a method name and the selector? Somehow it smells like a
selector should have a name.

https://chromiumcodereview.appspot.com/10191016/diff/2002/lib/compiler/implem...
lib/compiler/implementation/enqueue.dart:134: compiler.universe.invokedGetters;
More indentation.

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

https://chromiumcodereview.appspot.com/10191016/diff/2002/lib/compiler/implem...
lib/compiler/implementation/universe.dart:38: Set<Selector> selectors, Element
member, Compiler compiler) {
Move first argument to previous line and split after each argument?

https://chromiumcodereview.appspot.com/10191016/diff/2002/lib/compiler/implem...
lib/compiler/implementation/universe.dart:314: 
I'd remove this newline.

[ngeoffray, 2012-04-26 09:31:51]

Thanks Kasper

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

https://chromiumcodereview.appspot.com/10191016/diff/2002/lib/compiler/implem...
lib/compiler/implementation/enqueue.dart:71: 
On 2012/04/25 14:14:54, kasperl wrote:
> Why some many newlines?

Done.

https://chromiumcodereview.appspot.com/10191016/diff/2002/lib/compiler/implem...
lib/compiler/implementation/enqueue.dart:133: Map<SourceString, Set<Selector>>
invokedGetters =
On 2012/04/25 14:14:54, kasperl wrote:
> This shares a ton of code with registerSetter and registerInvocation. Refactor
> ala:
> 
>   registerInvocation(...) => 
>      registerSelector(compiler.universe.invokedNames,
>                       methodName,
>                       selector);
> 
> Why do you pass both a method name and the selector? Somehow it smells like a
> selector should have a name.

Done.

https://chromiumcodereview.appspot.com/10191016/diff/2002/lib/compiler/implem...
lib/compiler/implementation/enqueue.dart:134: compiler.universe.invokedGetters;
On 2012/04/25 14:14:54, kasperl wrote:
> More indentation.

Done.

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

https://chromiumcodereview.appspot.com/10191016/diff/2002/lib/compiler/implem...
lib/compiler/implementation/universe.dart:38: Set<Selector> selectors, Element
member, Compiler compiler) {
On 2012/04/25 14:14:54, kasperl wrote:
> Move first argument to previous line and split after each argument?

Done.

https://chromiumcodereview.appspot.com/10191016/diff/2002/lib/compiler/implem...
lib/compiler/implementation/universe.dart:314: 
On 2012/04/25 14:14:54, kasperl wrote:
> I'd remove this newline.

Done.