Make ClassMirror.typeVariables lazy and convert dependencies to native code.

BUG=

R=asiva@google.com, rmacnak@google.com

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

[Michael Lippautz (Google), 2013-07-25 20:17:57]

Inlined code for TypeVariableMirrors.

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

https://codereview.chromium.org/20216002/diff/3001/runtime/lib/mirrors_impl.d...
runtime/lib/mirrors_impl.dart:533: Map<Symbol, TypeVariableMirror> get
typeVariables {
Future API may return a List here. We already get a list from native code, but
convert it to a Map (not changing behavior) for now.

[siva, 2013-07-25 22:48:43]

https://codereview.chromium.org/20216002/diff/3001/runtime/lib/mirrors.cc
File runtime/lib/mirrors.cc (right):

https://codereview.chromium.org/20216002/diff/3001/runtime/lib/mirrors.cc#new...
runtime/lib/mirrors.cc:384: }
As discussed offline, let us investigate if we could return the type_parameters
array back to dart and run the creation of TypeVariables directly in dart
populating the Map as we go.

[Michael Lippautz (Google), 2013-07-26 19:59:15]

Moved actual mirror creation to Dart land, to avoid jumping back and forth
between native and Dart.

[siva, 2013-07-26 22:10:34]

https://codereview.chromium.org/20216002/diff/10001/runtime/lib/mirrors.cc
File runtime/lib/mirrors.cc (right):

https://codereview.chromium.org/20216002/diff/10001/runtime/lib/mirrors.cc#ne...
runtime/lib/mirrors.cc:37: #endif  // DEBUG
Not sure I understand why ThrowInvokeError should be called only in debug mode?

Would it be sufficient to have
ASSERT(!result.IsError());

https://codereview.chromium.org/20216002/diff/10001/runtime/lib/mirrors.cc#ne...
runtime/lib/mirrors.cc:377: // map then.
"let Dart code create the type mirror object and the ordered map."

https://codereview.chromium.org/20216002/diff/10001/runtime/lib/mirrors.cc#ne...
runtime/lib/mirrors.cc:379: ASSERT(cls.IsClass());
The function prototype states it is Class handle so why do you need to assert
again?

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

https://codereview.chromium.org/20216002/diff/10001/runtime/lib/mirrors_impl....
runtime/lib/mirrors_impl.dart:539: new _LocalTypeVariableMirrorImpl(params[i +
1], params[0], this);
params[i] instead of params[0]
(looks like we are missing a test for this).

Also you could do this as

var mirror_obj = new _LocalTypeVariableMirrorImpl(params[i + 1], params[0],
this);
_typeVariables[mirror_obj.simpleName] = mirror_obj;

[Michael Lippautz (Google), 2013-07-26 23:30:07]

Also fixed handling the case with no type parameters.

---

Peter: I added a test checking ClassMirror.typeParameters (still following the
api where it returns a Map). Since it's not yet implemented in dart2js I created
a tracking bug (#12087) and excluded it from dart2js. I hope you don't mind that
I assigned it directly to you.

https://codereview.chromium.org/20216002/diff/10001/runtime/lib/mirrors.cc
File runtime/lib/mirrors.cc (right):

https://codereview.chromium.org/20216002/diff/10001/runtime/lib/mirrors.cc#ne...
runtime/lib/mirrors.cc:37: #endif  // DEBUG
On 2013/07/26 22:10:34, siva wrote:
> Not sure I understand why ThrowInvokeError should be called only in debug
mode?
> 
> Would it be sufficient to have
> ASSERT(!result.IsError());

Done.

https://codereview.chromium.org/20216002/diff/10001/runtime/lib/mirrors.cc#ne...
runtime/lib/mirrors.cc:377: // map then.
On 2013/07/26 22:10:34, siva wrote:
> "let Dart code create the type mirror object and the ordered map."

Done.

https://codereview.chromium.org/20216002/diff/10001/runtime/lib/mirrors.cc#ne...
runtime/lib/mirrors.cc:379: ASSERT(cls.IsClass());
On 2013/07/26 22:10:34, siva wrote:
> The function prototype states it is Class handle so why do you need to assert
> again?

Removed.

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

https://codereview.chromium.org/20216002/diff/10001/runtime/lib/mirrors_impl....
runtime/lib/mirrors_impl.dart:539: new _LocalTypeVariableMirrorImpl(params[i +
1], params[0], this);
On 2013/07/26 22:10:34, siva wrote:
> params[i] instead of params[0]
> (looks like we are missing a test for this).
> 
> Also you could do this as
> 
> var mirror_obj = new _LocalTypeVariableMirrorImpl(params[i + 1], params[0],
> this);
> _typeVariables[mirror_obj.simpleName] = mirror_obj;

Done. I also added some tests do cover these things.

[rmacnak, 2013-07-30 20:20:23]

https://codereview.chromium.org/20216002/diff/25001/runtime/lib/mirrors.cc
File runtime/lib/mirrors.cc (right):

https://codereview.chromium.org/20216002/diff/25001/runtime/lib/mirrors.cc#ne...
runtime/lib/mirrors.cc:61: static Dart_Handle CoreLib() {
Dead code.

https://codereview.chromium.org/20216002/diff/25001/runtime/lib/mirrors.cc#ne...
runtime/lib/mirrors.cc:67: static Dart_Handle MapNew() {
Dead code.

https://codereview.chromium.org/20216002/diff/25001/runtime/lib/mirrors.cc#ne...
runtime/lib/mirrors.cc:77: static Dart_Handle MapAdd(Dart_Handle map,
Dart_Handle key, Dart_Handle value) {
Dead code.

https://codereview.chromium.org/20216002/diff/25001/runtime/lib/mirrors.cc#ne...
runtime/lib/mirrors.cc:210: TypeArguments& args =
TypeArguments::Handle(cls.type_parameters());
const

https://codereview.chromium.org/20216002/diff/25001/runtime/lib/mirrors.cc#ne...
runtime/lib/mirrors.cc:270: Dart_Handle intf_mirror = CreateLazyMirror(intf);
Dead code.

[Michael Lippautz (Google), 2013-07-30 20:26:46]

https://codereview.chromium.org/20216002/diff/25001/runtime/lib/mirrors.cc
File runtime/lib/mirrors.cc (right):

https://codereview.chromium.org/20216002/diff/25001/runtime/lib/mirrors.cc#ne...
runtime/lib/mirrors.cc:61: static Dart_Handle CoreLib() {
On 2013/07/30 20:20:23, Ryan Macnak wrote:
> Dead code.

Done.

https://codereview.chromium.org/20216002/diff/25001/runtime/lib/mirrors.cc#ne...
runtime/lib/mirrors.cc:67: static Dart_Handle MapNew() {
On 2013/07/30 20:20:23, Ryan Macnak wrote:
> Dead code.

Done.

https://codereview.chromium.org/20216002/diff/25001/runtime/lib/mirrors.cc#ne...
runtime/lib/mirrors.cc:77: static Dart_Handle MapAdd(Dart_Handle map,
Dart_Handle key, Dart_Handle value) {
On 2013/07/30 20:20:23, Ryan Macnak wrote:
> Dead code.

Done.

https://codereview.chromium.org/20216002/diff/25001/runtime/lib/mirrors.cc#ne...
runtime/lib/mirrors.cc:210: TypeArguments& args =
TypeArguments::Handle(cls.type_parameters());
On 2013/07/30 20:20:23, Ryan Macnak wrote:
> const

Done.

https://codereview.chromium.org/20216002/diff/25001/runtime/lib/mirrors.cc#ne...
runtime/lib/mirrors.cc:270: Dart_Handle intf_mirror = CreateLazyMirror(intf);
On 2013/07/30 20:20:23, Ryan Macnak wrote:
> Dead code.

Done.

[rmacnak, 2013-07-30 20:36:02]

lgtm

[siva, 2013-07-30 20:47:27]

lgtm

[Michael Lippautz (Google), 2013-07-30 22:58:07]

Committed patchset #9 manually as r25651 (presubmit successful).

[ahe, 2013-08-06 14:18:23]

lgtm