Add a VM defined class MirrorReference as an opaque pointer for Dart code to VM internal objects.

Add a VM defined class VMReference as an opaque pointer for Dart code to VM internal objects.

R=asiva@google.com

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

[siva, 2013-06-30 01:12:48]

some initial comments.

https://chromiumcodereview.appspot.com/18242003/diff/1/runtime/lib/mirrors.cc
File runtime/lib/mirrors.cc (right):

https://chromiumcodereview.appspot.com/18242003/diff/1/runtime/lib/mirrors.cc...
runtime/lib/mirrors.cc:128: Dart_Handle cls_name = NewString("OldVMReference");
VMReference for the new internal VM class seems very generic why not call it
something mirror or reflection related?

InstanceReflector
ObjectReflector
ObjectMirror

or something like that.

https://chromiumcodereview.appspot.com/18242003/diff/1/runtime/lib/vm_referen...
File runtime/lib/vm_reference.dart (right):

https://chromiumcodereview.appspot.com/18242003/diff/1/runtime/lib/vm_referen...
runtime/lib/vm_reference.dart:7: }
Why is this needed?

https://chromiumcodereview.appspot.com/18242003/diff/1/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://chromiumcodereview.appspot.com/18242003/diff/1/runtime/vm/object.cc#n...
runtime/vm/object.cc:13689: }
Need to add unit tests in object_test.cc creating this object etc.

https://chromiumcodereview.appspot.com/18242003/diff/1/runtime/vm/raw_object_...
File runtime/vm/raw_object_snapshot.cc (right):

https://chromiumcodereview.appspot.com/18242003/diff/1/runtime/vm/raw_object_...
runtime/vm/raw_object_snapshot.cc:2665: vm_reference.raw_ptr()->referent_ =
reader->ReadObject();
(*reader->ObjectHandle()) = reader->ReadObjectImpl();
vm_reference.set_referent(*reader->ObjectHandle());


(that way you get the StorePointer stuff).

https://chromiumcodereview.appspot.com/18242003/diff/1/runtime/vm/raw_object_...
runtime/vm/raw_object_snapshot.cc:2684: writer->WriteObject(ptr()->referent_);
writer->WriteObjectImpl(ptr()->referent_);

https://chromiumcodereview.appspot.com/18242003/diff/1/runtime/vm/raw_object_...
runtime/vm/raw_object_snapshot.cc:2685: }
Need to add unit tests in snapshot_test.cc so that these methods get
exercised...

[rmacnak, 2013-07-01 15:50:28]

https://chromiumcodereview.appspot.com/18242003/diff/1/runtime/lib/mirrors.cc
File runtime/lib/mirrors.cc (right):

https://chromiumcodereview.appspot.com/18242003/diff/1/runtime/lib/mirrors.cc...
runtime/lib/mirrors.cc:128: Dart_Handle cls_name = NewString("OldVMReference");
On 2013/06/30 01:12:48, siva wrote:
> VMReference for the new internal VM class seems very generic why not call it
> something mirror or reflection related?
> 
> InstanceReflector
> ObjectReflector
> ObjectMirror
> 
> or something like that.

It is intended to be rather generic. It might hold a Class, Library, Type,
Function or Instance depending on what kind of mirror is using it. Itself it is
not a mirror because it does not respond to any part of the mirror API. Perhaps
MirrorReference or MirrorReflectee?

https://chromiumcodereview.appspot.com/18242003/diff/1/runtime/lib/vm_referen...
File runtime/lib/vm_reference.dart (right):

https://chromiumcodereview.appspot.com/18242003/diff/1/runtime/lib/vm_referen...
runtime/lib/vm_reference.dart:7: }
On 2013/06/30 01:12:48, siva wrote:
> Why is this needed?

When one calls currentMirrorSystem(), the initial creation of the local mirror
system causes a class finalization. Without this stub, class finalization fails
when looking for source.

[rmacnak, 2013-07-01 18:35:49]

https://chromiumcodereview.appspot.com/18242003/diff/1/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://chromiumcodereview.appspot.com/18242003/diff/1/runtime/vm/object.cc#n...
runtime/vm/object.cc:13689: }
On 2013/06/30 01:12:48, siva wrote:
> Need to add unit tests in object_test.cc creating this object etc.

Added

https://chromiumcodereview.appspot.com/18242003/diff/1/runtime/vm/raw_object_...
File runtime/vm/raw_object_snapshot.cc (right):

https://chromiumcodereview.appspot.com/18242003/diff/1/runtime/vm/raw_object_...
runtime/vm/raw_object_snapshot.cc:2665: vm_reference.raw_ptr()->referent_ =
reader->ReadObject();
On 2013/06/30 01:12:48, siva wrote:
> (*reader->ObjectHandle()) = reader->ReadObjectImpl();
> vm_reference.set_referent(*reader->ObjectHandle());
> 
> 
> (that way you get the StorePointer stuff).

Changed

https://chromiumcodereview.appspot.com/18242003/diff/1/runtime/vm/raw_object_...
runtime/vm/raw_object_snapshot.cc:2684: writer->WriteObject(ptr()->referent_);
On 2013/06/30 01:12:48, siva wrote:
> writer->WriteObjectImpl(ptr()->referent_);

Changed. Required adding RawVMReference as a friend class.

[rmacnak, 2013-07-01 20:17:55]

Renamed to more intention-revealing MirrorReference. Asserted (de)serialization
unreachable.

[rmacnak, 2013-07-01 23:48:24]

Make MirrorReference's constructor private. Put MirrorReference in dart:mirrors
rather than dart:core.

[siva, 2013-07-02 01:07:11]

LGTM with some comments.

https://chromiumcodereview.appspot.com/18242003/diff/45001/runtime/lib/mirror...
File runtime/lib/mirror_reference.dart (right):

https://chromiumcodereview.appspot.com/18242003/diff/45001/runtime/lib/mirror...
runtime/lib/mirror_reference.dart:1: // Copyright (c) 2012, the Dart project
authors.  Please see the AUTHORS file
2013

https://chromiumcodereview.appspot.com/18242003/diff/45001/runtime/lib/mirror...
runtime/lib/mirror_reference.dart:7: }
I am assuming you will always create this object only from native code and do
not want it instantiated from Dart code.
You could use the model that bool.dart uses:

class bool {
  factory bool._uninstantiable() {
    throw new UnsupportedError(
        "class bool cannot be instantiated");
  }
}

https://chromiumcodereview.appspot.com/18242003/diff/45001/runtime/vm/object_...
File runtime/vm/object_test.cc (right):

https://chromiumcodereview.appspot.com/18242003/diff/45001/runtime/vm/object_...
runtime/vm/object_test.cc:3200: EXPECT_NE(reference.raw(),
other_reference.raw());
EXPECT_EQ(reference.referent(), other_reference.referent());

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

https://chromiumcodereview.appspot.com/18242003/diff/45001/runtime/vm/raw_obj...
runtime/vm/raw_object.h:1531: class RawMirrorReference : public RawInstance {
Could you add a brief comment on top indicating that this class is used by the
mirrors library to reflect on VM objects like library, class etc.

[rmacnak, 2013-07-02 16:17:08]

Stronger guard against MirrorReference instantiation. Additional clause in
object_test.cc. Comment on RawMirrorReference.

[rmacnak, 2013-07-02 16:17:32]

Committed patchset #7 manually as r24694 (presubmit successful).