Fix DispatchProxy not working with in parameters #49214
Conversation
src/libraries/System.Reflection.DispatchProxy/src/System/Reflection/DispatchProxyGenerator.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Christopher Watford <[email protected]>
| MethodBuilder mdb = _tb.DefineMethod(mi.Name, MethodAttributes.Public | MethodAttributes.Virtual, CallingConventions.HasThis, | ||
| returnParameter.ParameterType, returnReqMods, returnOptMods, | ||
| paramTypes, paramReqMods, paramOptMods); |
There was a problem hiding this comment.
Thanks, @wzchua , looks good to me.
To avoid side effects I would pass null for unnecessary parameters, plus i believe it's better to use the CallingConventions.Standard which was used by default in the previous call
| MethodBuilder mdb = _tb.DefineMethod(mi.Name, MethodAttributes.Public | MethodAttributes.Virtual, CallingConventions.HasThis, | |
| returnParameter.ParameterType, returnReqMods, returnOptMods, | |
| paramTypes, paramReqMods, paramOptMods); | |
| MethodBuilder mdb = _tb.DefineMethod(mi.Name, MethodAttributes.Public | MethodAttributes.Virtual, CallingConventions.Standard, | |
| mi.ReturnType, null, null, | |
| paramTypes, paramReqMods, null); |
There was a problem hiding this comment.
ok. I thought it would be more correct for all the mods to be included.
I made a mistake with the CallingConventions.
There was a problem hiding this comment.
ok. I thought it would be more correct for all the mods to be included.
Understand your motive, but adding those could affect performance, as we don't have a known issue with not adding them I prefer keep the existing behavior
| ParameterInfo returnParameter = mi.ReturnParameter; | ||
| Type[] returnReqMods = returnParameter.GetRequiredCustomModifiers(); | ||
| Type[] returnOptMods = returnParameter.GetOptionalCustomModifiers(); |
There was a problem hiding this comment.
Please remove all unneeded variables here and above
| ParameterInfo returnParameter = mi.ReturnParameter; | |
| Type[] returnReqMods = returnParameter.GetRequiredCustomModifiers(); | |
| Type[] returnOptMods = returnParameter.GetOptionalCustomModifiers(); |
Removed unneeded mods
| @@ -394,9 +394,19 @@ internal void AddInterfaceImpl([DynamicallyAccessedMembers(DynamicallyAccessedMe | |||
| private MethodBuilder AddMethodImpl(MethodInfo mi, int methodInfoIndex) | |||
| { | |||
| ParameterInfo[] parameters = mi.GetParameters(); | |||
| Type[] paramTypes = ParamTypes(parameters, false); | |||
| Type[] paramTypes = new Type[parameters.Length]; | |||
There was a problem hiding this comment.
Is ParamTypes still used anywhere?
There was a problem hiding this comment.
It is used on this line but it seems that it is also not needed there. I will create a PR for that.
|
I've closed, reopened to restart the CI |
ghost
locked as resolved and limited conversation to collaborators
Resolves #47522