Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve support for covariant returns (where generic types are involved) #643

Merged
merged 9 commits into from
Dec 29, 2022

Conversation

stakx
Copy link
Member

@stakx stakx commented Dec 28, 2022

Fixes #632.

It turns out that we only added support for non-generic covariant return types in #619; however there are two additional cases that we need to support, too:

  • covariant return types where one of them is generic but the other is not
  • covariant return types where both of them are generic

(I'm counting those as two distinct cases due to the code structure found inside MethodSignatureComparer.EqualSignatureTypes, which is the method that needs to be augmented.)

@stakx stakx added this to the vNext milestone Dec 28, 2022
@stakx stakx requested a review from jonorossi December 28, 2022 23:05
@stakx
Copy link
Member Author

stakx commented Dec 28, 2022

@jonorossi, I'd appreciate your input on one particular detail here. In principle, this PR is an extension of #619, whose code patterns it duplicates for the most part. After this PR, the IsCovariantReturnTypes check will happen in three code branches inside MethodSignatureComparer.EqualSignatureTypes, instead of just in one. I've tried to keep those checks out of the "hot" code paths as much as possible in order to not affect runtime performance. If you have any suggestions how they could be relegated to run even less frequently, I'd be interested to hear them.

@stakx stakx force-pushed the bug/covariant-returns branch 2 times, most recently from ef923a1 to 189dd28 Compare December 29, 2022 11:49
Performing this check for anything other than return types is basically
wasted time, so moving it to `EqualReturnTypes` means it will only run
when actually needed.
@stakx stakx merged commit 18ddd62 into castleproject:master Dec 29, 2022
@stakx stakx deleted the bug/covariant-returns branch December 29, 2022 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proxies using Records derived from a base generic record broken using .NET 6 compiler
2 participants