The ConstructorBuilder and MethodBuilder generated parameter should not has default value by default.

[edwardmeng]

The HasDefaultValue of parameters should be false by default, for the constructors and methods generated from System.Reflection.Emit.ConstructorBuilder and System.Reflection.Emit.MethodBuilder. But actually the HasDefaultValue of the parameters are true. It means the emitter generates parameter default values as null unexpected. The sample code as following:

var builder = AppDomain.CurrentDomain.DefineDynamicAssembly(new AssemblyName("DynamicProxyGenAssembly"), AssemblyBuilderAccess.Run).DefineDynamicModule("DynamicProxyGenAssembly");
var type = builder.DefineType("MyProxy", TypeAttributes.Public);

var constructorBuilder = type.DefineConstructor(MethodAttributes.Public, CallingConventions.Standard, new[] {typeof(Version)});
var il = constructorBuilder.GetILGenerator();
il.Emit(OpCodes.Ret);

var methodBuilder = type.DefineMethod("DoSomething", MethodAttributes.Public, CallingConventions.Standard, typeof(void),new[] {typeof(Version)});
il = methodBuilder.GetILGenerator();
il.Emit(OpCodes.Ret);

var typeInfo = type.CreateTypeInfo();
var constructor = typeInfo.GetConstructor(new[] {typeof(Version) });
var parameters = constructor.GetParameters();
Assert.False(parameters[0].HasDefaultValue);  // Fails

var method = typeInfo.GetMethod("DoSomething", new[] {typeof(Version)});
parameters = method.GetParameters();
Assert.False(parameters[0].HasDefaultValue); // Fails

But the DynamicMethod has the correct parameter default value as expected. The sample code as following:

var method = new DynamicMethod("x", typeof(void), new [] { typeof(Version) });
var il = method.GetILGenerator();
il.Emit(OpCodes.Ret);
var delegateMethod = method.CreateDelegate(typeof(Action<Version>));
var parameters = delegateMethod.GetMethodInfo().GetParameters();
Assert.False(parameters[0].HasDefaultValue); // Success

[edwardmeng]

It can cause some wired behaviors for the Autofac integrating with DynamicProxy2 when the dependency is not registered. Since there is a class just make use of it.
https://github.com/autofac/Autofac/blob/41044d7d1a4fa277c628021537d5a12016137c3b/src/Autofac/Core/Activators/Reflection/DefaultValueParameter.cs

[danmoseley]

@joshfree can you please triage and close/move to future/2.1 or assign for ZBB

[steveharter]

I agree that HasDefaultValue==false would be the better default in order to distinguish between an explicit default value and an implied default value other than null.

If the default value is changed to false I don't think this would be a breaking change to consumers of the IL since a null value for the DefaultProperty property seems to work fine for value types (i.e. no exception or apparent IL issue).

For example, changing the same code object to use a non-nullable int instead of Version (a reference type) works fine:

            var methodBuilder = type.DefineMethod("DoSomething", MethodAttributes.Public, CallingConventions.Standard, typeof(int), new[] { typeof(int) });
            il = methodBuilder.GetILGenerator();
            il.Emit(OpCodes.Ldarg_1);
            il.Emit(OpCodes.Ret);

along with consuming code that would try to use the default value (null) as the argument

            var typeInfo = type.CreateTypeInfo();
            var constructor = typeInfo.GetConstructor(new[] { typeof(int) });
            var parameters = constructor.GetParameters();

            var method = typeInfo.GetMethod("DoSomething", new[] { typeof(int) });
            parameters = method.GetParameters();

            Type? realType = type.CreateType();
            object obj = Activator.CreateInstance(realType, new object[] { null });

            MethodInfo mi = realType.GetMethod("DoSomething");
            object defaultValue = mi.GetParameters()[0].DefaultValue;
            // defaultValue is null
            int myInt = (int)mi.Invoke(obj, new object[] { defaultValue });
            // myInt is 0 even though the default value is "null"

cc @GrabYourPitchforks

[steveharter]

Moving to future based on priority + schedule.

[steveharter]

Moving to 8.0; seems like it should be a trivial fix.