Address Regis' comments, replace AbstractType with Type where possible.

Address Regis' comments, replace AbstractType with Type where possible.
Committed: https://code.google.com/p/dart/source/detail?r=1992

[regis, 2011-12-01 19:20:43]

LGTM with a few comments.

http://codereview.chromium.org/8772009/diff/1009/runtime/vm/class_finalizer.cc
File runtime/vm/class_finalizer.cc (right):

http://codereview.chromium.org/8772009/diff/1009/runtime/vm/class_finalizer.c...
runtime/vm/class_finalizer.cc:301: if (super_type.IsTypeParameter()) {
Since super_type is now a Type, this test always fails.
This test can be safely removed, because we do not resolve types to
TypeParameter in the class finalizer anymore. See the TODO at function
ResolveType().

http://codereview.chromium.org/8772009/diff/1009/runtime/vm/class_finalizer.c...
runtime/vm/class_finalizer.cc:691: RawAbstractType*
ClassFinalizer::FinalizeAndCanonicalizeType(
I think this should always return a RawType*.
However, ClassFinalizer::FinalizeType() must still return a RawAbstractType*,
because it can be called on TypeParameter, which is then not canonicalized.

http://codereview.chromium.org/8772009/diff/1009/runtime/vm/object.cc
File runtime/vm/object.cc (right):

http://codereview.chromium.org/8772009/diff/1009/runtime/vm/object.cc#newcode...
runtime/vm/object.cc:2123: RawAbstractType* Type::Canonicalize() const {
I think this should be RawType*

http://codereview.chromium.org/8772009/diff/1009/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

http://codereview.chromium.org/8772009/diff/1009/runtime/vm/parser.cc#newcode879
runtime/vm/parser.cc:879: signature_type ^=
^= not necessary if you change ClassFinalizer::FinalizeAndCanonicalizeType to
return a type

http://codereview.chromium.org/8772009/diff/1009/runtime/vm/parser.cc#newcode...
runtime/vm/parser.cc:3739: signature_type ^=
ditto

http://codereview.chromium.org/8772009/diff/1009/runtime/vm/parser.cc#newcode...
runtime/vm/parser.cc:6580: type ^=
ClassFinalizer::FinalizeAndCanonicalizeType(type, &errmsg);
ditto

http://codereview.chromium.org/8772009/diff/1009/runtime/vm/parser.cc#newcode...
runtime/vm/parser.cc:7122: type ^=
ClassFinalizer::FinalizeAndCanonicalizeType(type, &errmsg);
ditto

http://codereview.chromium.org/8772009/diff/1009/runtime/vm/raw_object.h
File runtime/vm/raw_object.h (right):

http://codereview.chromium.org/8772009/diff/1009/runtime/vm/raw_object.h#newc...
runtime/vm/raw_object.h:225: RawArray* interfaces_;  // Array of AbstractType.
TODO: RawTypeArguments* interfaces_;  // Array of Type.

http://codereview.chromium.org/8772009/diff/1009/runtime/vm/raw_object.h#newc...
runtime/vm/raw_object.h:235: RawArray* canonical_types_;  // Canonicalized types
of this class.
TODO: RawTypeArguments* canonical_types_;  // Canonicalized types of this class.

[srdjan, 2011-12-01 20:04:09]

Thanks!

http://codereview.chromium.org/8772009/diff/1009/runtime/vm/class_finalizer.cc
File runtime/vm/class_finalizer.cc (right):

http://codereview.chromium.org/8772009/diff/1009/runtime/vm/class_finalizer.c...
runtime/vm/class_finalizer.cc:301: if (super_type.IsTypeParameter()) {
On 2011/12/01 19:20:43, regis wrote:
> Since super_type is now a Type, this test always fails.
> This test can be safely removed, because we do not resolve types to
> TypeParameter in the class finalizer anymore. See the TODO at function
> ResolveType().

Removed code.
I think it is better if I implement the TODO for ResolveType, and remove the
result.

http://codereview.chromium.org/8772009/diff/1009/runtime/vm/class_finalizer.c...
runtime/vm/class_finalizer.cc:691: RawAbstractType*
ClassFinalizer::FinalizeAndCanonicalizeType(
On 2011/12/01 19:20:43, regis wrote:
> I think this should always return a RawType*.
> However, ClassFinalizer::FinalizeType() must still return a RawAbstractType*,
> because it can be called on TypeParameter, which is then not canonicalized.


Done.

http://codereview.chromium.org/8772009/diff/1009/runtime/vm/object.cc
File runtime/vm/object.cc (right):

http://codereview.chromium.org/8772009/diff/1009/runtime/vm/object.cc#newcode...
runtime/vm/object.cc:2123: RawAbstractType* Type::Canonicalize() const {
On 2011/12/01 19:20:43, regis wrote:
> I think this should be RawType*

This is a virtual, implemented also in TypeParameter, i.e., the return type must
be the RawAbstractType

http://codereview.chromium.org/8772009/diff/1009/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

http://codereview.chromium.org/8772009/diff/1009/runtime/vm/parser.cc#newcode879
runtime/vm/parser.cc:879: signature_type ^=
On 2011/12/01 19:20:43, regis wrote:
> ^= not necessary if you change ClassFinalizer::FinalizeAndCanonicalizeType to
> return a type

Done.

http://codereview.chromium.org/8772009/diff/1009/runtime/vm/parser.cc#newcode...
runtime/vm/parser.cc:3739: signature_type ^=
On 2011/12/01 19:20:43, regis wrote:
> ditto

Done.

http://codereview.chromium.org/8772009/diff/1009/runtime/vm/parser.cc#newcode...
runtime/vm/parser.cc:6580: type ^=
ClassFinalizer::FinalizeAndCanonicalizeType(type, &errmsg);
On 2011/12/01 19:20:43, regis wrote:
> ditto

Done.

http://codereview.chromium.org/8772009/diff/1009/runtime/vm/parser.cc#newcode...
runtime/vm/parser.cc:7122: type ^=
ClassFinalizer::FinalizeAndCanonicalizeType(type, &errmsg);
On 2011/12/01 19:20:43, regis wrote:
> ditto

Done.

http://codereview.chromium.org/8772009/diff/1009/runtime/vm/raw_object.h
File runtime/vm/raw_object.h (right):

http://codereview.chromium.org/8772009/diff/1009/runtime/vm/raw_object.h#newc...
runtime/vm/raw_object.h:225: RawArray* interfaces_;  // Array of AbstractType.
On 2011/12/01 19:20:43, regis wrote:
> TODO: RawTypeArguments* interfaces_;  // Array of Type.

Done.

http://codereview.chromium.org/8772009/diff/1009/runtime/vm/raw_object.h#newc...
runtime/vm/raw_object.h:235: RawArray* canonical_types_;  // Canonicalized types
of this class.
On 2011/12/01 19:20:43, regis wrote:
> TODO: RawTypeArguments* canonical_types_;  // Canonicalized types of this
class.

Done.

[regis, 2011-12-01 20:53:17]

LGTM

http://codereview.chromium.org/8772009/diff/2009/runtime/vm/class_finalizer.cc
File runtime/vm/class_finalizer.cc (right):

http://codereview.chromium.org/8772009/diff/2009/runtime/vm/class_finalizer.c...
runtime/vm/class_finalizer.cc:301: cls.set_super_type(super_type);
Since we now resolve the type in place, you do not need to write it back after
calling ResolveType.

http://codereview.chromium.org/8772009/diff/2009/runtime/vm/class_finalizer.c...
runtime/vm/class_finalizer.cc:474: arguments.SetTypeAt(i, type_argument);
ditto

http://codereview.chromium.org/8772009/diff/2009/runtime/vm/class_finalizer.c...
runtime/vm/class_finalizer.cc:720: function.set_result_type(type);
ditto

http://codereview.chromium.org/8772009/diff/2009/runtime/vm/class_finalizer.c...
runtime/vm/class_finalizer.cc:754: function.set_result_type(type);
ditto

http://codereview.chromium.org/8772009/diff/2009/runtime/vm/class_finalizer.c...
runtime/vm/class_finalizer.cc:764: function.SetParameterTypeAt(i, type);
ditto

http://codereview.chromium.org/8772009/diff/2009/runtime/vm/class_finalizer.c...
runtime/vm/class_finalizer.cc:821: extends_array.SetTypeAt(i, type_extends);
ditto

http://codereview.chromium.org/8772009/diff/2009/runtime/vm/class_finalizer.c...
runtime/vm/class_finalizer.cc:851: field.set_type(type);
ditto

http://codereview.chromium.org/8772009/diff/2009/runtime/vm/class_finalizer.c...
runtime/vm/class_finalizer.cc:1149: super_interfaces.SetAt(i, interface);
ditto

[srdjan, 2011-12-01 21:04:00]

http://codereview.chromium.org/8772009/diff/2009/runtime/vm/class_finalizer.cc
File runtime/vm/class_finalizer.cc (right):

http://codereview.chromium.org/8772009/diff/2009/runtime/vm/class_finalizer.c...
runtime/vm/class_finalizer.cc:301: cls.set_super_type(super_type);
On 2011/12/01 20:53:17, regis wrote:
> Since we now resolve the type in place, you do not need to write it back after
> calling ResolveType.

Done.

http://codereview.chromium.org/8772009/diff/2009/runtime/vm/class_finalizer.c...
runtime/vm/class_finalizer.cc:474: arguments.SetTypeAt(i, type_argument);
On 2011/12/01 20:53:17, regis wrote:
> ditto

Done.

http://codereview.chromium.org/8772009/diff/2009/runtime/vm/class_finalizer.c...
runtime/vm/class_finalizer.cc:720: function.set_result_type(type);
On 2011/12/01 20:53:17, regis wrote:
> ditto

Done.

http://codereview.chromium.org/8772009/diff/2009/runtime/vm/class_finalizer.c...
runtime/vm/class_finalizer.cc:754: function.set_result_type(type);
On 2011/12/01 20:53:17, regis wrote:
> ditto

Done.

http://codereview.chromium.org/8772009/diff/2009/runtime/vm/class_finalizer.c...
runtime/vm/class_finalizer.cc:764: function.SetParameterTypeAt(i, type);
On 2011/12/01 20:53:17, regis wrote:
> ditto

Done.

http://codereview.chromium.org/8772009/diff/2009/runtime/vm/class_finalizer.c...
runtime/vm/class_finalizer.cc:821: extends_array.SetTypeAt(i, type_extends);
On 2011/12/01 20:53:17, regis wrote:
> ditto

Done.

http://codereview.chromium.org/8772009/diff/2009/runtime/vm/class_finalizer.c...
runtime/vm/class_finalizer.cc:851: field.set_type(type);
On 2011/12/01 20:53:17, regis wrote:
> ditto

Done.

http://codereview.chromium.org/8772009/diff/2009/runtime/vm/class_finalizer.c...
runtime/vm/class_finalizer.cc:1149: super_interfaces.SetAt(i, interface);
On 2011/12/01 20:53:17, regis wrote:
> ditto

Done.