[mono][System.Reflection.Emit] Fix explicitly setting the return type on a dynamic method #95183
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes the support of
SetReturnType
andSetParameters
in theMethodBuilder
API with Mono, by properly setting the return type of the dynamic method only if it is non null value.This was manifested in two scenarios:
If the dynamic method return and parameter types are manually set in the following sequence:
The subsequent call to
SetParameters
(which internally callsSetSignatureCore
) would overwrite the set return type tonull
causing the method not to be considered as an implementation of a interface method. This scenario is used in https://github.com/dotnet/aspnetcore/blob/4dc81f80abd45cb5c2e04d3b742bc6755c1153ee/src/SignalR/server/Core/src/Internal/TypedClientBuilder.cs#L136-L137 causing ASP.NET test failures when using Mono.Setting explicit
null
as a return type viaSetReturnType
overwrites the default return type value which should betypeof(void)
This scenario was covered with
System.Reflection.Emit
libraries testsruntime/src/libraries/System.Reflection.Emit/tests/MethodBuilder/MethodBuilderSetReturnType.cs
Lines 141 to 143 in 4fa760f
Verification
This fix was verified by enabling the disabled unit test (and verifying locally that it passes) and manually verifying that the repro-iclientproxy-vtable-fail as described in #94490 (comment) does not crash.
Fixes #94490