Refactored the use of Delegate.CreateDelegate to use FrameworkPortability.CreateDelegate,

retooled MessageStreamIterator.cs to use safer/less complicated reflection.

4 files changed, 28 insertions, 105 deletions

diff --git a/src/ProtocolBuffers/FieldAccess/ReflectionUtil.cs b/src/ProtocolBuffers/FieldAccess/ReflectionUtil.cs
index 84c53e26..92e8a07c 100644
--- a/src/ProtocolBuffers/FieldAccess/ReflectionUtil.cs
+++ b/src/ProtocolBuffers/FieldAccess/ReflectionUtil.cs
@@ -71,7 +71,7 @@ namespace Google.ProtocolBuffers.FieldAccess
             // Convert the reflection call into an open delegate, i.e. instead of calling x.Method()

             // we'll call getter(x).

             Func<TSource, TResult> getter =

-                (Func<TSource, TResult>) Delegate.CreateDelegate(typeof(Func<TSource, TResult>), null, method);

+                (Func<TSource, TResult>)FrameworkPortability.CreateDelegate(typeof(Func<TSource, TResult>), null, method);

 

             // Implicit upcast to object (within the delegate)

             return delegate(TSource source) { return getter(source); };

@@ -93,7 +93,7 @@ namespace Google.ProtocolBuffers.FieldAccess
             // Convert the reflection call into an open delegate, i.e. instead of calling x.Method(y) we'll

             // call Method(x, y)

             Action<TSource, TParam> call =

-                (Action<TSource, TParam>) Delegate.CreateDelegate(typeof(Action<TSource, TParam>), null, method);

+                (Action<TSource, TParam>)FrameworkPortability.CreateDelegate(typeof(Action<TSource, TParam>), null, method);

 

             return delegate(TSource source, object parameter) { call(source, (TParam) parameter); };

         }

@@ -116,7 +116,7 @@ namespace Google.ProtocolBuffers.FieldAccess
             // Convert the reflection call into an open delegate, i.e. instead of calling x.Method(y) we'll

             // call Method(x, y)

             Func<TSource, TParam, TReturn> call = (Func<TSource, TParam, TReturn>)

-                                                  Delegate.CreateDelegate(typeof(Func<TSource, TParam, TReturn>), null,

+                                                  FrameworkPortability.CreateDelegate(typeof(Func<TSource, TParam, TReturn>), null,

                                                                           method);

 

             return delegate(TSource source, object parameter) { call(source, (TParam) parameter); };

@@ -134,7 +134,7 @@ namespace Google.ProtocolBuffers.FieldAccess
 

         public static Func<IBuilder> CreateStaticUpcastDelegateImpl<T>(MethodInfo method)

         {

-            Func<T> call = (Func<T>) Delegate.CreateDelegate(typeof(Func<T>), null, method);

+            Func<T> call = (Func<T>)FrameworkPortability.CreateDelegate(typeof(Func<T>), null, method);

             return delegate { return (IBuilder) call(); };

         }

     }

diff --git a/src/ProtocolBuffers/FieldAccess/RepeatedPrimitiveAccessor.cs b/src/ProtocolBuffers/FieldAccess/RepeatedPrimitiveAccessor.cs
index fd2c72b1..0a173fea 100644
--- a/src/ProtocolBuffers/FieldAccess/RepeatedPrimitiveAccessor.cs
+++ b/src/ProtocolBuffers/FieldAccess/RepeatedPrimitiveAccessor.cs
@@ -84,10 +84,9 @@ namespace Google.ProtocolBuffers.FieldAccess
             {

                 throw new ArgumentException("Not all required properties/methods available");

             }

-            clearDelegate =

-                (Func<TBuilder, IBuilder>) Delegate.CreateDelegate(typeof(Func<TBuilder, IBuilder>), null, clearMethod);

-            countDelegate = (Func<TMessage, int>) Delegate.CreateDelegate

-                                                      (typeof(Func<TMessage, int>), null, countProperty.GetGetMethod());

+            clearDelegate = (Func<TBuilder, IBuilder>)

+                FrameworkPortability.CreateDelegate(typeof(Func<TBuilder, IBuilder>), null, clearMethod);

+            countDelegate = (Func<TMessage, int>)FrameworkPortability.CreateDelegate(typeof(Func<TMessage, int>), null, countProperty.GetGetMethod());

             getValueDelegate = ReflectionUtil.CreateUpcastDelegate<TMessage>(messageProperty.GetGetMethod());

             addValueDelegate = ReflectionUtil.CreateDowncastDelegateIgnoringReturn<TBuilder>(addMethod);

             getRepeatedWrapperDelegate = ReflectionUtil.CreateUpcastDelegate<TBuilder>(builderProperty.GetGetMethod());

diff --git a/src/ProtocolBuffers/FieldAccess/SinglePrimitiveAccessor.cs b/src/ProtocolBuffers/FieldAccess/SinglePrimitiveAccessor.cs
index 7d20ea75..e52af03d 100644
--- a/src/ProtocolBuffers/FieldAccess/SinglePrimitiveAccessor.cs
+++ b/src/ProtocolBuffers/FieldAccess/SinglePrimitiveAccessor.cs
@@ -69,9 +69,9 @@ namespace Google.ProtocolBuffers.FieldAccess
             clrType = messageProperty.PropertyType;

             hasDelegate =

                 (Func<TMessage, bool>)

-                Delegate.CreateDelegate(typeof(Func<TMessage, bool>), null, hasProperty.GetGetMethod());

+                FrameworkPortability.CreateDelegate(typeof(Func<TMessage, bool>), null, hasProperty.GetGetMethod());

             clearDelegate =

-                (Func<TBuilder, IBuilder>) Delegate.CreateDelegate(typeof(Func<TBuilder, IBuilder>), null, clearMethod);

+                (Func<TBuilder, IBuilder>)FrameworkPortability.CreateDelegate(typeof(Func<TBuilder, IBuilder>), null, clearMethod);

             getValueDelegate = ReflectionUtil.CreateUpcastDelegate<TMessage>(messageProperty.GetGetMethod());

             setValueDelegate = ReflectionUtil.CreateDowncastDelegate<TBuilder>(builderProperty.GetSetMethod());

         }

diff --git a/src/ProtocolBuffers/MessageStreamIterator.cs b/src/ProtocolBuffers/MessageStreamIterator.cs
index f5d9bcbe..89fda91f 100644
--- a/src/ProtocolBuffers/MessageStreamIterator.cs
+++ b/src/ProtocolBuffers/MessageStreamIterator.cs
@@ -38,7 +38,6 @@ using System;
 using System.Collections;

 using System.Collections.Generic;

 using System.IO;

-using System.Reflection;

 

 namespace Google.ProtocolBuffers

 {

@@ -56,122 +55,45 @@ namespace Google.ProtocolBuffers
         private readonly ExtensionRegistry extensionRegistry;

         private readonly int sizeLimit;

 

-        // Type.EmptyTypes isn't present on the compact framework

-        private static readonly Type[] EmptyTypes = new Type[0];

-

         /// <summary>

-        /// Delegate created via reflection trickery (once per type) to create a builder

-        /// and read a message from a CodedInputStream with it. Note that unlike in Java,

-        /// there's one static field per constructed type.

+        /// The default instance of TMessage type used to construct builders while reading

         /// </summary>

-        private static readonly Func<CodedInputStream, ExtensionRegistry, TMessage> messageReader = BuildMessageReader();

-

+        private static readonly TMessage defaultMessageInstance = CreateDefaultInstance();

         /// <summary>

-        /// Any exception (within reason) thrown within messageReader is caught and rethrown in the constructor.

+        /// Any exception (within reason) thrown in type ctor is caught and rethrown in the constructor.

         /// This makes life a lot simpler for the caller.

         /// </summary>

         private static Exception typeInitializationException;

 

+

         /// <summary>

-        /// Creates the delegate later used to read messages. This is only called once per type, but to

-        /// avoid exceptions occurring at confusing times, if this fails it will set typeInitializationException

-        /// to the appropriate error and return null.

+        /// Vastly simplified the reflection to simply obtain the default instance and use it to construct

+        /// the weak builder while simply casting the result.  Ideally this class should have required a 

+        /// TBuilder type argument with a new() constraint to construct the initial instance thereby the

+        /// reflection could be eliminated.

         /// </summary>

-        private static Func<CodedInputStream, ExtensionRegistry, TMessage> BuildMessageReader()

+        private static TMessage CreateDefaultInstance()

         {

             try

             {

-                Type builderType = FindBuilderType();

-

-                // Yes, it's redundant to find this again, but it's only the once...

-                MethodInfo createBuilderMethod = typeof(TMessage).GetMethod("CreateBuilder", EmptyTypes);

-                Delegate builderBuilder = Delegate.CreateDelegate(

-                    typeof(Func<>).MakeGenericType(builderType), null, createBuilderMethod);

-

-                MethodInfo buildMethod = typeof(MessageStreamIterator<TMessage>)

-                    .GetMethod("BuildImpl", BindingFlags.Static | BindingFlags.NonPublic)

-                    .MakeGenericMethod(typeof(TMessage), builderType);

-

-                return (Func<CodedInputStream, ExtensionRegistry, TMessage>) Delegate.CreateDelegate(

-                    typeof(Func<CodedInputStream, ExtensionRegistry, TMessage>), builderBuilder, buildMethod);

+                return (TMessage)typeof(TMessage)

+                                      .GetProperty("DefaultInstance", typeof(TMessage), new Type[0])

+                                      .GetValue(null, null);

             }

-            catch (ArgumentException e)

+            catch (Exception e)

             {

                 typeInitializationException = e;

+                return default(TMessage);

             }

-            catch (InvalidOperationException e)

-            {

-                typeInitializationException = e;

-            }

-            catch (InvalidCastException e)

-            {

-                // Can't see why this would happen, but best to know about it.

-                typeInitializationException = e;

-            }

-            return null;

-        }

-

-        /// <summary>

-        /// Works out the builder type for TMessage, or throws an ArgumentException to explain why it can't.

-        /// </summary>

-        private static Type FindBuilderType()

-        {

-            MethodInfo createBuilderMethod = typeof(TMessage).GetMethod("CreateBuilder", EmptyTypes);

-            if (createBuilderMethod == null)

-            {

-                throw new ArgumentException("Message type " + typeof(TMessage).FullName +

-                                            " has no CreateBuilder method.");

-            }

-            if (createBuilderMethod.ReturnType == typeof(void))

-            {

-                throw new ArgumentException("CreateBuilder method in " + typeof(TMessage).FullName +

-                                            " has void return type");

-            }

-            Type builderType = createBuilderMethod.ReturnType;

-            Type messageInterface = typeof(IMessage<,>).MakeGenericType(typeof(TMessage), builderType);

-            Type builderInterface = typeof(IBuilder<,>).MakeGenericType(typeof(TMessage), builderType);

-            if (Array.IndexOf(typeof(TMessage).GetInterfaces(), messageInterface) == -1)

-            {

-                throw new ArgumentException("Message type " + typeof(TMessage) + " doesn't implement " +

-                                            messageInterface.FullName);

-            }

-            if (Array.IndexOf(builderType.GetInterfaces(), builderInterface) == -1)

-            {

-                throw new ArgumentException("Builder type " + typeof(TMessage) + " doesn't implement " +

-                                            builderInterface.FullName);

-            }

-            return builderType;

-        }

-

-// This is only ever fetched by reflection, so the compiler may

-// complain that it's unused

-#pragma warning disable 0169

-        /// <summary>

-        /// Method we'll use to build messageReader, with the first parameter fixed to TMessage.CreateBuilder. Note that we

-        /// have to introduce another type parameter (TMessage2) as we can't constrain TMessage for just a single method

-        /// (and we can't do it at the type level because we don't know TBuilder). However, by constraining TMessage2

-        /// to not only implement IMessage appropriately but also to derive from TMessage2, we can avoid doing a cast

-        /// for every message; the implicit reference conversion will be fine. In practice, TMessage2 and TMessage will

-        /// be the same type when we construct the generic method by reflection.

-        /// </summary>

-        private static TMessage BuildImpl<TMessage2, TBuilder>(Func<TBuilder> builderBuilder, CodedInputStream input,

-                                                               ExtensionRegistry registry)

-            where TBuilder : IBuilder<TMessage2, TBuilder>

-            where TMessage2 : TMessage, IMessage<TMessage2, TBuilder>

-        {

-            TBuilder builder = builderBuilder();

-            input.ReadMessage(builder, registry);

-            return builder.Build();

         }

-#pragma warning restore 0414

 

         private static readonly uint ExpectedTag = WireFormat.MakeTag(1, WireFormat.WireType.LengthDelimited);

 

         private MessageStreamIterator(StreamProvider streamProvider, ExtensionRegistry extensionRegistry, int sizeLimit)

         {

-            if (messageReader == null)

+            if (ReferenceEquals(defaultMessageInstance, null))

             {

-                throw typeInitializationException;

+                throw new System.Reflection.TargetInvocationException(typeInitializationException);

             }

             this.streamProvider = streamProvider;

             this.extensionRegistry = extensionRegistry;

@@ -224,7 +146,9 @@ namespace Google.ProtocolBuffers
                 {

                     if ((tag == 0 && name == "item") || (tag == ExpectedTag))

                     {

-                        yield return messageReader(input, extensionRegistry);

+                        IBuilder builder = defaultMessageInstance.WeakCreateBuilderForType();

+                        input.ReadMessage(builder, extensionRegistry);

+                        yield return (TMessage)builder.WeakBuild();

                     }

                     else

                     {