Fix calling existing ctor with MethodInvoker; share tests with invokers#90796
Fix calling existing ctor with MethodInvoker; share tests with invokers#90796steveharter merged 2 commits intodotnet:mainfrom
Conversation
|
Tagging subscribers to this area: @dotnet/area-system-reflection Issue Details[verifying tests]
|
|
/azp run runtime-extra-platforms |
|
Azure Pipelines successfully started running 1 pipeline(s). |
jkotas
left a comment
There was a problem hiding this comment.
LGTM (Please check that the affected tests are passing in the extra-platforms legs before merging.)
|
/azp run runtime-extra-platforms |
|
Azure Pipelines successfully started running 1 pipeline(s). |
src/libraries/System.Private.CoreLib/src/System/Reflection/MethodInvokerCommon.cs
Outdated
Show resolved
Hide resolved
|
/azp run runtime-extra-platforms |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
CI failures in runtime-extra-platforms appear consistent with other recent PRs including #90836 and #90864. Created some known issues to help filter the errors in the future. Also, there's some infra work needed so failures running runtime-extra-platforms show all of the unknown failures (currently just the first 4 are shown). |
|
/backport to release/8.0 |
1 similar comment
|
/backport to release/8.0 |
|
Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/5945904726 |
Refactored the existing
MethodInfo\ConstructorInfotests so they can also run under the new invoker classesMethodInvoker\ConstructorInvoker; they are run in both a forced-interpreted and a forced-emit mode for additional coverage.This uncovered two edge case issues (fixed in this PR) that only affect the new invoker classes that were added in Preview 7:
MethodInvokernow works as expected: theobjparameter's constructor is invoked andnullis returned; this is consistent withConstructorInfo.Invoke(obj, ...)MemberAccessException(same exception type used for similar cases). Calling static constructors can still be done withConstructorInfobut since that is rare, adds overhead, and is not supported on NativeAOT, support wasn't added toMethodInvokerorConstructorInvoker.No tests were removed except for the invoker test "ExistingInstance()" which was already covered in the shared tests.
These changes should be ported to v8\RC2 since they only affect the new APIs added in Preview 7 and will avoid a breaking change in v9.