Make TypedefMirror a direct descendant of TypeMirror.

Make TypedefMirror a direct descendant of TypeMirror.

BUG=http://dartbug.com/11616
R=ahe@google.com, asiva@google.com

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

[rmacnak, 2013-09-05 23:01:17]

Just enough changes in dart2js to keep tests passing.

API agreed to in https://chromiumcodereview.appspot.com/23490019/

[Michael Lippautz (Google), 2013-09-06 16:58:40]

DBC

https://codereview.chromium.org/24005002/diff/1/runtime/lib/mirrors_impl.dart
File runtime/lib/mirrors_impl.dart (right):

https://codereview.chromium.org/24005002/diff/1/runtime/lib/mirrors_impl.dart...
runtime/lib/mirrors_impl.dart:994: _types = _filterMap(members,
weird alignment

https://codereview.chromium.org/24005002/diff/1/runtime/lib/mirrors_impl.dart...
runtime/lib/mirrors_impl.dart:1127: _returnType =
fits on prev line?

https://codereview.chromium.org/24005002/diff/1/runtime/lib/mirrors_impl.dart...
runtime/lib/mirrors_impl.dart:1285: _type =
ditto

[rmacnak, 2013-09-09 16:51:20]

On 2013/09/06 16:58:40, Michael Lippautz wrote:
> DBC
> 
> https://codereview.chromium.org/24005002/diff/1/runtime/lib/mirrors_impl.dart
> File runtime/lib/mirrors_impl.dart (right):
> 
>
https://codereview.chromium.org/24005002/diff/1/runtime/lib/mirrors_impl.dart...
> runtime/lib/mirrors_impl.dart:994: _types = _filterMap(members,
> weird alignment
> 
>
https://codereview.chromium.org/24005002/diff/1/runtime/lib/mirrors_impl.dart...
> runtime/lib/mirrors_impl.dart:1127: _returnType =
> fits on prev line?
> 
>
https://codereview.chromium.org/24005002/diff/1/runtime/lib/mirrors_impl.dart...
> runtime/lib/mirrors_impl.dart:1285: _type =
> ditto

Fixed whitespace.

[rmacnak, 2013-09-10 20:56:35]

Fixed outstanding typo in dartdoc.

[rmacnak, 2013-09-12 22:40:30]

Rebased.

[rmacnak, 2013-09-16 20:20:35]

On 2013/09/12 22:40:30, Ryan Macnak wrote:
> Rebased.

Ping.

[rmacnak, 2013-09-19 18:38:39]

On 2013/09/16 20:20:35, Ryan Macnak wrote:
> On 2013/09/12 22:40:30, Ryan Macnak wrote:
> > Rebased.
> 
> Ping.

Ping. This is the CL that adds reflectType.

[rmacnak, 2013-09-24 17:10:17]

Ping.

[rmacnak, 2013-10-02 20:08:29]

-Michael +Gilad

Resynced.

[gbracha, 2013-10-02 22:49:53]

Tests and API look good to me, modulo 2 comments.

https://codereview.chromium.org/24005002/diff/27001/sdk/lib/mirrors/mirrors.dart
File sdk/lib/mirrors/mirrors.dart (right):

https://codereview.chromium.org/24005002/diff/27001/sdk/lib/mirrors/mirrors.d...
sdk/lib/mirrors/mirrors.dart:470: Future<InstanceMirror> getFieldAsync(Symbol
fieldName);
Didn't we get rid of getFieldAsync?

https://codereview.chromium.org/24005002/diff/27001/tests/lib/mirrors/mirrors...
File tests/lib/mirrors/mirrors_test.dart (right):

https://codereview.chromium.org/24005002/diff/27001/tests/lib/mirrors/mirrors...
tests/lib/mirrors/mirrors_test.dart:245: equals(const
Symbol('MirrorsTest.Class.T')));
Can we use literal symbols here?

https://codereview.chromium.org/24005002/diff/27001/tests/lib/mirrors/typevar...
File tests/lib/mirrors/typevariable_mirror_metadata_test.dart (right):

https://codereview.chromium.org/24005002/diff/27001/tests/lib/mirrors/typevar...
tests/lib/mirrors/typevariable_mirror_metadata_test.dart:47:
checkMetadata(ftm.typeVariables[0], [m1, m2]);
The referent of Predicate is a function type G-> bool which has no type
variables,

[rmacnak, 2013-10-03 17:24:47]

https://codereview.chromium.org/24005002/diff/27001/sdk/lib/mirrors/mirrors.dart
File sdk/lib/mirrors/mirrors.dart (right):

https://codereview.chromium.org/24005002/diff/27001/sdk/lib/mirrors/mirrors.d...
sdk/lib/mirrors/mirrors.dart:470: Future<InstanceMirror> getFieldAsync(Symbol
fieldName);
On 2013/10/02 22:49:53, gbracha wrote:
> Didn't we get rid of getFieldAsync?

In another CL awaiting review.

https://codereview.chromium.org/24005002/diff/27001/tests/lib/mirrors/mirrors...
File tests/lib/mirrors/mirrors_test.dart (right):

https://codereview.chromium.org/24005002/diff/27001/tests/lib/mirrors/mirrors...
tests/lib/mirrors/mirrors_test.dart:245: equals(const
Symbol('MirrorsTest.Class.T')));
On 2013/10/02 22:49:53, gbracha wrote:
> Can we use literal symbols here?

Yes, but I don't want to do that in this CL because of the merge conflicts it
will create with the private symbols CL.

https://codereview.chromium.org/24005002/diff/27001/tests/lib/mirrors/typevar...
File tests/lib/mirrors/typevariable_mirror_metadata_test.dart (right):

https://codereview.chromium.org/24005002/diff/27001/tests/lib/mirrors/typevar...
tests/lib/mirrors/typevariable_mirror_metadata_test.dart:47:
checkMetadata(ftm.typeVariables[0], [m1, m2]);
On 2013/10/02 22:49:53, gbracha wrote:
> The referent of Predicate is a function type G-> bool which has no type
> variables,

Done. Strange this was working in the VM...

[rmacnak, 2013-10-09 01:31:36]

Fixing mirrors/generic_function_typedef_test / Issue 12282 is blocked by this
CL.

[ahe, 2013-10-11 10:30:20]

LGTM!

https://chromiumcodereview.appspot.com/24005002/diff/52001/sdk/lib/mirrors/mi...
File sdk/lib/mirrors/mirrors.dart (right):

https://chromiumcodereview.appspot.com/24005002/diff/52001/sdk/lib/mirrors/mi...
sdk/lib/mirrors/mirrors.dart:675: Map<Symbol, TypeMirror> get types;
This is temporary, right?

[ahe, 2013-10-11 10:30:51]

I only reviewed the dart2js, make sure get someone to look at the runtime
changes.

[rmacnak, 2013-10-11 18:06:08]

+Siva for runtime changes

Synced with generics substitution changes.

https://chromiumcodereview.appspot.com/24005002/diff/52001/sdk/lib/mirrors/mi...
File sdk/lib/mirrors/mirrors.dart (right):

https://chromiumcodereview.appspot.com/24005002/diff/52001/sdk/lib/mirrors/mi...
sdk/lib/mirrors/mirrors.dart:675: Map<Symbol, TypeMirror> get types;
On 2013/10/11 10:30:21, ahe wrote:
> This is temporary, right?

Right, this should also be subsumed by declarations.

[rmacnak, 2013-10-16 17:37:43]

Resynced with the new tests.

[gbracha, 2013-10-16 17:51:25]

tests still look good, minor comments

https://codereview.chromium.org/24005002/diff/74001/tests/lib/mirrors/mirrors...
File tests/lib/mirrors/mirrors_test.dart (right):

https://codereview.chromium.org/24005002/diff/74001/tests/lib/mirrors/mirrors...
tests/lib/mirrors/mirrors_test.dart:243: expect(typeVariable.simpleName,
equals(const Symbol('T')));
Can we just use #T (and #MirrorsTest.Class.T below, if that works)?

[rmacnak, 2013-10-16 23:50:24]

https://codereview.chromium.org/24005002/diff/74001/tests/lib/mirrors/mirrors...
File tests/lib/mirrors/mirrors_test.dart (right):

https://codereview.chromium.org/24005002/diff/74001/tests/lib/mirrors/mirrors...
tests/lib/mirrors/mirrors_test.dart:243: expect(typeVariable.simpleName,
equals(const Symbol('T')));
On 2013/10/16 17:51:26, gbracha wrote:
> Can we just use #T (and #MirrorsTest.Class.T below, if that works)?

Done.

[siva, 2013-10-18 16:44:14]

lgtm

[rmacnak, 2013-10-18 17:00:11]

Committed patchset #15 manually as r28862 (presubmit successful).