Second local mirrors CL.

Second local mirrors CL.

Implement a few chunks of new mirror functionality.  Many changes, notably:

- Added currentIsolateMirror() to provide non-async access to the
  local isolate mirror.

- Added mirrorOf(o) to provide access to local instance mirrors.

- Fleshed out InstanceMirror, InterfaceMirror, and LibraryMirror to
  various degrees.

- Added the MirroredError hierarchy, which lets the user handle Dart
  errors of various sorts from invocation, etc.

- Introduce lazy resolvers for interface mirrors and library mirrors.
  These allow us to build the self-referential library/interface
  hierarchy without going mad.

- Added better toStrings all around.

- Extended the isolate_mirror_local_test to cover new functionality.

Other changes that were required or seemed prudent:

- Exposed the error hierarchy to the embedder.  They need to be able
  to distinguish different error types in some situations.  Improved
  the documetation around error handles and error propagation.

- Added new functions to the dart embedding api as required.  Added
  tests for all new dart api functions.
Committed: https://code.google.com/p/dart/source/detail?r=8575

[turnidge, 2012-06-07 20:43:40]

Ivan,

Here's the next batch of mirror implemention.  Please let me know if you are too
swamped with Danish concerns, and I will switch to a local reviewer.

Gilad,

Just sending this out FYI.  You might want to take a look at mirrors.dart.

Todd

[Ivan Posva, 2012-06-11 16:45:01]

LGTM with comments.
-Ivan

https://chromiumcodereview.appspot.com/10538043/diff/11001/lib/mirrors/mirror...
File lib/mirrors/mirrors.dart (right):

https://chromiumcodereview.appspot.com/10538043/diff/11001/lib/mirrors/mirror...
lib/mirrors/mirrors.dart:163: * TODO(turnidge): Gilad had this as a Map. 
Consider changing this.
What would you map here?

https://chromiumcodereview.appspot.com/10538043/diff/11001/lib/mirrors/mirror...
lib/mirrors/mirrors.dart:239: abstract class MirroredError implements Exception
{
I am not sure how the empty Exception interface helps here.

https://chromiumcodereview.appspot.com/10538043/diff/11001/runtime/include/da...
File runtime/include/dart_api.h (right):

https://chromiumcodereview.appspot.com/10538043/diff/11001/runtime/include/da...
runtime/include/dart_api.h:146: * 17       Dart_PropagateError(result);
You might want to make it clear in the code here that execution does not
continue after Dart_PropagateError() by adding abort() or something similar on
the next line.

https://chromiumcodereview.appspot.com/10538043/diff/11001/runtime/lib/mirror...
File runtime/lib/mirrors_impl.dart (right):

https://chromiumcodereview.appspot.com/10538043/diff/11001/runtime/lib/mirror...
runtime/lib/mirrors_impl.dart:98: output = "\\\\";
You could also use raw strings here. Then the input and output visually match.

https://chromiumcodereview.appspot.com/10538043/diff/11001/runtime/lib/mirror...
runtime/lib/mirrors_impl.dart:208: InterfaceMirror superclass() {
There seems to a mismatch between getters and methods here. We should consider
making these all getters in the interface, but now we have "get library()" but
non-getter "superclass()".

https://chromiumcodereview.appspot.com/10538043/diff/11001/runtime/lib/mirror...
runtime/lib/mirrors_impl.dart:216: List<InterfaceMirror> superinterfaces() {
ditto

https://chromiumcodereview.appspot.com/10538043/diff/11001/runtime/lib/mirror...
runtime/lib/mirrors_impl.dart:229: InterfaceMirror defaultFactory() {
ditto.

https://chromiumcodereview.appspot.com/10538043/diff/11001/runtime/vm/raw_obj...
File runtime/vm/raw_object.h (right):

https://chromiumcodereview.appspot.com/10538043/diff/11001/runtime/vm/raw_obj...
runtime/vm/raw_object.h:308: static bool IsInstanceClassIndex(intptr_t index);
Where is the implementation for this?

[turnidge, 2012-06-12 20:51:46]

Hi Ivan,

I am going to try to commit this change, despite some ongoing discussion on a
few interface issues (getters v. functions, etc.) since it is easy for me to fix
these in my next mirrors CL.  This is all still very much in flux.

Thanks for the review.

Todd

https://chromiumcodereview.appspot.com/10538043/diff/11001/lib/mirrors/mirror...
File lib/mirrors/mirrors.dart (right):

https://chromiumcodereview.appspot.com/10538043/diff/11001/lib/mirrors/mirror...
lib/mirrors/mirrors.dart:163: * TODO(turnidge): Gilad had this as a Map. 
Consider changing this.
On 2012/06/11 16:45:02, Ivan Posva wrote:
> What would you map here?

I have no idea.  Removing the TODO.

https://chromiumcodereview.appspot.com/10538043/diff/11001/lib/mirrors/mirror...
lib/mirrors/mirrors.dart:239: abstract class MirroredError implements Exception
{
On 2012/06/11 16:45:02, Ivan Posva wrote:
> I am not sure how the empty Exception interface helps here.

I thought that it would be useful for people who want to write a catch handler
for all subtypes.

BTW, I have this as a class right now.  Should it be an interface?

https://chromiumcodereview.appspot.com/10538043/diff/11001/runtime/include/da...
File runtime/include/dart_api.h (right):

https://chromiumcodereview.appspot.com/10538043/diff/11001/runtime/include/da...
runtime/include/dart_api.h:146: * 17       Dart_PropagateError(result);
On 2012/06/11 16:45:02, Ivan Posva wrote:
> You might want to make it clear in the code here that execution does not
> continue after Dart_PropagateError() by adding abort() or something similar on
> the next line.

Done.

https://chromiumcodereview.appspot.com/10538043/diff/11001/runtime/lib/mirror...
File runtime/lib/mirrors_impl.dart (right):

https://chromiumcodereview.appspot.com/10538043/diff/11001/runtime/lib/mirror...
runtime/lib/mirrors_impl.dart:98: output = "\\\\";
On 2012/06/11 16:45:02, Ivan Posva wrote:
> You could also use raw strings here. Then the input and output visually match.

Oh, forgot about raw strings.  Thanks.

https://chromiumcodereview.appspot.com/10538043/diff/11001/runtime/lib/mirror...
runtime/lib/mirrors_impl.dart:208: InterfaceMirror superclass() {
On 2012/06/11 16:45:02, Ivan Posva wrote:
> There seems to a mismatch between getters and methods here. We should consider
> making these all getters in the interface, but now we have "get library()" but
> non-getter "superclass()".

I agree that there is a mismatch between getters and methods.  So far I was
making them getters/functions based on Gilad's draft api that he sent out, with
some small variations.

What would your preference be?  All getters?  All functions?

https://chromiumcodereview.appspot.com/10538043/diff/11001/runtime/vm/raw_obj...
File runtime/vm/raw_object.h (right):

https://chromiumcodereview.appspot.com/10538043/diff/11001/runtime/vm/raw_obj...
runtime/vm/raw_object.h:308: static bool IsInstanceClassIndex(intptr_t index);
On 2012/06/11 16:45:02, Ivan Posva wrote:
> Where is the implementation for this?

Nowhere.  Reverted this file.

[ahe, 2012-06-15 13:45:46]

Mirror API changes look great!

Cheers,
Peter

https://chromiumcodereview.appspot.com/10538043/diff/6014/lib/mirrors/mirrors...
File lib/mirrors/mirrors.dart (right):

https://chromiumcodereview.appspot.com/10538043/diff/6014/lib/mirrors/mirrors...
lib/mirrors/mirrors.dart:25: IsolateMirror currentIsolateMirror() {
Long term, I think it would be great if we can separate these methods from the
interfaces. This will make it easier to reuse in the compiler.

https://chromiumcodereview.appspot.com/10538043/diff/6014/lib/mirrors/mirrors...
lib/mirrors/mirrors.dart:141: final String simpleName;
Shouldn't this just be "name"?

https://chromiumcodereview.appspot.com/10538043/diff/6014/lib/mirrors/mirrors...
lib/mirrors/mirrors.dart:169: InterfaceMirror defaultFactory();
Rename to "factoryClass"?

https://chromiumcodereview.appspot.com/10538043/diff/6014/lib/mirrors/mirrors...
lib/mirrors/mirrors.dart:172: * An immutable map from from names to mirrors for
all members of
Why immutable? Do you mean read-only?

https://chromiumcodereview.appspot.com/10538043/diff/6014/lib/mirrors/mirrors...
lib/mirrors/mirrors.dart:178: * TODO(turnidge): Currently empty.
General comment. I think it would be best to move TODOs out of documentation
comments.

https://chromiumcodereview.appspot.com/10538043/diff/6014/lib/mirrors/mirrors...
lib/mirrors/mirrors.dart:180: Map<String, Mirror> members();
For example, add the TODO here using //.

https://chromiumcodereview.appspot.com/10538043/diff/6014/lib/mirrors/mirrors...
lib/mirrors/mirrors.dart:203: * An immutable map from from top-level names to
mirrors for all
Why immutable?

https://chromiumcodereview.appspot.com/10538043/diff/6014/lib/mirrors/mirrors...
lib/mirrors/mirrors.dart:264: "Uncaught exception during mirrored execution:
<${exception_string}>";
I think you can remove the <...> around the original message.

https://chromiumcodereview.appspot.com/10538043/diff/6014/lib/mirrors/mirrors...
lib/mirrors/mirrors.dart:282: return "Compile-time error during mirrored
execution: <$message>";
Ditto for <...>