EAKING CHANGE: Rename InstanceMIrror.invoke, .getField, .setField to invokeAsync, getFieldAsync, se…

BREAKING CHANGE: Rename InstanceMIrror.invoke, .getField, .setField to invokeAsync, getFieldAsync, setFieldAsync. Likewise ClosureMirror.apply and ClassMIrror.newInstance. This is in preparation of new synchronous versions of the originals.
Committed: https://code.google.com/p/dart/source/detail?r=21242

[gbracha, 2013-04-10 00:12:08]

BREAKING CHANGE. First step toward synchronous mirrors.

[Ivan Posva, 2013-04-10 00:30:10]

Please remove the unused code from the review.

Thanks,
-Ivan

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

https://codereview.chromium.org/13967003/diff/1/runtime/lib/mirrors_impl.dart...
runtime/lib/mirrors_impl.dart:121: String toString() => "IsolateMirror on
'$debugName'";
Line ends in white space.

https://codereview.chromium.org/13967003/diff/1/runtime/lib/mirrors_impl.dart...
runtime/lib/mirrors_impl.dart:158: return _getField(this, fieldName); // needs
to be distinct for sync calls
Please replace tab with spaces eventually.

https://codereview.chromium.org/13967003/diff/1/runtime/lib/mirrors_impl.dart...
runtime/lib/mirrors_impl.dart:170: List positionalArguments,
Indentation.

https://codereview.chromium.org/13967003/diff/1/runtime/lib/mirrors_impl.dart...
runtime/lib/mirrors_impl.dart:335: [Map<String,Object> namedArguments]) {
Indentation.

https://codereview.chromium.org/13967003/diff/1/runtime/lib/mirrors_impl.dart...
runtime/lib/mirrors_impl.dart:531: List positionalArguments,
Indentation.

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

https://codereview.chromium.org/13967003/diff/1/sdk/lib/mirrors/mirrors.dart#...
sdk/lib/mirrors/mirrors.dart:185: List<Object> positionalArguments,
Indentation.

https://codereview.chromium.org/13967003/diff/1/sdk/lib/mirrors/mirrors.dart#...
sdk/lib/mirrors/mirrors.dart:201: * The argument must be either an
InstanceMirrors, a number, a string
InstanceMirror minus the s.

https://codereview.chromium.org/13967003/diff/1/sdk/lib/mirrors/mirrors.dart#...
sdk/lib/mirrors/mirrors.dart:273: [Map<String,Object> namedArguments]);
Indentation.

https://codereview.chromium.org/13967003/diff/1/sdk/lib/mirrors/mirrors.dart#...
sdk/lib/mirrors/mirrors.dart:449: List<Object> positionalArguments,
ditto

[ahe, 2013-04-10 05:52:22]

There are a bunch of stylistic issues, but otherwise LGTM!

This will break dart2js, see
sdk/lib/_internal/compiler/implementation/lib/mirrors_patch.dart, and the test
tests/compiler/dart2js_extra/mirror_test.dart.

When you have uploaded a new patch, it is customary to hit "publish+mail
comments" and say that you did so, then the reviewers know to take another look.
Also, "please take another look" can be abbreviated to PTAL. So I often send out
a message saying simply PTAL.

https://codereview.chromium.org/13967003/diff/9001/runtime/lib/mirrors_impl.dart
File runtime/lib/mirrors_impl.dart (left):

https://codereview.chromium.org/13967003/diff/9001/runtime/lib/mirrors_impl.d...
runtime/lib/mirrors_impl.dart:166: {
Open curly brace on previous line

https://codereview.chromium.org/13967003/diff/9001/runtime/lib/mirrors_impl.d...
runtime/lib/mirrors_impl.dart:177: {
Ditto.

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

https://codereview.chromium.org/13967003/diff/9001/runtime/lib/mirrors_impl.d...
runtime/lib/mirrors_impl.dart:978: 
Delete two extra lines.

https://codereview.chromium.org/13967003/diff/9001/runtime/tests/vm/dart/isol...
File runtime/tests/vm/dart/isolate_mirror_local_test.dart (right):

https://codereview.chromium.org/13967003/diff/9001/runtime/tests/vm/dart/isol...
runtime/tests/vm/dart/isolate_mirror_local_test.dart:120:
lib_mirror.invokeAsync('function', [ 123 ]).then(
Delete spaces after open square bracket and before close square bracket.

https://codereview.chromium.org/13967003/diff/9001/sdk/lib/mirrors/mirrors.dart
File sdk/lib/mirrors/mirrors.dart (left):

https://codereview.chromium.org/13967003/diff/9001/sdk/lib/mirrors/mirrors.da...
sdk/lib/mirrors/mirrors.dart:180: * TODO(turnidge): Handle optional & named
arguments.
Remove TODO from documentation, it should just be plain comments.

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

https://codereview.chromium.org/13967003/diff/9001/sdk/lib/mirrors/mirrors.da...
sdk/lib/mirrors/mirrors.dart:178: * or booleans.
[InstanceMirror], [num], [String], or [bool].

[gbracha, 2013-04-10 18:01:16]

Acted on Peter's comments.

https://codereview.chromium.org/13967003/diff/9001/sdk/lib/mirrors/mirrors.dart
File sdk/lib/mirrors/mirrors.dart (left):

https://codereview.chromium.org/13967003/diff/9001/sdk/lib/mirrors/mirrors.da...
sdk/lib/mirrors/mirrors.dart:180: * TODO(turnidge): Handle optional & named
arguments.
On 2013/04/10 05:52:22, ahe wrote:
> Remove TODO from documentation, it should just be plain comments.

I will do this for method affected by this CL.  There are other such cases which
should also change, but others have expressed a desire that the CL focus on  one
main purpose, so the others can be changed later.

[Ivan Posva, 2013-04-10 23:21:21]

LGTM -ip

[gbracha, 2013-04-10 23:58:30]

Committed patchset #4 manually as r21242 (presubmit successful).