[SRE] Refactor a few icalls
authorAleksey Kliger <aleksey@xamarin.com>
Tue, 15 Mar 2016 17:30:55 +0000 (10:30 -0700)
committerAleksey Kliger <aleksey@xamarin.com>
Fri, 18 Mar 2016 21:41:26 +0000 (14:41 -0700)
The icalls follow the pattern:
    void
    the_icall (args... )
    {
       MonoError error;
       (void) the_real_work_function (args..., &error);
       mono_error_set_pending_exception (&error);
    }

    static gboolean
    the_real_work_function (args..., MonoError *error)
    {
       ... /* return TRUE on success, FALSE and set error on failure */ ...
    }

Do this to:
1. mono_reflection_register_with_runtime
2. mono_reflection_setup_internal_class
3. mono_reflection_create_internal_class
4. mono_reflection_generic_class_initialize
5. mono_reflection_event_builder_get_event_info
6. mono_reflection_initialize_generic_parameter
7. mono_reflection_create_dynamic_method

mono/metadata/reflection.c

index 86eb725aadb9de9354f95b95a418d236e60e4fe5..1b634905995780b073cbab8329dfd29969226acf 100644 (file)
@@ -10691,17 +10691,20 @@ mono_reflection_create_unmanaged_type (MonoReflectionType *type)
        mono_error_set_pending_exception (&error);
 }
 
-void
-mono_reflection_register_with_runtime (MonoReflectionType *type)
+static gboolean
+reflection_register_with_runtime (MonoReflectionType *type, MonoError *error)
 {
-       MonoError error;
-       MonoType *res = mono_reflection_type_get_handle (type, &error);
-       mono_error_raise_exception (&error); /* FIXME don't raise here */
        MonoDomain *domain = mono_object_domain ((MonoObject*)type);
        MonoClass *klass;
 
-       if (!res)
-               mono_raise_exception (mono_get_exception_argument (NULL, "Invalid generic instantiation, one or more arguments are not proper user types"));
+       mono_error_init (error);
+
+       MonoType *res = mono_reflection_type_get_handle (type, error);
+
+       if (!res && is_ok (error)) {
+               mono_error_set_argument (error, NULL, "Invalid generic instantiation, one or more arguments are not proper user types");
+       }
+       return_val_if_nok (error, FALSE);
 
        klass = mono_class_from_mono_type (res);
 
@@ -10718,6 +10721,16 @@ mono_reflection_register_with_runtime (MonoReflectionType *type)
        }
        mono_domain_unlock (domain);
        mono_loader_unlock ();
+
+       return TRUE;
+}
+
+void
+mono_reflection_register_with_runtime (MonoReflectionType *type)
+{
+       MonoError error;
+       (void) reflection_register_with_runtime (type, &error);
+       mono_error_set_pending_exception (&error);
 }
 
 /**
@@ -11363,30 +11376,33 @@ leave:
        return result;
 }
 
-/*
- * mono_reflection_setup_internal_class:
+/**
+ * reflection_setup_internal_class:
  * @tb: a TypeBuilder object
+ * @error: set on error
  *
  * Creates a MonoClass that represents the TypeBuilder.
  * This is a trick that lets us simplify a lot of reflection code
  * (and will allow us to support Build and Run assemblies easier).
+ *
+ * Returns TRUE on success. On failure, returns FALSE and sets @error.
  */
-void
-mono_reflection_setup_internal_class (MonoReflectionTypeBuilder *tb)
+static gboolean
+reflection_setup_internal_class (MonoReflectionTypeBuilder *tb, MonoError *error)
 {
-       MonoError error;
        MonoClass *klass, *parent;
 
-       RESOLVE_TYPE (tb->parent, &error);
-       mono_error_raise_exception (&error); /* FIXME don't raise here */
+       mono_error_init (error);
+       RESOLVE_TYPE (tb->parent, error);
+       return_val_if_nok (error, FALSE);
 
        mono_loader_lock ();
 
        if (tb->parent) {
-               MonoType *parent_type = mono_reflection_type_get_handle ((MonoReflectionType*)tb->parent, &error);
-               if (!is_ok (&error)) {
+               MonoType *parent_type = mono_reflection_type_get_handle ((MonoReflectionType*)tb->parent, error);
+               if (!is_ok (error)) {
                        mono_loader_unlock ();
-                       mono_error_raise_exception (&error); /* FIXME don't raise here */
+                       return FALSE;
                }
                /* check so we can compile corlib correctly */
                if (strcmp (mono_object_class (tb->parent)->name, "TypeBuilder") == 0) {
@@ -11408,7 +11424,7 @@ mono_reflection_setup_internal_class (MonoReflectionTypeBuilder *tb)
                mono_class_setup_parent (klass, parent);
                mono_class_setup_mono_type (klass);
                mono_loader_unlock ();
-               return;
+               return TRUE;
        }
 
        klass = (MonoClass *)mono_image_alloc0 (&tb->module->dynamic_image->image, sizeof (MonoClass));
@@ -11416,11 +11432,11 @@ mono_reflection_setup_internal_class (MonoReflectionTypeBuilder *tb)
        klass->image = &tb->module->dynamic_image->image;
 
        klass->inited = 1; /* we lie to the runtime */
-       klass->name = mono_string_to_utf8_image (klass->image, tb->name, &error);
-       if (!mono_error_ok (&error))
+       klass->name = mono_string_to_utf8_image (klass->image, tb->name, error);
+       if (!is_ok (error))
                goto failure;
-       klass->name_space = mono_string_to_utf8_image (klass->image, tb->nspace, &error);
-       if (!mono_error_ok (&error))
+       klass->name_space = mono_string_to_utf8_image (klass->image, tb->nspace, error);
+       if (!is_ok (error))
                goto failure;
        klass->type_token = MONO_TOKEN_TYPE_DEF | tb->table_idx;
        klass->flags = tb->attrs;
@@ -11481,8 +11497,8 @@ mono_reflection_setup_internal_class (MonoReflectionTypeBuilder *tb)
 
        if (tb->nesting_type) {
                g_assert (tb->nesting_type->type);
-               MonoType *nesting_type = mono_reflection_type_get_handle (tb->nesting_type, &error);
-               if (!is_ok (&error)) goto failure;
+               MonoType *nesting_type = mono_reflection_type_get_handle (tb->nesting_type, error);
+               if (!is_ok (error)) goto failure;
                klass->nested_in = mono_class_from_mono_type (nesting_type);
        }
 
@@ -11491,11 +11507,29 @@ mono_reflection_setup_internal_class (MonoReflectionTypeBuilder *tb)
        mono_profiler_class_loaded (klass, MONO_PROFILE_OK);
        
        mono_loader_unlock ();
-       return;
+       return TRUE;
 
 failure:
        mono_loader_unlock ();
-       mono_error_raise_exception (&error);
+       return FALSE;
+}
+
+/**
+ * mono_reflection_setup_internal_class:
+ * @tb: a TypeBuilder object
+ *
+ * (icall)
+ * Creates a MonoClass that represents the TypeBuilder.
+ * This is a trick that lets us simplify a lot of reflection code
+ * (and will allow us to support Build and Run assemblies easier).
+ *
+ */
+void
+mono_reflection_setup_internal_class (MonoReflectionTypeBuilder *tb)
+{
+       MonoError error;
+       (void) reflection_setup_internal_class (tb, &error);
+       mono_error_set_pending_exception (&error);
 }
 
 /*
@@ -11556,18 +11590,22 @@ mono_reflection_create_generic_class (MonoReflectionTypeBuilder *tb)
        klass->generic_container->context.class_inst = mono_get_shared_generic_inst (klass->generic_container);
 }
 
-/*
- * mono_reflection_create_internal_class:
+/**
+ * reflection_create_internal_class:
  * @tb: a TypeBuilder object
+ * @error: set on error
  *
  * Actually create the MonoClass that is associated with the TypeBuilder.
+ * On success returns TRUE, on failure returns FALSE and sets @error.
+ *
  */
-void
-mono_reflection_create_internal_class (MonoReflectionTypeBuilder *tb)
+static gboolean
+reflection_create_internal_class (MonoReflectionTypeBuilder *tb, MonoError *error)
 {
-       MonoError error;
+
        MonoClass *klass;
 
+       mono_error_init (error);
        klass = mono_class_from_mono_type (tb->type.type);
 
        mono_loader_lock ();
@@ -11581,20 +11619,20 @@ mono_reflection_create_internal_class (MonoReflectionTypeBuilder *tb)
 
                fb = mono_array_get (tb->fields, MonoReflectionFieldBuilder*, 0);
 
-               MonoType *field_type = mono_reflection_type_get_handle ((MonoReflectionType*)fb->type, &error);
-               if (!is_ok (&error)) {
+               MonoType *field_type = mono_reflection_type_get_handle ((MonoReflectionType*)fb->type, error);
+               if (!is_ok (error)) {
                        mono_loader_unlock ();
-                       mono_error_raise_exception (&error); /* FIXME don't raise here */
+                       return FALSE;
                }
                if (!mono_type_is_valid_enum_basetype (field_type)) {
                        mono_loader_unlock ();
-                       return;
+                       return TRUE;
                }
 
-               enum_basetype = mono_reflection_type_get_handle ((MonoReflectionType*)fb->type, &error);
-               if (!is_ok (&error)) {
+               enum_basetype = mono_reflection_type_get_handle ((MonoReflectionType*)fb->type, error);
+               if (!is_ok (error)) {
                        mono_loader_unlock ();
-                       mono_error_raise_exception (&error); /* FIXME don't raise here */
+                       return FALSE;
                }
                klass->element_class = mono_class_from_mono_type (enum_basetype);
                if (!klass->element_class)
@@ -11614,6 +11652,22 @@ mono_reflection_create_internal_class (MonoReflectionTypeBuilder *tb)
                mono_class_setup_vtable_general (klass, NULL, 0, NULL);
        }
        mono_loader_unlock ();
+       return TRUE;
+}
+
+/**
+ * mono_reflection_create_internal_class:
+ * @tb: a TypeBuilder object
+ *
+ * (icall)
+ * Actually create the MonoClass that is associated with the TypeBuilder.
+ */
+void
+mono_reflection_create_internal_class (MonoReflectionTypeBuilder *tb)
+{
+       MonoError error;
+       (void) reflection_create_internal_class (tb, &error);
+       mono_error_set_pending_exception (&error);
 }
 
 static MonoMarshalSpec*
@@ -12358,29 +12412,30 @@ inflate_method (MonoReflectionType *type, MonoObject *obj, MonoError *error)
 }
 
 /*TODO avoid saving custom attrs for generic classes as it's enough to have them on the generic type definition.*/
-void
-mono_reflection_generic_class_initialize (MonoReflectionGenericClass *type, MonoArray *fields)
+static gboolean
+reflection_generic_class_initialize (MonoReflectionGenericClass *type, MonoArray *fields, MonoError *error)
 {
-       MonoError error;
        MonoGenericClass *gclass;
        MonoDynamicGenericClass *dgclass;
        MonoClass *klass, *gklass;
        MonoType *gtype;
        int i;
 
-       gtype = mono_reflection_type_get_handle ((MonoReflectionType*)type, &error);
-       mono_error_raise_exception (&error); /* FIXME don't raise here */
+       mono_error_init (error);
+
+       gtype = mono_reflection_type_get_handle ((MonoReflectionType*)type, error);
+       return_val_if_nok (error, FALSE);
        klass = mono_class_from_mono_type (gtype);
        g_assert (gtype->type == MONO_TYPE_GENERICINST);
        gclass = gtype->data.generic_class;
 
        if (!gclass->is_dynamic)
-               return;
+               return TRUE;
 
        dgclass = (MonoDynamicGenericClass *) gclass;
 
        if (dgclass->initialized)
-               return;
+               return TRUE;
 
        gklass = gclass->container_class;
        mono_class_init (gklass);
@@ -12392,13 +12447,12 @@ mono_reflection_generic_class_initialize (MonoReflectionGenericClass *type, Mono
        dgclass->field_generic_types = mono_image_set_new0 (gclass->owner, MonoType*, dgclass->count_fields);
 
        for (i = 0; i < dgclass->count_fields; i++) {
-               MonoError error;
                MonoObject *obj = (MonoObject *)mono_array_get (fields, gpointer, i);
                MonoClassField *field, *inflated_field = NULL;
 
                if (!strcmp (obj->vtable->klass->name, "FieldBuilder")) {
-                       inflated_field = field = fieldbuilder_to_mono_class_field (klass, (MonoReflectionFieldBuilder *) obj, &error);
-                       mono_error_raise_exception (&error); /* FIXME don't raise here */
+                       inflated_field = field = fieldbuilder_to_mono_class_field (klass, (MonoReflectionFieldBuilder *) obj, error);
+                       return_val_if_nok (error, FALSE);
                } else if (!strcmp (obj->vtable->klass->name, "MonoField"))
                        field = ((MonoReflectionField *) obj)->field;
                else {
@@ -12409,8 +12463,8 @@ mono_reflection_generic_class_initialize (MonoReflectionGenericClass *type, Mono
                dgclass->fields [i] = *field;
                dgclass->fields [i].parent = klass;
                dgclass->fields [i].type = mono_class_inflate_generic_type_checked (
-                       field->type, mono_generic_class_get_context ((MonoGenericClass *) dgclass), &error);
-               mono_error_assert_ok (&error); /* FIXME don't swallow the error */
+                       field->type, mono_generic_class_get_context ((MonoGenericClass *) dgclass), error);
+               mono_error_assert_ok (error); /* FIXME don't swallow the error */
                dgclass->field_generic_types [i] = field->type;
                MONO_GC_REGISTER_ROOT_IF_MOVING (dgclass->field_objects [i], MONO_ROOT_SOURCE_REFLECTION, "dynamic generic class field object");
                dgclass->field_objects [i] = obj;
@@ -12423,6 +12477,15 @@ mono_reflection_generic_class_initialize (MonoReflectionGenericClass *type, Mono
        }
 
        dgclass->initialized = TRUE;
+       return TRUE;
+}
+
+void
+mono_reflection_generic_class_initialize (MonoReflectionGenericClass *type, MonoArray *fields)
+{
+       MonoError error;
+       (void) reflection_generic_class_initialize (type, fields, &error);
+       mono_error_set_pending_exception (&error);
 }
 
 void
@@ -12884,15 +12947,19 @@ typebuilder_setup_properties (MonoClass *klass, MonoError *error)
        }
 }
 
-MonoReflectionEvent *
-mono_reflection_event_builder_get_event_info (MonoReflectionTypeBuilder *tb, MonoReflectionEventBuilder *eb)
+static MonoReflectionEvent *
+reflection_event_builder_get_event_info (MonoReflectionTypeBuilder *tb, MonoReflectionEventBuilder *eb, MonoError *error)
 {
-       MonoError error;
+       mono_error_init (error);
+
        MonoEvent *event = g_new0 (MonoEvent, 1);
        MonoClass *klass;
 
-       MonoType *type = mono_reflection_type_get_handle ((MonoReflectionType*)tb, &error);
-       mono_error_raise_exception (&error); /* FIXME don't raise here */
+       MonoType *type = mono_reflection_type_get_handle ((MonoReflectionType*)tb, error);
+       if (!is_ok (error)) {
+               g_free (event);
+               return NULL;
+       }
        klass = mono_class_from_mono_type (type);
 
        event->parent = klass;
@@ -12918,11 +12985,26 @@ mono_reflection_event_builder_get_event_info (MonoReflectionTypeBuilder *tb, Mon
        }
 #endif
 
-       MonoReflectionEvent *ev_obj = mono_event_get_object_checked (mono_object_domain (tb), klass, event, &error);
-       mono_error_raise_exception (&error); /* FIXME don't raise here */
+       MonoReflectionEvent *ev_obj = mono_event_get_object_checked (mono_object_domain (tb), klass, event, error);
+       if (!is_ok (error)) {
+#ifndef MONO_SMALL_CONFIG
+               g_free (event->other);
+#endif
+               g_free (event);
+               return NULL;
+       }
        return ev_obj;
 }
 
+MonoReflectionEvent *
+mono_reflection_event_builder_get_event_info (MonoReflectionTypeBuilder *tb, MonoReflectionEventBuilder *eb)
+{
+       MonoError error;
+       MonoReflectionEvent *result = reflection_event_builder_get_event_info (tb, eb, &error);
+       mono_error_set_pending_exception (&error);
+       return result;
+}
+
 static void
 typebuilder_setup_events (MonoClass *klass, MonoError *error)
 {
@@ -13219,26 +13301,27 @@ failure:
        return NULL;
 }
 
-void
-mono_reflection_initialize_generic_parameter (MonoReflectionGenericParam *gparam)
+static gboolean
+reflection_initialize_generic_parameter (MonoReflectionGenericParam *gparam, MonoError *error)
 {
        MonoGenericParamFull *param;
        MonoImage *image;
        MonoClass *pklass;
-       MonoError error;
+
+       mono_error_init (error);
 
        image = &gparam->tbuilder->module->dynamic_image->image;
 
        param = mono_image_new0 (image, MonoGenericParamFull, 1);
 
-       param->info.name = mono_string_to_utf8_image (image, gparam->name, &error);
-       g_assert (mono_error_ok (&error));
+       param->info.name = mono_string_to_utf8_image (image, gparam->name, error);
+       mono_error_assert_ok (error);
        param->param.num = gparam->index;
 
        if (gparam->mbuilder) {
                if (!gparam->mbuilder->generic_container) {
-                       MonoType *tb = mono_reflection_type_get_handle ((MonoReflectionType*)gparam->mbuilder->type, &error);
-                       mono_error_raise_exception (&error); /* FIXME don't raise here */
+                       MonoType *tb = mono_reflection_type_get_handle ((MonoReflectionType*)gparam->mbuilder->type, error);
+                       return_val_if_nok (error, FALSE);
 
                        MonoClass *klass = mono_class_from_mono_type (tb);
                        gparam->mbuilder->generic_container = (MonoGenericContainer *)mono_image_alloc0 (klass->image, sizeof (MonoGenericContainer));
@@ -13253,8 +13336,8 @@ mono_reflection_initialize_generic_parameter (MonoReflectionGenericParam *gparam
                param->param.owner = gparam->mbuilder->generic_container;
        } else if (gparam->tbuilder) {
                if (!gparam->tbuilder->generic_container) {
-                       MonoType *tb = mono_reflection_type_get_handle ((MonoReflectionType*)gparam->tbuilder, &error);
-                       mono_error_raise_exception (&error); /* FIXME don't raise here */
+                       MonoType *tb = mono_reflection_type_get_handle ((MonoReflectionType*)gparam->tbuilder, error);
+                       return_val_if_nok (error, FALSE);
                        MonoClass *klass = mono_class_from_mono_type (tb);
                        gparam->tbuilder->generic_container = (MonoGenericContainer *)mono_image_alloc0 (klass->image, sizeof (MonoGenericContainer));
                        gparam->tbuilder->generic_container->owner.klass = klass;
@@ -13268,8 +13351,19 @@ mono_reflection_initialize_generic_parameter (MonoReflectionGenericParam *gparam
 
        mono_class_set_ref_info (pklass, gparam);
        mono_image_append_class_to_reflection_info_set (pklass);
+
+       return TRUE;
 }
 
+void
+mono_reflection_initialize_generic_parameter (MonoReflectionGenericParam *gparam)
+{
+       MonoError error;
+       (void) reflection_initialize_generic_parameter (gparam, &error);
+       mono_error_set_pending_exception (&error);
+}
+
+
 MonoArray *
 mono_reflection_sighelper_get_signature_local (MonoReflectionSigHelper *sig)
 {
@@ -13371,10 +13465,9 @@ free_dynamic_method (void *dynamic_method)
        g_free (data);
 }
 
-void 
-mono_reflection_create_dynamic_method (MonoReflectionDynamicMethod *mb)
+static gboolean
+reflection_create_dynamic_method (MonoReflectionDynamicMethod *mb, MonoError *error)
 {
-       MonoError error;
        MonoReferenceQueue *queue;
        MonoMethod *handle;
        DynamicMethodReleaseData *release_data;
@@ -13385,8 +13478,12 @@ mono_reflection_create_dynamic_method (MonoReflectionDynamicMethod *mb)
        GSList *l;
        int i;
 
-       if (mono_runtime_is_shutting_down ())
-               mono_raise_exception (mono_get_exception_invalid_operation (""));
+       mono_error_init (error);
+
+       if (mono_runtime_is_shutting_down ()) {
+               mono_error_set_generic_error (error, "System", "InvalidOperationException", "");
+               return FALSE;
+       }
 
        if (!(queue = dynamic_method_queue)) {
                mono_loader_lock ();
@@ -13395,9 +13492,8 @@ mono_reflection_create_dynamic_method (MonoReflectionDynamicMethod *mb)
                mono_loader_unlock ();
        }
 
-       sig = dynamic_method_to_signature (mb, &error);
-       mono_error_raise_exception (&error); /* FIXME don't raise here */
-
+       sig = dynamic_method_to_signature (mb, error);
+       return_val_if_nok (error, FALSE);
 
        reflection_methodbuilder_from_dynamic_method (&rmb, mb);
 
@@ -13435,10 +13531,10 @@ mono_reflection_create_dynamic_method (MonoReflectionDynamicMethod *mb)
                        handle_class = mono_defaults.methodhandle_class;
                } else {
                        MonoException *ex = NULL;
-                       ref = resolve_object (mb->module->image, obj, &handle_class, NULL, &error);
-                       if (!is_ok  (&error)) {
+                       ref = resolve_object (mb->module->image, obj, &handle_class, NULL, error);
+                       if (!is_ok  (error)) {
                                g_free (rmb.refs);
-                               mono_error_raise_exception (&error); /* FIXME don't raise here */
+                               return FALSE;
                        }
                        if (!ref)
                                ex = mono_get_exception_type_load (NULL, NULL);
@@ -13447,8 +13543,8 @@ mono_reflection_create_dynamic_method (MonoReflectionDynamicMethod *mb)
 
                        if (ex) {
                                g_free (rmb.refs);
-                               mono_raise_exception (ex);
-                               return;
+                               mono_error_set_exception_instance (error, ex);
+                               return FALSE;
                        }
                }
 
@@ -13457,10 +13553,10 @@ mono_reflection_create_dynamic_method (MonoReflectionDynamicMethod *mb)
        }               
 
        if (mb->owner) {
-               MonoType *owner_type = mono_reflection_type_get_handle ((MonoReflectionType*)mb->owner, &error);
-               if (!is_ok (&error)) {
+               MonoType *owner_type = mono_reflection_type_get_handle ((MonoReflectionType*)mb->owner, error);
+               if (!is_ok (error)) {
                        g_free (rmb.refs);
-                       mono_error_raise_exception (&error); /* FIXME don't raise here */
+                       return FALSE;
                }
                klass = mono_class_from_mono_type (owner_type);
        } else {
@@ -13501,6 +13597,16 @@ mono_reflection_create_dynamic_method (MonoReflectionDynamicMethod *mb)
                domain->method_to_dyn_method = g_hash_table_new (NULL, NULL);
        g_hash_table_insert (domain->method_to_dyn_method, handle, (gpointer)(size_t)mono_gchandle_new_weakref ((MonoObject *)mb, TRUE));
        mono_domain_unlock (domain);
+
+       return TRUE;
+}
+
+void
+mono_reflection_create_dynamic_method (MonoReflectionDynamicMethod *mb)
+{
+       MonoError error;
+       (void) reflection_create_dynamic_method (mb, &error);
+       mono_error_set_pending_exception (&error);
 }
 
 #endif /* DISABLE_REFLECTION_EMIT */