Implement ClosureMirror.

Implement ClosureMirror.

R=kasperl@google.com

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

[ahe, 2013-05-23 16:00:46]

Now 5 out of 10 tests are passing in tests/lib/mirrors/mirrors_test.dart.

[kasperl, 2013-05-24 06:46:01]

LGTM!

https://codereview.chromium.org/15895002/diff/1/dart/sdk/lib/_internal/compil...
File dart/sdk/lib/_internal/compiler/implementation/lib/mirrors_patch.dart
(right):

https://codereview.chromium.org/15895002/diff/1/dart/sdk/lib/_internal/compil...
dart/sdk/lib/_internal/compiler/implementation/lib/mirrors_patch.dart:277: var
extractCallName = JS('', r'''
How does this deal with optional arguments to the closures (there can be many
call$<n> functions)? Do we need test coverage for that? At least add a TODO.

https://codereview.chromium.org/15895002/diff/1/dart/sdk/lib/_internal/compil...
dart/sdk/lib/_internal/compiler/implementation/lib/mirrors_patch.dart:303: new
Future<InstanceMirror>(
Put new Future<...> on the same line as the return?

https://codereview.chromium.org/15895002/diff/1/dart/tests/lib/mirrors/mirror...
File dart/tests/lib/mirrors/mirrors_test.dart (right):

https://codereview.chromium.org/15895002/diff/1/dart/tests/lib/mirrors/mirror...
dart/tests/lib/mirrors/mirrors_test.dart:107: testClosureMirrors(mirrors) {
Add a TODO here to add tests for optional parameters (positional and named
ones).

https://codereview.chromium.org/15895002/diff/1/dart/tests/lib/mirrors/mirror...
dart/tests/lib/mirrors/mirrors_test.dart:119: var future = mirror.applyAsync([2,
4, 8]);
Should this be rewritten (not in this CL) to use apply rather than applyAsync?
Or should we have both?

[ahe, 2013-05-24 06:56:55]

https://codereview.chromium.org/15895002/diff/1/dart/tests/lib/mirrors/mirror...
File dart/tests/lib/mirrors/mirrors_test.dart (right):

https://codereview.chromium.org/15895002/diff/1/dart/tests/lib/mirrors/mirror...
dart/tests/lib/mirrors/mirrors_test.dart:119: var future = mirror.applyAsync([2,
4, 8]);
On 2013/05/24 06:46:01, kasperl wrote:
> Should this be rewritten (not in this CL) to use apply rather than applyAsync?
> Or should we have both?

Not sure if you're suggesting a change to dart:mirrors. If we have applyAsync,
this seems like a great place to test it.

[ahe, 2013-05-24 11:57:37]

Thank you, Kasper!

https://codereview.chromium.org/15895002/diff/1/dart/sdk/lib/_internal/compil...
File dart/sdk/lib/_internal/compiler/implementation/lib/mirrors_patch.dart
(right):

https://codereview.chromium.org/15895002/diff/1/dart/sdk/lib/_internal/compil...
dart/sdk/lib/_internal/compiler/implementation/lib/mirrors_patch.dart:277: var
extractCallName = JS('', r'''
On 2013/05/24 06:46:01, kasperl wrote:
> How does this deal with optional arguments to the closures (there can be many
> call$<n> functions)? Do we need test coverage for that? At least add a TODO.

Done.

https://codereview.chromium.org/15895002/diff/1/dart/sdk/lib/_internal/compil...
dart/sdk/lib/_internal/compiler/implementation/lib/mirrors_patch.dart:303: new
Future<InstanceMirror>(
On 2013/05/24 06:46:01, kasperl wrote:
> Put new Future<...> on the same line as the return?

Done.

https://codereview.chromium.org/15895002/diff/1/dart/tests/lib/mirrors/mirror...
File dart/tests/lib/mirrors/mirrors_test.dart (right):

https://codereview.chromium.org/15895002/diff/1/dart/tests/lib/mirrors/mirror...
dart/tests/lib/mirrors/mirrors_test.dart:107: testClosureMirrors(mirrors) {
On 2013/05/24 06:46:01, kasperl wrote:
> Add a TODO here to add tests for optional parameters (positional and named
> ones).

Done.

[ahe, 2013-05-24 12:01:15]

Committed patchset #2 manually as r23135 (presubmit successful).