Reflection tidying.

Move out arguments to the right and make pointer not reference types.
Remove unused unbox for argument routine.
Simplify convert primitive routine for the case of identical types.

Change-Id: I6456331b0f3f3e5f0b2c361a9f50b4ed1c9462a3

4 files changed, 47 insertions, 78 deletions

diff --git a/runtime/entrypoints/entrypoint_utils.cc b/runtime/entrypoints/entrypoint_utils.cc
index 829ec4a..9e5f54c 100644
--- a/runtime/entrypoints/entrypoint_utils.cc
+++ b/runtime/entrypoints/entrypoint_utils.cc
@@ -200,7 +200,7 @@ JValue InvokeProxyInvocationHandler(ScopedObjectAccessUnchecked& soa, const char
       }
       ThrowLocation throw_location(rcvr, proxy_method, -1);
       JValue result_unboxed;
-      if (!UnboxPrimitiveForResult(throw_location, result_ref, result_type, result_unboxed)) {
+      if (!UnboxPrimitiveForResult(throw_location, result_ref, result_type, &result_unboxed)) {
         DCHECK(soa.Self()->IsExceptionPending());
         return zero;
       }
diff --git a/runtime/native/java_lang_reflect_Field.cc b/runtime/native/java_lang_reflect_Field.cc
index 6667d51..a69bd05 100644
--- a/runtime/native/java_lang_reflect_Field.cc
+++ b/runtime/native/java_lang_reflect_Field.cc
@@ -141,7 +141,7 @@ static JValue GetPrimitiveField(JNIEnv* env, jobject javaField, jobject javaObj,
   // Widen it if necessary (and possible).
   JValue wide_value;
   if (!ConvertPrimitiveValue(NULL, false, field_type, Primitive::GetType(dst_descriptor),
-                             field_value, wide_value)) {
+                             field_value, &wide_value)) {
     DCHECK(soa.Self()->IsExceptionPending());
     return JValue();
   }
@@ -257,7 +257,7 @@ static void Field_set(JNIEnv* env, jobject javaField, jobject javaObj, jobject j
   // Unbox the value, if necessary.
   mirror::Object* boxed_value = soa.Decode<mirror::Object*>(javaValue);
   JValue unboxed_value;
-  if (!UnboxPrimitiveForField(boxed_value, field_type, unboxed_value, f)) {
+  if (!UnboxPrimitiveForField(boxed_value, field_type, f, &unboxed_value)) {
     DCHECK(soa.Self()->IsExceptionPending());
     return;
   }
@@ -282,7 +282,7 @@ static void SetPrimitiveField(JNIEnv* env, jobject javaField, jobject javaObj, c
   // Widen the value if necessary (and possible).
   JValue wide_value;
   if (!ConvertPrimitiveValue(nullptr, false, Primitive::GetType(src_descriptor),
-                             field_type, new_value, wide_value)) {
+                             field_type, new_value, &wide_value)) {
     DCHECK(soa.Self()->IsExceptionPending());
     return;
   }
diff --git a/runtime/reflection.cc b/runtime/reflection.cc
index f567055..7f39e70 100644
--- a/runtime/reflection.cc
+++ b/runtime/reflection.cc
@@ -543,76 +543,58 @@ bool VerifyObjectIsClass(mirror::Object* o, mirror::Class* c) {
 
 bool ConvertPrimitiveValue(const ThrowLocation* throw_location, bool unbox_for_result,
                            Primitive::Type srcType, Primitive::Type dstType,
-                           const JValue& src, JValue& dst) {
-  CHECK(srcType != Primitive::kPrimNot && dstType != Primitive::kPrimNot);
+                           const JValue& src, JValue* dst) {
+  DCHECK(srcType != Primitive::kPrimNot && dstType != Primitive::kPrimNot);
+  if (LIKELY(srcType == dstType)) {
+    dst->SetJ(src.GetJ());
+    return true;
+  }
   switch (dstType) {
-  case Primitive::kPrimBoolean:
-    if (srcType == Primitive::kPrimBoolean) {
-      dst.SetZ(src.GetZ());
-      return true;
-    }
-    break;
-  case Primitive::kPrimChar:
-    if (srcType == Primitive::kPrimChar) {
-      dst.SetC(src.GetC());
-      return true;
-    }
-    break;
+  case Primitive::kPrimBoolean:  // Fall-through.
+  case Primitive::kPrimChar:  // Fall-through.
   case Primitive::kPrimByte:
-    if (srcType == Primitive::kPrimByte) {
-      dst.SetB(src.GetB());
-      return true;
-    }
+    // Only expect assignment with source and destination of identical type.
     break;
   case Primitive::kPrimShort:
-    if (srcType == Primitive::kPrimByte || srcType == Primitive::kPrimShort) {
-      dst.SetS(src.GetI());
+    if (srcType == Primitive::kPrimByte) {
+      dst->SetS(src.GetI());
       return true;
     }
     break;
   case Primitive::kPrimInt:
     if (srcType == Primitive::kPrimByte || srcType == Primitive::kPrimChar ||
-        srcType == Primitive::kPrimShort || srcType == Primitive::kPrimInt) {
-      dst.SetI(src.GetI());
+        srcType == Primitive::kPrimShort) {
+      dst->SetI(src.GetI());
       return true;
     }
     break;
   case Primitive::kPrimLong:
     if (srcType == Primitive::kPrimByte || srcType == Primitive::kPrimChar ||
         srcType == Primitive::kPrimShort || srcType == Primitive::kPrimInt) {
-      dst.SetJ(src.GetI());
-      return true;
-    } else if (srcType == Primitive::kPrimLong) {
-      dst.SetJ(src.GetJ());
+      dst->SetJ(src.GetI());
       return true;
     }
     break;
   case Primitive::kPrimFloat:
     if (srcType == Primitive::kPrimByte || srcType == Primitive::kPrimChar ||
         srcType == Primitive::kPrimShort || srcType == Primitive::kPrimInt) {
-      dst.SetF(src.GetI());
+      dst->SetF(src.GetI());
       return true;
     } else if (srcType == Primitive::kPrimLong) {
-      dst.SetF(src.GetJ());
-      return true;
-    } else if (srcType == Primitive::kPrimFloat) {
-      dst.SetF(src.GetF());
+      dst->SetF(src.GetJ());
       return true;
     }
     break;
   case Primitive::kPrimDouble:
     if (srcType == Primitive::kPrimByte || srcType == Primitive::kPrimChar ||
         srcType == Primitive::kPrimShort || srcType == Primitive::kPrimInt) {
-      dst.SetD(src.GetI());
+      dst->SetD(src.GetI());
       return true;
     } else if (srcType == Primitive::kPrimLong) {
-      dst.SetD(src.GetJ());
+      dst->SetD(src.GetJ());
       return true;
     } else if (srcType == Primitive::kPrimFloat) {
-      dst.SetD(src.GetF());
-      return true;
-    } else if (srcType == Primitive::kPrimDouble) {
-      dst.SetJ(src.GetJ());
+      dst->SetD(src.GetF());
       return true;
     }
     break;
@@ -642,7 +624,7 @@ mirror::Object* BoxPrimitive(Primitive::Type src_class, const JValue& value) {
     return nullptr;
   }
 
-  jmethodID m = NULL;
+  jmethodID m = nullptr;
   const char* shorty;
   switch (src_class) {
   case Primitive::kPrimBoolean:
@@ -698,29 +680,25 @@ mirror::Object* BoxPrimitive(Primitive::Type src_class, const JValue& value) {
   return result.GetL();
 }
 
-static std::string UnboxingFailureKind(mirror::ArtMethod* m, int index, mirror::ArtField* f)
+static std::string UnboxingFailureKind(mirror::ArtField* f)
     SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
-  if (m != NULL && index != -1) {
-    ++index;  // Humans count from 1.
-    return StringPrintf("method %s argument %d", PrettyMethod(m, false).c_str(), index);
-  }
-  if (f != NULL) {
+  if (f != nullptr) {
     return "field " + PrettyField(f, false);
   }
   return "result";
 }
 
 static bool UnboxPrimitive(const ThrowLocation* throw_location, mirror::Object* o,
-                           mirror::Class* dst_class, JValue& unboxed_value,
-                           mirror::ArtMethod* m, int index, mirror::ArtField* f)
+                           mirror::Class* dst_class, mirror::ArtField* f,
+                           JValue* unboxed_value)
     SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
-  bool unbox_for_result = (f == NULL) && (index == -1);
+  bool unbox_for_result = (f == nullptr);
   if (!dst_class->IsPrimitive()) {
-    if (UNLIKELY(o != NULL && !o->InstanceOf(dst_class))) {
+    if (UNLIKELY(o != nullptr && !o->InstanceOf(dst_class))) {
       if (!unbox_for_result) {
         ThrowIllegalArgumentException(throw_location,
                                       StringPrintf("%s has type %s, got %s",
-                                                   UnboxingFailureKind(m, index, f).c_str(),
+                                                   UnboxingFailureKind(f).c_str(),
                                                    PrettyDescriptor(dst_class).c_str(),
                                                    PrettyTypeOf(o).c_str()).c_str());
       } else {
@@ -731,20 +709,20 @@ static bool UnboxPrimitive(const ThrowLocation* throw_location, mirror::Object*
       }
       return false;
     }
-    unboxed_value.SetL(o);
+    unboxed_value->SetL(o);
     return true;
   }
   if (UNLIKELY(dst_class->GetPrimitiveType() == Primitive::kPrimVoid)) {
     ThrowIllegalArgumentException(throw_location,
                                   StringPrintf("Can't unbox %s to void",
-                                               UnboxingFailureKind(m, index, f).c_str()).c_str());
+                                               UnboxingFailureKind(f).c_str()).c_str());
     return false;
   }
-  if (UNLIKELY(o == NULL)) {
+  if (UNLIKELY(o == nullptr)) {
     if (!unbox_for_result) {
       ThrowIllegalArgumentException(throw_location,
                                     StringPrintf("%s has type %s, got null",
-                                                 UnboxingFailureKind(m, index, f).c_str(),
+                                                 UnboxingFailureKind(f).c_str(),
                                                  PrettyDescriptor(dst_class).c_str()).c_str());
     } else {
       ThrowNullPointerException(throw_location,
@@ -756,7 +734,7 @@ static bool UnboxPrimitive(const ThrowLocation* throw_location, mirror::Object*
 
   JValue boxed_value;
   const StringPiece src_descriptor(ClassHelper(o->GetClass()).GetDescriptor());
-  mirror::Class* src_class = NULL;
+  mirror::Class* src_class = nullptr;
   ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
   mirror::ArtField* primitive_field = o->GetClass()->GetIFields()->Get(0);
   if (src_descriptor == "Ljava/lang/Boolean;") {
@@ -786,7 +764,7 @@ static bool UnboxPrimitive(const ThrowLocation* throw_location, mirror::Object*
   } else {
     ThrowIllegalArgumentException(throw_location,
                                   StringPrintf("%s has type %s, got %s",
-                                               UnboxingFailureKind(m, index, f).c_str(),
+                                               UnboxingFailureKind(f).c_str(),
                                                PrettyDescriptor(dst_class).c_str(),
                                                PrettyDescriptor(src_descriptor.data()).c_str()).c_str());
     return false;
@@ -797,21 +775,15 @@ static bool UnboxPrimitive(const ThrowLocation* throw_location, mirror::Object*
                                boxed_value, unboxed_value);
 }
 
-bool UnboxPrimitiveForArgument(mirror::Object* o, mirror::Class* dst_class, JValue& unboxed_value,
-                               mirror::ArtMethod* m, size_t index) {
-  CHECK(m != NULL);
-  return UnboxPrimitive(NULL, o, dst_class, unboxed_value, m, index, NULL);
-}
-
-bool UnboxPrimitiveForField(mirror::Object* o, mirror::Class* dst_class, JValue& unboxed_value,
-                            mirror::ArtField* f) {
-  CHECK(f != NULL);
-  return UnboxPrimitive(NULL, o, dst_class, unboxed_value, NULL, -1, f);
+bool UnboxPrimitiveForField(mirror::Object* o, mirror::Class* dst_class, mirror::ArtField* f,
+                            JValue* unboxed_value) {
+  DCHECK(f != nullptr);
+  return UnboxPrimitive(nullptr, o, dst_class, f, unboxed_value);
 }
 
 bool UnboxPrimitiveForResult(const ThrowLocation& throw_location, mirror::Object* o,
-                             mirror::Class* dst_class, JValue& unboxed_value) {
-  return UnboxPrimitive(&throw_location, o, dst_class, unboxed_value, NULL, -1, NULL);
+                             mirror::Class* dst_class, JValue* unboxed_value) {
+  return UnboxPrimitive(&throw_location, o, dst_class, nullptr, unboxed_value);
 }
 
 }  // namespace art
diff --git a/runtime/reflection.h b/runtime/reflection.h
index d2f9f25..325998f 100644
--- a/runtime/reflection.h
+++ b/runtime/reflection.h
@@ -36,19 +36,16 @@ class ThrowLocation;
 
 mirror::Object* BoxPrimitive(Primitive::Type src_class, const JValue& value)
     SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
-bool UnboxPrimitiveForArgument(mirror::Object* o, mirror::Class* dst_class, JValue& unboxed_value,
-                               mirror::ArtMethod* m, size_t index)
-    SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
-bool UnboxPrimitiveForField(mirror::Object* o, mirror::Class* dst_class, JValue& unboxed_value,
-                            mirror::ArtField* f)
+bool UnboxPrimitiveForField(mirror::Object* o, mirror::Class* dst_class, mirror::ArtField* f,
+                            JValue* unboxed_value)
     SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
 bool UnboxPrimitiveForResult(const ThrowLocation& throw_location, mirror::Object* o,
-                             mirror::Class* dst_class, JValue& unboxed_value)
+                             mirror::Class* dst_class, JValue* unboxed_value)
     SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
 
 bool ConvertPrimitiveValue(const ThrowLocation* throw_location, bool unbox_for_result,
                            Primitive::Type src_class, Primitive::Type dst_class,
-                           const JValue& src, JValue& dst)
+                           const JValue& src, JValue* dst)
     SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
 
 JValue InvokeWithVarArgs(const ScopedObjectAccess& soa, jobject obj, jmethodID mid, va_list args)