A round of edits to make mirrors.dart more like our current working

A round of edits to make mirrors.dart more like our current working
proposal.
Committed: https://code.google.com/p/dart/source/detail?r=10918

[cshapiro, 2012-08-17 19:06:09]

lgtm - just minor tweaks to make before submit...

http://codereview.chromium.org/10854197/diff/1/lib/mirrors/mirrors.dart
File lib/mirrors/mirrors.dart (right):

http://codereview.chromium.org/10854197/diff/1/lib/mirrors/mirrors.dart#newco...
lib/mirrors/mirrors.dart:17: // #library("mirrors");
Any reason this is still commented out?

http://codereview.chromium.org/10854197/diff/1/lib/mirrors/mirrors.dart#newco...
lib/mirrors/mirrors.dart:87: * The [MirrorSystem] which contains this mirror.
that contains?

http://codereview.chromium.org/10854197/diff/1/lib/mirrors/mirrors.dart#newco...
lib/mirrors/mirrors.dart:138: * may be either the implicit getter for a field or
a user-defined
can be the implicit...

http://codereview.chromium.org/10854197/diff/1/lib/mirrors/mirrors.dart#newco...
lib/mirrors/mirrors.dart:168: * mirror reflects a simple value.
How about adding a top-level function called IsSimpleValue that returns true
when it is invoked with an object of one of the four simple types?

http://codereview.chromium.org/10854197/diff/1/lib/mirrors/mirrors.dart#newco...
lib/mirrors/mirrors.dart:192: * A [ClosureMirror] provides access to it's
captured variables and
its not it's.

http://codereview.chromium.org/10854197/diff/1/lib/mirrors/mirrors.dart#newco...
lib/mirrors/mirrors.dart:193: * provides the ability to execute it's reflectee.
its not it's.

http://codereview.chromium.org/10854197/diff/1/lib/mirrors/mirrors.dart#newco...
lib/mirrors/mirrors.dart:197: * A mirror on the function for this closure.
of this closure

http://codereview.chromium.org/10854197/diff/1/lib/mirrors/mirrors.dart#newco...
lib/mirrors/mirrors.dart:202: * The source code for this closure, if available.
And what if it's not available?  Null?

http://codereview.chromium.org/10854197/diff/1/lib/mirrors/mirrors.dart#newco...
lib/mirrors/mirrors.dart:440: List<ParameterMirror> parameters();
This should be a final field (a getter).

http://codereview.chromium.org/10854197/diff/1/lib/mirrors/mirrors.dart#newco...
lib/mirrors/mirrors.dart:450: TypeMirror type;
This should be final.

http://codereview.chromium.org/10854197/diff/1/lib/mirrors/mirrors.dart#newco...
lib/mirrors/mirrors.dart:455: String defaultValue;
And this should be final too.

http://codereview.chromium.org/10854197/diff/1/runtime/tests/vm/dart/isolate_...
File runtime/tests/vm/dart/isolate_mirror_local_test.dart (right):

http://codereview.chromium.org/10854197/diff/1/runtime/tests/vm/dart/isolate_...
runtime/tests/vm/dart/isolate_mirror_local_test.dart:17: print('Test $test
finished');
Is this stale debugging code?

http://codereview.chromium.org/10854197/diff/1/runtime/tests/vm/dart/isolate_...
runtime/tests/vm/dart/isolate_mirror_local_test.dart:23: print('All dart:mirrors
tests complete');
Same here.

http://codereview.chromium.org/10854197/diff/1/runtime/tests/vm/dart/isolate_...
runtime/tests/vm/dart/isolate_mirror_local_test.dart:444: print('Starting
dart:mirrors tests');
More stale code?

[turnidge, 2012-08-17 19:44:09]

https://chromiumcodereview.appspot.com/10854197/diff/1/lib/mirrors/mirrors.dart
File lib/mirrors/mirrors.dart (right):

https://chromiumcodereview.appspot.com/10854197/diff/1/lib/mirrors/mirrors.da...
lib/mirrors/mirrors.dart:17: // #library("mirrors");
On 2012/08/17 19:06:09, cshapiro wrote:
> Any reason this is still commented out?

For now we can't have these #directives in code built into the vm.  I hope this
will change.

https://chromiumcodereview.appspot.com/10854197/diff/1/lib/mirrors/mirrors.da...
lib/mirrors/mirrors.dart:87: * The [MirrorSystem] which contains this mirror.
On 2012/08/17 19:06:09, cshapiro wrote:
> that contains?

Done.

https://chromiumcodereview.appspot.com/10854197/diff/1/lib/mirrors/mirrors.da...
lib/mirrors/mirrors.dart:138: * may be either the implicit getter for a field or
a user-defined
On 2012/08/17 19:06:09, cshapiro wrote:
> can be the implicit...

Done.

https://chromiumcodereview.appspot.com/10854197/diff/1/lib/mirrors/mirrors.da...
lib/mirrors/mirrors.dart:168: * mirror reflects a simple value.
On 2012/08/17 19:06:09, cshapiro wrote:
> How about adding a top-level function called IsSimpleValue that returns true
> when it is invoked with an object of one of the four simple types?

Will do in a follow-up CL.

https://chromiumcodereview.appspot.com/10854197/diff/1/lib/mirrors/mirrors.da...
lib/mirrors/mirrors.dart:192: * A [ClosureMirror] provides access to it's
captured variables and
On 2012/08/17 19:06:09, cshapiro wrote:
> its not it's.

Done.

https://chromiumcodereview.appspot.com/10854197/diff/1/lib/mirrors/mirrors.da...
lib/mirrors/mirrors.dart:193: * provides the ability to execute it's reflectee.
On 2012/08/17 19:06:09, cshapiro wrote:
> its not it's.

Done.  How embarrassing.

https://chromiumcodereview.appspot.com/10854197/diff/1/lib/mirrors/mirrors.da...
lib/mirrors/mirrors.dart:197: * A mirror on the function for this closure.
On 2012/08/17 19:06:09, cshapiro wrote:
> of this closure

Changed to "function associated with this closure".  "Function of" causes my
brain to misfire when I read it.

https://chromiumcodereview.appspot.com/10854197/diff/1/lib/mirrors/mirrors.da...
lib/mirrors/mirrors.dart:202: * The source code for this closure, if available.
On 2012/08/17 19:06:09, cshapiro wrote:
> And what if it's not available?  Null?

Done.  This field will probably go away anyways, I think.

https://chromiumcodereview.appspot.com/10854197/diff/1/lib/mirrors/mirrors.da...
lib/mirrors/mirrors.dart:440: List<ParameterMirror> parameters();
On 2012/08/17 19:06:09, cshapiro wrote:
> This should be a final field (a getter).

Done.

https://chromiumcodereview.appspot.com/10854197/diff/1/lib/mirrors/mirrors.da...
lib/mirrors/mirrors.dart:450: TypeMirror type;
On 2012/08/17 19:06:09, cshapiro wrote:
> This should be final.

Done.

https://chromiumcodereview.appspot.com/10854197/diff/1/lib/mirrors/mirrors.da...
lib/mirrors/mirrors.dart:455: String defaultValue;
On 2012/08/17 19:06:09, cshapiro wrote:
> And this should be final too.

Done.

https://chromiumcodereview.appspot.com/10854197/diff/1/runtime/tests/vm/dart/...
File runtime/tests/vm/dart/isolate_mirror_local_test.dart (right):

https://chromiumcodereview.appspot.com/10854197/diff/1/runtime/tests/vm/dart/...
runtime/tests/vm/dart/isolate_mirror_local_test.dart:17: print('Test $test
finished');
On 2012/08/17 19:06:09, cshapiro wrote:
> Is this stale debugging code?

Removed the prints.