Make LibraryMirror.members an internal native and lazy.

runtime/lib/mirrors.cc

Index: runtime/lib/mirrors.cc
===================================================================
--- runtime/lib/mirrors.cc	(revision 25418)
+++ runtime/lib/mirrors.cc	(working copy)
@@ -505,7 +505,6 @@
 }
 
 
-static Dart_Handle CreateMemberMap(Dart_Handle owner, Dart_Handle owner_mirror);
 static Dart_Handle CreateConstructorMap(Dart_Handle owner,
                                         Dart_Handle owner_mirror);
 
@@ -615,84 +614,6 @@
 }
 
 
-static Dart_Handle AddMemberClasses(Dart_Handle map,
-                                    Dart_Handle owner,
-                                    Dart_Handle owner_mirror) {
-  ASSERT(Dart_IsLibrary(owner));
-  Dart_Handle result;
-  Dart_Handle names = Dart_LibraryGetClassNames(owner);
-  if (Dart_IsError(names)) {
-    return names;
-  }
-  intptr_t len;
-  result = Dart_ListLength(names, &len);
-  if (Dart_IsError(result)) {
-    return result;
-  }
-  for (intptr_t i = 0; i < len; i++) {
-    Dart_Handle intf_name = Dart_ListGetAt(names, i);
-    Dart_Handle intf = Dart_GetClass(owner, intf_name);
-    if (Dart_IsError(intf)) {
-      return intf;
-    }
-    Dart_Handle intf_mirror =
-        CreateClassMirrorUsingApi(intf, intf_name, owner_mirror);
-    if (Dart_IsError(intf_mirror)) {
-      return intf_mirror;
-    }
-    result = MapAdd(map, intf_name, intf_mirror);
-    if (Dart_IsError(result)) {
-      return result;
-    }
-  }
-  return Dart_True();
-}
-
-
-static Dart_Handle AddMemberFunctions(Dart_Handle map,
-                                      Dart_Handle owner,
-                                      Dart_Handle owner_mirror) {
-  Dart_Handle result;
-  Dart_Handle names = Dart_GetFunctionNames(owner);
-  if (Dart_IsError(names)) {
-    return names;
-  }
-  intptr_t len;
-  result = Dart_ListLength(names, &len);
-  if (Dart_IsError(result)) {
-    return result;
-  }
-  for (intptr_t i = 0; i < len; i++) {
-    Dart_Handle func_name = Dart_ListGetAt(names, i);
-    Dart_Handle func = Dart_LookupFunction(owner, func_name);
-    if (Dart_IsError(func)) {
-      return func;
-    }
-    ASSERT(!Dart_IsNull(func));
-
-    bool is_constructor = false;
-    result = Dart_FunctionIsConstructor(func, &is_constructor);
-    if (Dart_IsError(result)) {
-      return result;
-    }
-    if (is_constructor) {
-      // Skip constructors.
-      continue;
-    }
-
-    Dart_Handle func_mirror = CreateMethodMirrorUsingApi(func, owner_mirror);
-    if (Dart_IsError(func_mirror)) {
-      return func_mirror;
-    }
-    result = MapAdd(map, func_name, func_mirror);
-    if (Dart_IsError(result)) {
-      return result;
-    }
-  }
-  return Dart_True();
-}
-
-
 static Dart_Handle AddConstructors(Dart_Handle map,
                                    Dart_Handle owner,
                                    Dart_Handle owner_mirror) {
@@ -737,70 +658,6 @@
 }
 
 
-static Dart_Handle AddMemberVariables(Dart_Handle map,
-                                      Dart_Handle owner,
-                                      Dart_Handle owner_mirror) {
-  Isolate* isolate = Isolate::Current();
-  Dart_Handle result;
-  Dart_Handle names = Dart_GetVariableNames(owner);
-  if (Dart_IsError(names)) {
-    return names;
-  }
-  intptr_t len;
-  result = Dart_ListLength(names, &len);
-  if (Dart_IsError(result)) {
-    return result;
-  }
-  for (intptr_t i = 0; i < len; i++) {
-    Dart_Handle var_name = Dart_ListGetAt(names, i);
-    Dart_Handle var = Dart_LookupVariable(owner, var_name);
-    if (Dart_IsError(var)) {
-      return var;
-    }
-    ASSERT(!Dart_IsNull(var));
-    ASSERT(Dart_IsVariable(var));
-    const Field& field = Api::UnwrapFieldHandle(isolate, var);
-    const Instance& owner_mirror_inst =
-      Api::UnwrapInstanceHandle(isolate, owner_mirror);
-    const Instance& var_mirror_inst =
-      Instance::Handle(CreateVariableMirror(field, owner_mirror_inst));
-    Dart_Handle var_mirror = Api::NewHandle(isolate, var_mirror_inst.raw());
-
-    result = MapAdd(map, var_name, var_mirror);
-    if (Dart_IsError(result)) {
-      return result;
-    }
-  }
-  return Dart_True();
-}
-
-
-static Dart_Handle CreateMemberMap(Dart_Handle owner,
-                                   Dart_Handle owner_mirror) {
-  // TODO(turnidge): This should be an immutable map.
-  if (Dart_IsError(owner_mirror)) {
-    return owner_mirror;
-  }
-  Dart_Handle result;
-  Dart_Handle map = MapNew();
-  if (Dart_IsLibrary(owner)) {
-    result = AddMemberClasses(map, owner, owner_mirror);
-    if (Dart_IsError(result)) {
-      return result;
-    }
-  }
-  result = AddMemberFunctions(map, owner, owner_mirror);
-  if (Dart_IsError(result)) {
-    return result;
-  }
-  result = AddMemberVariables(map, owner, owner_mirror);
-  if (Dart_IsError(result)) {
-    return result;
-  }
-  return map;
-}
-
-
 static Dart_Handle CreateConstructorMap(Dart_Handle owner,
                                         Dart_Handle owner_mirror) {
   // TODO(turnidge): This should be an immutable map.
@@ -827,15 +684,10 @@
   if (Dart_IsError(lazy_lib_mirror)) {
     return lazy_lib_mirror;
   }
-  Dart_Handle member_map = CreateMemberMap(lib, lazy_lib_mirror);
-  if (Dart_IsError(member_map)) {
-    return member_map;
-  }
   Dart_Handle args[] = {
     CreateMirrorReference(lib),
     Dart_LibraryName(lib),
     Dart_LibraryUrl(lib),
-    member_map,
   };
   Dart_Handle lib_mirror = Dart_New(type, Dart_Null(), ARRAY_SIZE(args), args);
   if (Dart_IsError(lib_mirror)) {
@@ -1247,7 +1099,7 @@
 DEFINE_NATIVE_ENTRY(ClassMirror_name, 1) {
   GET_NON_NULL_NATIVE_ARGUMENT(MirrorReference, ref, arguments->NativeArgAt(0));
   const Class& klass = Class::Handle(ref.GetClassReferent());
-  return klass.Name();
+  return klass.UserVisibleName();
 }
 
 
@@ -1303,6 +1155,69 @@
 }
 
 
+DEFINE_NATIVE_ENTRY(LibraryMirror_members, 2) {
+  GET_NON_NULL_NATIVE_ARGUMENT(Instance,
+                               owner_mirror,
+                               arguments->NativeArgAt(0));
+  GET_NON_NULL_NATIVE_ARGUMENT(MirrorReference, ref, arguments->NativeArgAt(1));
+  const Library& library = Library::Handle(ref.GetLibraryReferent());
+
+  Instance& member_mirror = Instance::Handle();
+  const GrowableObjectArray& member_mirrors =
+      GrowableObjectArray::Handle(GrowableObjectArray::New());
+
+  Field& field = Field::Handle();
+  Class& klass = Class::Handle();
+  Function& func = Function::Handle();

[siva, 2013/07/24 22:41:35]

You don't need these 3 handles, see comment below.

+
+  Object& entry = Object::Handle();
+  DictionaryIterator entries(library);
+
+  String& user_visible_name = String::Handle();
+  Class& lookup_class = Class::Handle();
+
+  while (entries.HasNext()) {
+    entry = entries.GetNext();
+    if (entry.IsClass()) {
+      klass ^= entry.raw();

[siva, 2013/07/24 22:41:35]

you could replace this with the Cast operator:

   const Klass& klass = Klass::Cast(entry);

(this doesn't create a new handle just casts the existing handle to
the new type which you have already checked and verified by doing a
entry.IsClass() above).

Ditto comment for 'field' and 'func' below.

[rmacnak, 2013/07/24 22:51:43]

Done.

+      if (!klass.IsCanonicalSignatureClass()) {
+        // The various implementations of public classes don't always have the
+        // expected superinterfaces or other properties, so we filter them out
+        // (e.g., ObjectArray would displace the abstract class List but doesn't
+        // answer [Iterable] as its superinterfaces). Checking that the normal
+        // and user-visible names match does not work because it excludes
+        // private members.
+        user_visible_name = klass.UserVisibleName();
+        lookup_class = library.LookupClassAllowPrivate(user_visible_name, NULL);
+        bool is_lookup_class;
+        {
+          NoGCScope();
+          is_lookup_class = klass.raw() == lookup_class.raw();
+        }

[siva, 2013/07/24 22:41:35]

Replace this with the internal class id check as discussed offline.

[rmacnak, 2013/07/24 22:51:43]

Done.

+        if (is_lookup_class) {
+          member_mirror = CreateClassMirror(klass, owner_mirror);
+          member_mirrors.Add(member_mirror);
+        }
+      }
+    } else if (entry.IsField()) {
+      field ^= entry.raw();
+      member_mirror = CreateVariableMirror(field, owner_mirror);
+      member_mirrors.Add(member_mirror);
+    } else if (entry.IsFunction()) {
+      func ^= entry.raw();
+      if (func.kind() == RawFunction::kRegularFunction ||
+          func.kind() == RawFunction::kGetterFunction ||
+          func.kind() == RawFunction::kSetterFunction) {
+        member_mirror = CreateMethodMirror(func, owner_mirror);
+        member_mirrors.Add(member_mirror);
+      }
+    }
+  }
+
+  return member_mirrors.raw();
+}
+
+
 // Invoke the function, or noSuchMethod if it is null. Propagate any unhandled
 // exceptions. Wrap and propagate any compilation errors.
 static RawObject* ReflectivelyInvokeDynamicFunction(const Instance& receiver,