Implement VariableMirror creation with non-API code. To break circular initialization, have them fi…

Implement VariableMirror creation with non-API code. To break circular initialization, have them find their type lazily.

R=asiva@google.com

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

[rmacnak, 2013-07-20 00:46:52]

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

https://codereview.chromium.org/19512003/diff/6001/runtime/lib/mirrors.cc#new...
runtime/lib/mirrors.cc:615: args.SetAt(3, Instance::Handle());  // Null for
type.
We still pass this argument because ParameterMirrors, a subclass, use it.

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

https://codereview.chromium.org/19512003/diff/6001/runtime/lib/mirrors_impl.d...
runtime/lib/mirrors_impl.dart:1106: operator ==(other) {
Allow tests to pass which previously relied on the mirrors for void and dynamic
being canonical. They will be canonical again once they are in the object store.

[siva, 2013-07-22 05:25:32]

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

https://codereview.chromium.org/19512003/diff/6001/runtime/lib/mirrors.cc#new...
runtime/lib/mirrors.cc:781: owner_mirror);
Looks like this is the only caller for CreateVariableMirrorUsingApi.
Since this function is already using API stuff why not just inline all the
unwrap stuff here and just call CreateVariableMirror.

Do you anticipate more callers for CreateVariableMirrorUsingApi ?

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

https://codereview.chromium.org/19512003/diff/6001/runtime/lib/mirrors_impl.d...
runtime/lib/mirrors_impl.dart:1106: operator ==(other) {
Please add an issue to track this temporary workaround and add a TODO(#number)
here to get rid of this code once void and dynamic types are created in the VM
isolate.

On 2013/07/20 00:46:52, Ryan Macnak wrote:
> Allow tests to pass which previously relied on the mirrors for void and
dynamic
> being canonical. They will be canonical again once they are in the object
store.

[rmacnak, 2013-07-22 19:05:09]

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

https://codereview.chromium.org/19512003/diff/6001/runtime/lib/mirrors.cc#new...
runtime/lib/mirrors.cc:781: owner_mirror);
On 2013/07/22 05:25:32, siva wrote:
> Looks like this is the only caller for CreateVariableMirrorUsingApi.
> Since this function is already using API stuff why not just inline all the
> unwrap stuff here and just call CreateVariableMirror.
> 
> Do you anticipate more callers for CreateVariableMirrorUsingApi ?

I don't. Inlined.

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

https://codereview.chromium.org/19512003/diff/6001/runtime/lib/mirrors_impl.d...
runtime/lib/mirrors_impl.dart:1106: operator ==(other) {
On 2013/07/22 05:25:32, siva wrote:
> Please add an issue to track this temporary workaround and add a TODO(#number)
> here to get rid of this code once void and dynamic types are created in the VM
> isolate.

Done.

[siva, 2013-07-22 22:42:46]

lgtm

[rmacnak, 2013-07-22 23:00:18]

Merged.

[rmacnak, 2013-07-22 23:06:01]

Committed patchset #3 manually as r25317 (presubmit successful).

[ahe, 2013-07-24 09:45:30]

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

https://codereview.chromium.org/19512003/diff/23001/runtime/lib/mirrors_impl....
runtime/lib/mirrors_impl.dart:1113: }
Always override hashCode when overriding operator==.