Copy default value back when Missing.Value is provided as argument #73670
Conversation
ghost
added
the
|
Tagging subscribers to this area: @dotnet/area-system-reflection Issue DetailsFix Missing.Value corner-case behavior changes in .NET 7 In .NET 6 and below all arguments were copied back Later in .NET 7 it seems updated to copy only if ByRef are present as mentioned in the comment, this is causing a regression when Fixes #73106
|
src/libraries/System.Private.CoreLib/src/System/Reflection/MethodBase.cs
Outdated
Show resolved
Hide resolved
src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.CoreCLR.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Reflection/MethodBase.cs
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| arg = paramInfo.DefaultValue; | ||
| parameters[i] = arg = paramInfo.DefaultValue; |
There was a problem hiding this comment.
It would be nice to do the parameters[i] assignment only once in all cases, ie do it in the fast path and in the slow path right before the CheckValue check.
BTW: The value of the fast path for default values is questionable. Reading of the default value is slow, and so we gaining very little from this fast path. You can also just delete the fast path here.
There was a problem hiding this comment.
The check for "fast path" was actually required since the "slow path" did not re-check this and would have failed in Debug build due to an Assert.
The later refactor commit changed the order and now the "fast path" can occur naturally after the Type.Missing fix-ups.
There was a problem hiding this comment.
he check for "fast path" was actually required since the "slow path" did not re-check this and would have failed in Debug build due to an Assert.
The assert in the slow path is broken. It should be removed. It is what #73103 is about.
|
The failure looks infra related, merging |
3 participants
Fix Missing.Value corner-case behavior changes in .NET 7
In .NET 6 and below all arguments were copied back
runtime/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.cs
Lines 437 to 440 in 510444c
Later in .NET 7 it seems updated to copy only if ByRef are present as mentioned in the comment, this is causing a regression when
Missing.Valueis provides as argument and default value used to written back instead ofMissing.ValueFixes #73106