Convert MethodMirror.returnType to native calls (and more)

runtime/lib/mirrors.cc

Index: runtime/lib/mirrors.cc
diff --git a/runtime/lib/mirrors.cc b/runtime/lib/mirrors.cc
index 149ec5063260cb9a908b1621d6b6036584b286f7..08563906f4654dcba10ae263a6e6694fa628266e 100644
--- a/runtime/lib/mirrors.cc
+++ b/runtime/lib/mirrors.cc
@@ -17,6 +17,22 @@
 
 namespace dart {
 
+static RawInstance* CreateMirror(const String& mirror_class_name,
+                                 const Array& constructor_arguments) {
+  const Library& mirrors_lib = Library::Handle(Library::MirrorsLibrary());
+  const String& constructor_name = Symbols::Dot();
+
+  const Object& result = Object::Handle(
+      DartLibraryCalls::ExceptionCreate(mirrors_lib,

[Michael Lippautz (Google), 2013/07/19 19:03:02]

ExceptionCreate should at some point (this CL?) be renamed to CreateObject,
because it's a general function to create objects (nothing specific to
exceptions).

[siva, 2013/07/19 21:01:40]

Good point, I do not know why we called it ExceptionCreate, we should probably
call it InstanceCreate (creates an instance using the constructor).
Please do the rename in a new CL (the only place it is used is exceptions.cc).

[Michael Lippautz (Google), 2013/07/19 22:13:20]

Done.

+                                        mirror_class_name,
+                                        constructor_name,
+                                        constructor_arguments));
+  ASSERT(!result.IsError());
+  ASSERT(result.IsInstance());

[rmacnak, 2013/07/19 20:37:22]

Doesn't IsInstance() guarantee !IsError()?

[Michael Lippautz (Google), 2013/07/19 20:53:01]

Leftover from debugging. Done.

+  return Instance::Cast(result).raw();
+}
+
+
 inline Dart_Handle NewString(const char* str) {
   return Dart_NewStringFromCString(str);
 }
@@ -355,9 +371,9 @@ static Dart_Handle CreateImplementsList(Dart_Handle intf) {
 }
 
 
-static Dart_Handle CreateTypeVariableMirror(Dart_Handle type_var,
-                                            Dart_Handle type_var_name,
-                                            Dart_Handle owner_mirror) {
+static Dart_Handle CreateTypeVariableMirrorUsingApi(Dart_Handle type_var,
+                                                    Dart_Handle type_var_name,
+                                                    Dart_Handle owner_mirror) {
   ASSERT(Dart_IsTypeVariable(type_var));
   Dart_Handle cls_name = NewString("_LocalTypeVariableMirrorImpl");
   Dart_Handle type = Dart_GetType(MirrorLib(), cls_name, 0, NULL);
@@ -381,6 +397,50 @@ static Dart_Handle CreateTypeVariableMirror(Dart_Handle type_var,
 }
 
 
+static RawInstance* CreateTypeVariableMirror(const TypeParameter& param,
+                                             const Instance& owner_mirror) {
+  Instance& retvalue = Instance::Handle();
+  Dart_EnterScope();
+  Isolate* isolate = Isolate::Current();
+  Dart_Handle param_handle = Api::NewHandle(isolate, param.raw());
+  if (Dart_IsError(param_handle)) {
+    Dart_PropagateError(param_handle);
+  }
+  Dart_Handle name_handle = Api::NewHandle(isolate, param.Name());
+  if (Dart_IsError(name_handle)) {
+    Dart_PropagateError(name_handle);
+  }
+  // Until we get rid of lazy mirrors, we must have owners.
+  Dart_Handle owner_handle;
+  if (owner_mirror.IsNull()) {
+    owner_handle = Api::NewHandle(isolate, param.parameterized_class());
+    if (Dart_IsError(owner_handle)) {
+      Dart_PropagateError(owner_handle);
+    }
+    owner_handle = CreateLazyMirror(owner_handle);
+    if (Dart_IsError(owner_handle)) {
+      Dart_PropagateError(owner_handle);
+    }
+  } else {
+    owner_handle = Api::NewHandle(isolate, owner_mirror.raw());
+    if (Dart_IsError(owner_handle)) {
+      Dart_PropagateError(owner_handle);
+    }
+  }
+  // TODO(11742): At some point the handle calls will be replaced by inlined
+  // functionality.
+  Dart_Handle result =  CreateTypeVariableMirrorUsingApi(param_handle,
+                                                         name_handle,
+                                                         owner_handle);
+  if (Dart_IsError(result)) {
+    Dart_PropagateError(result);
+  }
+  retvalue ^= Api::UnwrapHandle(result);
+  Dart_ExitScope();
+  return retvalue.raw();
+}
+
+
 static Dart_Handle CreateTypeVariableMap(Dart_Handle owner,
                                          Dart_Handle owner_mirror) {
   ASSERT(Dart_IsClass(owner));
@@ -407,7 +467,7 @@ static Dart_Handle CreateTypeVariableMap(Dart_Handle owner,
     }
     ASSERT(!Dart_IsNull(type_var));
     Dart_Handle type_var_mirror =
-        CreateTypeVariableMirror(type_var, type_var_name, owner_mirror);
+        CreateTypeVariableMirrorUsingApi(type_var, type_var_name, owner_mirror);
     if (Dart_IsError(type_var_mirror)) {
       return type_var_mirror;
     }
@@ -513,7 +573,6 @@ static Dart_Handle CreateMethodMirrorUsingApi(Dart_Handle func,
                                               Dart_Handle owner_mirror) {
   // TODO(11742): Unwrapping is needed until the whole method is converted.
   Isolate* isolate = Isolate::Current();
-  DARTSCOPE(isolate);
   const Function& func_obj = Api::UnwrapFunctionHandle(isolate, func);
 
   Dart_Handle mirror_cls_name = NewString("_LocalMethodMirrorImpl");
@@ -522,17 +581,11 @@ static Dart_Handle CreateMethodMirrorUsingApi(Dart_Handle func,
     return mirror_type;
   }
 
-  Dart_Handle return_type = Dart_FunctionReturnType(func);

[Michael Lippautz (Google), 2013/07/19 19:03:02]

We get rid of Dart_FunctionReturnType() in this place. There's still one caller
left, though.

-  if (Dart_IsError(return_type)) {
-    return return_type;
-  }
-
   // TODO(turnidge): Implement constructor kinds (arguments 7 - 10).
   Dart_Handle args[] = {
     CreateMirrorReference(func),
     owner_mirror,
     CreateParameterMirrorList(func),
-    CreateLazyMirror(return_type),
     func_obj.is_static() ? Api::True() : Api::False(),
     func_obj.is_abstract() ? Api::True() : Api::False(),
     func_obj.IsGetterFunction() ? Api::True() : Api::False(),
@@ -980,7 +1033,20 @@ static RawInstance* CreateClassMirror(const Class& cls,
   if (Dart_IsError(name_handle)) {
     Dart_PropagateError(name_handle);
   }
-  Dart_Handle lib_mirror = Api::NewHandle(isolate, owner_mirror.raw());
+  // Until we get rid of lazy mirrors, we must have owners.

[rmacnak, 2013/07/19 20:37:22]

Unnecessary after https://chromiumcodereview.appspot.com/19749008/ ?

[Michael Lippautz (Google), 2013/07/19 20:53:01]

As I think of it now, it was not even necessary in first place, because _owner
gets initialized lazily if it was unknown at creation time.

+  Dart_Handle lib_mirror;
+  if (owner_mirror.IsNull()) {
+    Dart_Handle owner_handle = Api::NewHandle(isolate, cls.library());
+    if (Dart_IsError(owner_handle)) {
+      Dart_PropagateError(owner_handle);
+    }
+    lib_mirror = CreateLazyMirror(owner_handle);
+  } else {
+    lib_mirror = Api::NewHandle(isolate, owner_mirror.raw());
+  }
+  if (Dart_IsError(lib_mirror)) {
+    Dart_PropagateError(lib_mirror);
+  }
   // TODO(11742): At some point the handle calls will be replaced by inlined
   // functionality.
   Dart_Handle result =  CreateClassMirrorUsingApi(cls_handle,
@@ -1031,6 +1097,38 @@ static RawInstance* CreateMethodMirror(const Function& func,
 }
 
 
+static RawInstance* CreateTypeMirror(const AbstractType& type) {
+  ASSERT(!type.IsMalformed());
+  Isolate* isolate = Isolate::Current();
+  if (type.HasResolvedTypeClass()) {
+    const Class& cls = Class::Handle(type.type_class());
+    // Handle void and dynamic types.
+    if (cls.IsVoidClass()) {
+      const String& class_name = String::Handle(
+          String::New("_SpecialTypeMirrorImpl"));

[siva, 2013/07/19 21:01:40]

Please add _SpecialTypeMirrorImpl to the symbols list and use
Symbols::_SpecialTypeMirrorImpl() here

[Michael Lippautz (Google), 2013/07/19 22:13:20]

Done.

+      const String& name = String::Handle(String::New("void"));

[siva, 2013/07/19 21:01:40]

this should be Symbols::Void()

[Michael Lippautz (Google), 2013/07/19 22:13:20]

Done.

+      Array& args = Array::Handle(Array::New(1));
+      args.SetAt(0, name);
+      return CreateMirror(class_name, args);
+    } else if (cls.IsDynamicClass()) {
+      const String& class_name = String::Handle(
+          String::New("_SpecialTypeMirrorImpl"));

[siva, 2013/07/19 21:01:40]

Symbols::_SpecialTypeMirrorImpl() here.

[Michael Lippautz (Google), 2013/07/19 22:13:20]

Done.

+      const String& name = String::Handle(String::New("dynamic"));

[siva, 2013/07/19 21:01:40]

Symbols::Dynamic() here.

[Michael Lippautz (Google), 2013/07/19 22:13:20]

Done.

+      Array& args = Array::Handle(Array::New(1));
+      args.SetAt(0, name);
+      return CreateMirror(class_name, args);
+    }
+    return CreateClassMirror(cls, Instance::Handle());
+  } else if (type.IsTypeParameter()) {
+    TypeParameter& param = TypeParameter::Handle(
+        isolate, TypeParameter::Cast(type).raw());

[siva, 2013/07/19 21:01:40]

why cast an reassign back to another newly created handle?

return CreateTypeVariableMirror(TypeParameter::Cast(type),
Object::null_instance());

[Michael Lippautz (Google), 2013/07/19 22:13:20]

Done.

+    return CreateTypeVariableMirror(param, Instance::Handle());
+  }
+  UNREACHABLE();
+  return Instance::null();
+}
+
+
 void NATIVE_ENTRY_FUNCTION(Mirrors_makeLocalMirrorSystem)(
     Dart_NativeArguments args) {
   Dart_EnterScope();
@@ -1806,19 +1904,15 @@ DEFINE_NATIVE_ENTRY(LibraryMirror_invokeSetter, 4) {
 
 
 DEFINE_NATIVE_ENTRY(MethodMirror_name, 1) {
-  const MirrorReference& func_ref =
-      MirrorReference::CheckedHandle(arguments->NativeArgAt(0));
-  Function& func = Function::Handle();
-  func ^= func_ref.referent();
+  const Function& func = Function::Handle(
+      MirrorReference::GetFunctionReferent(arguments->NativeArgAt(0)));

[siva, 2013/07/19 21:01:40]

I just realized for all native methods we normally throw an exception if an
illegal argument is passed in. For some we may also have to
check to make sure it is not null.

We should write these native methods as:

So this would be:

{
  GET_NON_NULL_NATIVE_ARGUMENT(MirrorReference, ref, arguments->NativeArgAt(0));
  const Function& func = Function::Handle(ref.GetFunctionReferent());
  return func.UserVisibleName();

}

Could you do this part in a separate CL.

[Michael Lippautz (Google), 2013/07/19 22:13:20]

Ok, I will do the change for the two existing native calls in a separate CL.

   return func.UserVisibleName();
 }
 
 
 DEFINE_NATIVE_ENTRY(MethodMirror_owner, 1) {
-  const MirrorReference& func_ref =
-      MirrorReference::CheckedHandle(arguments->NativeArgAt(0));
-  Function& func = Function::Handle();
-  func ^= func_ref.referent();
+  const Function& func = Function::Handle(
+      MirrorReference::GetFunctionReferent(arguments->NativeArgAt(0)));
   if (func.IsNonImplicitClosureFunction()) {
     return CreateMethodMirror(Function::Handle(
         func.parent_function()), Instance::Handle());
@@ -1830,4 +1924,14 @@ DEFINE_NATIVE_ENTRY(MethodMirror_owner, 1) {
   return CreateClassMirror(owner, Instance::Handle());
 }
 
+
+DEFINE_NATIVE_ENTRY(MethodMirror_return_type, 1) {
+  const Function& func = Function::Handle(
+      MirrorReference::GetFunctionReferent(arguments->NativeArgAt(0)));
+  // We handle constructors in dart code.

[rmacnak, 2013/07/19 20:37:22]

Dart

[Michael Lippautz (Google), 2013/07/19 20:53:01]

Done.

+  ASSERT(!func.IsConstructor());
+  const AbstractType& return_type = AbstractType::Handle(func.result_type());
+  return CreateTypeMirror(return_type);
+}
+
 }  // namespace dart