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

runtime/lib/mirrors.cc

Index: runtime/lib/mirrors.cc
===================================================================
--- runtime/lib/mirrors.cc	(revision 25253)
+++ runtime/lib/mirrors.cc	(working copy)
@@ -600,44 +600,37 @@
   return mirror;
 }
 
+static RawInstance* CreateVariableMirror(const Field& field,
+                                         const Instance& owner_mirror) {
+  const MirrorReference& field_ref =
+      MirrorReference::Handle(MirrorReference::New());
+  field_ref.set_referent(field);
 
-static Dart_Handle CreateVariableMirror(Dart_Handle var,
-                                        Dart_Handle var_name,
-                                        Dart_Handle owner_mirror) {
-  ASSERT(Dart_IsVariable(var));
-  Dart_Handle cls_name = NewString("_LocalVariableMirrorImpl");
-  Dart_Handle type = Dart_GetType(MirrorLib(), cls_name, 0, NULL);
-  if (Dart_IsError(type)) {
-    return type;
-  }
+  const String& name = String::Handle(field.UserVisibleName());
 
-  bool is_static = false;
-  bool is_final = false;
+  const Array& args = Array::Handle(Array::New(6));
+  args.SetAt(0, field_ref);
+  args.SetAt(1, name);
+  args.SetAt(2, owner_mirror);
+  args.SetAt(3, Instance::Handle());  // Null for type.

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

We still pass this argument because ParameterMirrors, a subclass, use it.

+  args.SetAt(4, field.is_static() ? Bool::True() : Bool::False());
+  args.SetAt(5, field.is_final()  ? Bool::True() : Bool::False());
 
-  Dart_Handle result = Dart_VariableIsStatic(var, &is_static);
-  if (Dart_IsError(result)) {
-    return result;
-  }
-  result = Dart_VariableIsFinal(var, &is_final);
-  if (Dart_IsError(result)) {
-    return result;
-  }
+  return CreateMirror(Symbols::_LocalVariableMirrorImpl(), args);
+}
 
-  Dart_Handle var_type = Dart_VariableType(var);
-  if (Dart_IsError(var_type)) {
-    return var_type;
-  }
 
-  Dart_Handle args[] = {
-    CreateMirrorReference(var),
-    var_name,
-    owner_mirror,
-    CreateLazyMirror(var_type),
-    Dart_NewBoolean(is_static),
-    Dart_NewBoolean(is_final),
-  };
-  Dart_Handle mirror = Dart_New(type, Dart_Null(), ARRAY_SIZE(args), args);
-  return mirror;
+static Dart_Handle CreateVariableMirrorUsingApi(Dart_Handle var,
+                                                Dart_Handle var_name,
+                                                Dart_Handle owner_mirror) {
+  ASSERT(Dart_IsVariable(var));
+  Isolate* isolate = Isolate::Current();
+  const Field& field = Api::UnwrapFieldHandle(isolate, var);
+  const Instance& owner_mirror_inst =
+      Api::UnwrapInstanceHandle(isolate, owner_mirror);
+  const Instance& var_mirror =
+      Instance::Handle(CreateVariableMirror(field, owner_mirror_inst));
+  return Api::NewHandle(isolate, var_mirror.raw());
 }
 
 
@@ -783,7 +776,9 @@
       return var;
     }
     ASSERT(!Dart_IsNull(var));
-    Dart_Handle var_mirror = CreateVariableMirror(var, var_name, owner_mirror);
+    Dart_Handle var_mirror = CreateVariableMirrorUsingApi(var,
+                                                          var_name,
+                                                          owner_mirror);

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

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 ?

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

I don't. Inlined.

     if (Dart_IsError(var_mirror)) {
       return var_mirror;
     }
@@ -1918,4 +1913,13 @@
   return CreateTypeMirror(return_type);
 }
 
+
+DEFINE_NATIVE_ENTRY(VariableMirror_type, 1) {
+  GET_NON_NULL_NATIVE_ARGUMENT(MirrorReference, ref, arguments->NativeArgAt(0));
+  const Field& field = Field::Handle(ref.GetFieldReferent());
+
+  const AbstractType& type = AbstractType::Handle(field.type());
+  return CreateTypeMirror(type);
+}
+
 }  // namespace dart