The ConstructorBuilder and MethodBuilder generated parameter should not have default value by default

[pedrobsaila]

Fixes #20909

[ghost]

Tagging subscribers to this area: @dotnet/area-system-reflection-emit
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Fixes #20909

Author:	pedrobsaila
Assignees:	-
Labels:	area-System.Reflection.Emit
Milestone:	-

[buyaa-n]

Could not think of any case where return parameter could have a default value, double checking with the area experts CC @AaronRobinsonMSFT @jkotas @steveharter

[buyaa-n]

Tagging mono folks @lambdageek @marek-safar @vargaz

[marek-safar]

Why is the extra if needed?

[pedrobsaila]

Because GetDefaultValue returns null instead of DBNull.Value/Type.Missing, which makes HasDefaultValue return true for return parameters. Do we consider a return parameter in mono to have default values for a valuable reason or it is just an error ?

[marek-safar]

I think better fix would be to update add_parameter_object_to_array

[pedrobsaila]

I don't think it would fix the probem. The add_parameter_object_to_array processes only input parameters. In fact, The ReturnParameter is constructed here :

runtime/src/mono/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.Mono.cs

Lines 135 to 138 in bc76652

	internal static ParameterInfo GetReturnParameterInfo(RuntimeMethodInfo method)
	{
	return RuntimeParameterInfo.New(GetReturnType(method.mhandle), method, get_retval_marshal(method.mhandle));
	}

Mono does not provide DefaultValueImpl to RuntimeParameterInfo as you can see, we can add a new method in mono like get_retval_marshal that return a hardcoded DbNull.Value or do it directly in the dedicated constructor:

runtime/src/mono/System.Private.CoreLib/src/System/Reflection/RuntimeParameterInfo.cs

Lines 113 to 122 in bc76652

	/* to build a ParameterInfo for the return type of a method */
	internal RuntimeParameterInfo(Type type, MemberInfo member, MarshalAsAttribute marshalAs)
	{
	this.ClassImpl = type;
	this.MemberImpl = member;
	this.NameImpl = null;
	this.PositionImpl = -1; // since parameter positions are zero-based, return type pos is -1
	this.AttrsImpl = ParameterAttributes.Retval;
	this.marshalAs = marshalAs;
	}

Let me know which one do you prefer

[buyaa-n]

Setting the default DbNull.Value directly in the dedicated constructor sounds reasonable to me, ping @marek-safar for opinion

[buyaa-n]

LGTM, thanks @pedrobsaila

[buyaa-n]

The failure unrelated and known

[steveharter]

Breaking change: dotnet/docs#36725