Support named and optional positional arguments in reflective invocation.

Support named and optional positional arguments in reflective invocation.

R=ahe@google.com, regis@google.com

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

[siva, 2013-08-29 00:16:29]

Adding Regis to the review list.

[regis, 2013-08-29 23:31:31]

LGTM

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

https://codereview.chromium.org/23604003/diff/12001/runtime/lib/mirrors.cc#ne...
runtime/lib/mirrors.cc:736: // Note "arguments" is already the internal
arguments with the receiver as
You mean "args" instead of "arguments".

https://codereview.chromium.org/23604003/diff/12001/runtime/lib/mirrors.cc#ne...
runtime/lib/mirrors.cc:876: // InstanceMirror_invoke.
This kind of comment should be preceded by a TODO, or we will forget about it.
TODO(iposva) is a good choice, since he is working on that :-)

https://codereview.chromium.org/23604003/diff/12001/runtime/lib/mirrors.cc#ne...
runtime/lib/mirrors.cc:1126: // arguments?
This should be a TODO too, but not TODO(iposva) for this one.

https://codereview.chromium.org/23604003/diff/12001/runtime/lib/mirrors.cc#ne...
runtime/lib/mirrors.cc:1127: args.SetAt(0, TypeArguments::Handle());
I think we preallocate such a null handle in object.cc.

[ahe, 2013-09-04 12:52:18]

Nice test which LGTM!
Did look carefully at the changes to runtime.

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

https://codereview.chromium.org/23604003/diff/20001/runtime/lib/mirrors_impl....
runtime/lib/mirrors_impl.dart:107: throw "named argument ${_n(name)} ($arg) was
not a simple value or InstanceMirror";
Long name.

https://codereview.chromium.org/23604003/diff/20001/tests/lib/mirrors/invoke_...
File tests/lib/mirrors/invoke_named_test.dart (right):

https://codereview.chromium.org/23604003/diff/20001/tests/lib/mirrors/invoke_...
tests/lib/mirrors/invoke_named_test.dart:1: // Copyright (c) 2013, the Dart
project authors.  Please see the AUTHORS file
Test contains many long lines, is that to work around lack of symbol literals in
dart2js?

[rmacnak, 2013-09-04 17:12:04]

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

https://codereview.chromium.org/23604003/diff/20001/runtime/lib/mirrors_impl....
runtime/lib/mirrors_impl.dart:107: throw "named argument ${_n(name)} ($arg) was
not a simple value or InstanceMirror";
On 2013/09/04 12:52:18, ahe wrote:
> Long name.

Wrong variable name here and in the positional counterpart too.

https://codereview.chromium.org/23604003/diff/20001/tests/lib/mirrors/invoke_...
File tests/lib/mirrors/invoke_named_test.dart (right):

https://codereview.chromium.org/23604003/diff/20001/tests/lib/mirrors/invoke_...
tests/lib/mirrors/invoke_named_test.dart:1: // Copyright (c) 2013, the Dart
project authors.  Please see the AUTHORS file
On 2013/09/04 12:52:18, ahe wrote:
> Test contains many long lines, is that to work around lack of symbol literals
in
> dart2js?

The VM doesn't support symbol literals yet either. I originally wrote the test
and implementation with strings. Things fit then.

[rmacnak, 2013-09-04 17:12:35]

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