Implement ObjectMirror.[]

Implement ObjectMirror.[] the VM.

R=ahe@google.com

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

[rmacnak, 2013-10-03 00:36:51]

https://codereview.chromium.org/25741005/diff/6001/tests/lib/lib.status
File tests/lib/lib.status (right):

https://codereview.chromium.org/25741005/diff/6001/tests/lib/lib.status#newco...
tests/lib/lib.status:48: mirrors/invoke_subscript_test: RuntimeError # Issue
13766
Several other tests break in checked mode because of the same issue >:|

[gbracha, 2013-10-03 00:58:54]

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

https://codereview.chromium.org/25741005/diff/6001/sdk/lib/mirrors/mirrors.da...
sdk/lib/mirrors/mirrors.dart:570: */
See proposed modification to wording below.  Applies to all 3 versions.

https://codereview.chromium.org/25741005/diff/6001/sdk/lib/mirrors/mirrors.da...
sdk/lib/mirrors/mirrors.dart:961: * with defaults d1, ..., dk.
Otherwise, the result of this method is a closure equivalent to:

[ahe, 2013-10-03 07:26:34]

https://codereview.chromium.org/25741005/diff/6001/sdk/lib/_internal/lib/js_m...
File sdk/lib/_internal/lib/js_mirrors.dart (right):

https://codereview.chromium.org/25741005/diff/6001/sdk/lib/_internal/lib/js_m...
sdk/lib/_internal/lib/js_mirrors.dart:633: noSuchMethod(Invocation msg) {
In no way can we use "noSuchMethod" in implementation libraries.

The reason is that noSuchMethod has global impact on the compiler and negatively
impacts generated code.

Fortunately, I'm currently working on implementing InstanceMirror.getField for
methods. This implementation should do the right thing, also for this use case.

[rmacnak, 2013-10-03 16:58:06]

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

https://codereview.chromium.org/25741005/diff/6001/sdk/lib/mirrors/mirrors.da...
sdk/lib/mirrors/mirrors.dart:570: */
On 2013/10/03 00:58:54, gbracha wrote:
> See proposed modification to wording below.  Applies to all 3 versions.

Done.

[rmacnak, 2013-10-03 16:59:47]

On 2013/10/03 07:26:34, ahe wrote:
>
https://codereview.chromium.org/25741005/diff/6001/sdk/lib/_internal/lib/js_m...
> File sdk/lib/_internal/lib/js_mirrors.dart (right):
> 
>
https://codereview.chromium.org/25741005/diff/6001/sdk/lib/_internal/lib/js_m...
> sdk/lib/_internal/lib/js_mirrors.dart:633: noSuchMethod(Invocation msg) {
> In no way can we use "noSuchMethod" in implementation libraries.
> 
> The reason is that noSuchMethod has global impact on the compiler and
negatively
> impacts generated code.

Forgot about that :(  Removed dart2js implementation.

> Fortunately, I'm currently working on implementing InstanceMirror.getField for
> methods. This implementation should do the right thing, also for this use
case.

We can't answer the ordinary closurized methods here because they don't return
mirrors.

[ahe, 2013-10-29 10:25:23]

LGTM!

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

https://codereview.chromium.org/25741005/diff/16001/sdk/lib/mirrors/mirrors.d...
sdk/lib/mirrors/mirrors.dart:557: *     (r1, .., rn, {p1: d1, ..., pk: dk}) {
Change ".." (two periods) to "..." (three periods), several places.

https://codereview.chromium.org/25741005/diff/16001/sdk/lib/mirrors/mirrors.d...
sdk/lib/mirrors/mirrors.dart:565: *     (r1, .., rn, [p1 = d1, …, pk = dk]) {
Change "…" (a single unicode character) to "..." (three ASCII periods).

https://codereview.chromium.org/25741005/diff/16001/sdk/lib/mirrors/mirrors.d...
sdk/lib/mirrors/mirrors.dart:570: * parameters p1, ..., pk with defaults d1,
..., dk.
What happens if [name] isn't a regular method?

https://codereview.chromium.org/25741005/diff/16001/sdk/lib/mirrors/mirrors.d...
sdk/lib/mirrors/mirrors.dart:573: 
Extra line.

https://codereview.chromium.org/25741005/diff/16001/sdk/lib/mirrors/mirrors.d...
sdk/lib/mirrors/mirrors.dart:724: *     (r1, .., rn, {p1: d1, ..., pk: dk}) {
Change ".." (two periods) to "..." (three periods). Several places.

https://codereview.chromium.org/25741005/diff/16001/sdk/lib/mirrors/mirrors.d...
sdk/lib/mirrors/mirrors.dart:732: *     (r1, .., rn, [p1 = d1, …, pk = dk]) {
Change to three periods.

https://codereview.chromium.org/25741005/diff/16001/sdk/lib/mirrors/mirrors.d...
sdk/lib/mirrors/mirrors.dart:737: * parameters p1, ..., pk with defaults d1,
..., dk.
What happens if [name] isn't a regular method.

https://codereview.chromium.org/25741005/diff/16001/sdk/lib/mirrors/mirrors.d...
sdk/lib/mirrors/mirrors.dart:958: *     (r1, .., rn, {p1: d1, ..., pk: dk}) {
Same period issues as above:
Change ".." to "...", and replace the unicode character.

https://codereview.chromium.org/25741005/diff/16001/sdk/lib/mirrors/mirrors.d...
sdk/lib/mirrors/mirrors.dart:971: * parameters p1, ..., pk with defaults d1,
..., dk.
What happens if [name] is not a regular method.

https://codereview.chromium.org/25741005/diff/16001/tests/lib/mirrors/invoke_...
File tests/lib/mirrors/invoke_subscript_test.dart (right):

https://codereview.chromium.org/25741005/diff/16001/tests/lib/mirrors/invoke_...
tests/lib/mirrors/invoke_subscript_test.dart:39: main() {
Could you add some tests of what happens for im[#missingMethod]?

[rmacnak, 2013-10-29 18:03:38]

Fixed ellipses and clarified the result is null if the method doesn't exist or
isn't a regular method.

https://codereview.chromium.org/25741005/diff/16001/tests/lib/mirrors/invoke_...
File tests/lib/mirrors/invoke_subscript_test.dart (right):

https://codereview.chromium.org/25741005/diff/16001/tests/lib/mirrors/invoke_...
tests/lib/mirrors/invoke_subscript_test.dart:39: main() {
On 2013/10/29 10:25:24, ahe wrote:
> Could you add some tests of what happens for im[#missingMethod]?

Already have im[#doesntExist].

[ahe, 2013-10-29 18:16:04]

On 2013/10/29 18:03:38, Ryan Macnak wrote:
> Fixed ellipses and clarified the result is null if the method doesn't exist or
> isn't a regular method.

Hmm. I wonder if we should extend this further: call noSuchMethod if the method
doesn't exist and return an instance mirror if there is a getter.

So short-term, how about throwing ArgumentError, then we can fancify it later.


>
https://codereview.chromium.org/25741005/diff/16001/tests/lib/mirrors/invoke_...
> File tests/lib/mirrors/invoke_subscript_test.dart (right):
> 
>
https://codereview.chromium.org/25741005/diff/16001/tests/lib/mirrors/invoke_...
> tests/lib/mirrors/invoke_subscript_test.dart:39: main() {
> On 2013/10/29 10:25:24, ahe wrote:
> > Could you add some tests of what happens for im[#missingMethod]?
> 
> Already have im[#doesntExist].

I overlooked it because I was scanning for Expect.throws :-)

[rmacnak, 2013-10-30 18:15:16]

On 2013/10/29 18:16:04, ahe wrote:
> 
> So short-term, how about throwing ArgumentError, then we can fancify it later.

Change error result from null to ArgumentError (implementation, test, doc).
Fixed copy-paste error in test.

[ahe, 2013-10-30 18:33:04]

Still LGTM

[rmacnak, 2013-10-30 19:02:53]

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