Associate the correct type to method receivers (instead of Dynamic type).

Associate the correct type to method receivers (instead of Dynamic type).
Properly name the first implicit parameter to factories as ':type_arguments'
(instead of 'this', which made it accessible to user code).
Various cleanup.
Committed: https://code.google.com/p/dart/source/detail?r=10203

[srdjan, 2012-08-02 22:29:46]

LGTM

https://chromiumcodereview.appspot.com/10831142/diff/1/runtime/vm/ast.h
File runtime/vm/ast.h (right):

https://chromiumcodereview.appspot.com/10831142/diff/1/runtime/vm/ast.h#newco...
runtime/vm/ast.h:933: ASSERT(&local_ != NULL);
That can't be, can it? You can't pass *NULL to it?
Ditto for cases below.

[regis, 2012-08-02 23:01:26]

Thanks

https://chromiumcodereview.appspot.com/10831142/diff/1/runtime/vm/ast.h
File runtime/vm/ast.h (right):

https://chromiumcodereview.appspot.com/10831142/diff/1/runtime/vm/ast.h#newco...
runtime/vm/ast.h:933: ASSERT(&local_ != NULL);
On 2012/08/02 22:29:46, srdjan wrote:
> That can't be, can it? You can't pass *NULL to it?
> Ditto for cases below.

Actually, yes, you can pass *receiver with receiver == NULL. It just happened to
me while debugging this cl. It's why I added the assert.

[hausner, 2012-08-02 23:25:00]

LGTM but I am worried that you allocate a new type object each time the receiver
gets added to a parameter list.

https://chromiumcodereview.appspot.com/10831142/diff/1/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://chromiumcodereview.appspot.com/10831142/diff/1/runtime/vm/object.cc#n...
runtime/vm/object.cc:3704: void Function::set_owner(const Class& value) const {
Make this accessor private?

https://chromiumcodereview.appspot.com/10831142/diff/1/runtime/vm/object.h
File runtime/vm/object.h (right):

https://chromiumcodereview.appspot.com/10831142/diff/1/runtime/vm/object.h#ne...
runtime/vm/object.h:1677: void set_owner(const Class& value) const {
Make this setter private?

https://chromiumcodereview.appspot.com/10831142/diff/1/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

https://chromiumcodereview.appspot.com/10831142/diff/1/runtime/vm/parser.cc#n...
runtime/vm/parser.cc:430: AddFinalParameter(receiver_type->token_pos(),
Why are you using the token position of the type, not the token position that is
passed in (typically the call position)?

https://chromiumcodereview.appspot.com/10831142/diff/1/runtime/vm/parser.cc#n...
runtime/vm/parser.cc:7236: Type::New(current_class(), type_arguments,
type_pos));
I don't understand why you have to allocate a new type object each time you add
a receiver to an argument list. Can they not all share the same type object?

[regis, 2012-08-02 23:58:37]

Thanks Matthias.

Since I submitted already, I will address your comments in a following cl.

I will also change references to pointers in the ast, since people got shocked
by the ASSERTs introduced in this cl.

Indeed, C++ let you pass *receiver with receiver == NULL to a formal parameter
of a reference type const LocalVariable& local. Then, &local == NULL.

https://chromiumcodereview.appspot.com/10831142/diff/1/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://chromiumcodereview.appspot.com/10831142/diff/1/runtime/vm/object.cc#n...
runtime/vm/object.cc:3704: void Function::set_owner(const Class& value) const {
On 2012/08/02 23:25:00, hausner wrote:
> Make this accessor private?

Good point. Done for Function and for Field.

https://chromiumcodereview.appspot.com/10831142/diff/1/runtime/vm/object.h
File runtime/vm/object.h (right):

https://chromiumcodereview.appspot.com/10831142/diff/1/runtime/vm/object.h#ne...
runtime/vm/object.h:1677: void set_owner(const Class& value) const {
On 2012/08/02 23:25:00, hausner wrote:
> Make this setter private?

Done.

https://chromiumcodereview.appspot.com/10831142/diff/1/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

https://chromiumcodereview.appspot.com/10831142/diff/1/runtime/vm/parser.cc#n...
runtime/vm/parser.cc:430: AddFinalParameter(receiver_type->token_pos(),
On 2012/08/02 23:25:00, hausner wrote:
> Why are you using the token position of the type, not the token position that
is
> passed in (typically the call position)?

The token position is now passed to ReceiverType to build the type, and this
same position is then used here.

Well, that was the idea, but your comment made me realize that the type may get
canonicalized, in which case the token position gets lost.
I'll keep the name_pos as a separate parameter.

https://chromiumcodereview.appspot.com/10831142/diff/1/runtime/vm/parser.cc#n...
runtime/vm/parser.cc:7236: Type::New(current_class(), type_arguments,
type_pos));
On 2012/08/02 23:25:00, hausner wrote:
> I don't understand why you have to allocate a new type object each time you
add
> a receiver to an argument list. Can they not all share the same type object?

We add this type to the formal parameter list, not to the argument list. So
there are not that many. Also, these types get canonicalized, so they get shared
anyway.