Skip to content

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

Merged
stakx merged 9 commits intocastleproject:masterfrom
stakx:bug/covariant-returns
Dec 29, 2022
Merged

Improve support for covariant returns (where generic types are involved)#643
stakx merged 9 commits intocastleproject:masterfrom
stakx:bug/covariant-returns

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 from 6aec65a to 33fa217 Compare December 29, 2022 11:38
@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 force-pushed the bug/covariant-returns branch from 189dd28 to 6b16135 Compare December 29, 2022 11:51
@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