[Mono-dev] [patch] partial implementation of System.Reflection.Emit.DynamicILInfo

[Paolo Molaro]

On 03/07/08 Casey Marshall wrote:
> The attached patches add a partial implementation of DynamicILInfo --
> basically enough that you can specify the IL code and locals for a
> method, and have that method runnable.
> 
> I might work on this more if I have time, *plus* I'm not sure if this
> here is too naive, and would need to change a lot for a full
> implementation.

A few comments below.

> Index: mono/metadata/object-internals.h
> ===================================================================
> --- mono/metadata/object-internals.h	(revision 97635)
> +++ mono/metadata/object-internals.h	(working copy)
> @@ -1004,6 +1004,15 @@
>  } MonoReflectionGuidAttribute;
>  
>  typedef struct {
> +    MonoObject object;
> +    MonoArray *code;
> +    gint32 code_len;
> +    gint32 max_stack;
> +    MonoArray *exceptions;
> +    MonoArray *localsig;

For newly introduced types, please put all the reference fields at the
start.

> +} MonoReflectionDynamicILInfo;
> +
> +typedef struct {
>  	MonoObject object;
>  	MonoMethod *mhandle;
>  	MonoString *name;
> @@ -1019,8 +1028,10 @@
>  	MonoArray *refs;
>  	GSList *referenced_by;
>  	MonoReflectionType *owner;
> +    MonoReflectionDynamicILInfo *dynil;

Use TABs to indent.

> Index: mono/metadata/reflection.c
> ===================================================================
> --- mono/metadata/reflection.c	(revision 97635)
> +++ mono/metadata/reflection.c	(working copy)
> @@ -1454,7 +1455,7 @@
>  	rmb->attrs = mb->attrs;
>  	rmb->iattrs = 0;
>  	rmb->call_conv = mb->call_conv;
> -	rmb->code = NULL;
> +    rmb->code = NULL;

Your patch introduces whitespace damage.

> @@ -8682,6 +8684,16 @@
>  			num_locals = rmb->ilgen->locals ? mono_array_length (rmb->ilgen->locals) : 0;
>  			if (rmb->ilgen->ex_handlers)
>  				num_clauses = method_count_clauses (rmb->ilgen);
> +		} else if (rmb->dynil) {
> +			char *ptr = mono_array_addr (rmb->dynil->localsig, guint8, 0);
> +			code = mono_array_addr (rmb->dynil->code, guint8, 0);
> +			code_size = rmb->dynil->code_len;
> +			max_stack = rmb->dynil->max_stack;
> +			if (*ptr == 0x07)
> +			{

Put the opening brace at the end of the previous line.

> +		if (rmb->dynil)
> +		{
> +			const char *ptr = mono_array_addr (rmb->dynil->localsig, guint8, 0);
> +			ptr += 2;
> +			for (i = 0; i < num_locals; i++)
> +			{
> +				MonoType *t = mono_metadata_parse_type_full(NULL, NULL, MONO_PARSE_LOCAL,
> +															0, ptr, &ptr);

Unfortunately it isn't this simple: the above will work only for very
simple types. Extensive testing may be needed to see which tokens are
assigned for user types.

> Index: class/corlib/System.Reflection.Emit/DynamicMethod.cs
> ===================================================================
> --- class/corlib/System.Reflection.Emit/DynamicMethod.cs	(revision 97638)
> +++ class/corlib/System.Reflection.Emit/DynamicMethod.cs	(working copy)
> @@ -61,6 +61,7 @@
>  		private object[] refs;
>  		private IntPtr referenced_by;
>  		private Type owner;
> +        private DynamicILInfo dynil;


Whitespace damage here as well.

It would be nice if you added also the tests for the new functionality.
This is good for a start, though.
Note that for runtime changes for us to be able to accept them you need
to explicitly license them under the MIT/X11 license.
Thanks!

lupus

-- 
-----------------------------------------------------------------
lupus at debian.org                                     debian/rules
lupus at ximian.com                             Monkeys do it better