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

Let InnerMockSetup (i.e. internal setups for cached return values) match generic args exactly instead of by assignment compatibility. #949

Merged
merged 4 commits into from
Oct 19, 2019

Conversation

stakx
Copy link
Contributor

@stakx stakx commented Oct 19, 2019

Resolves #932.

As mentioned in one commit message, I am not convinced this is a 100% correct bug fix... it could even introduce new problems. It would benefit from some additional thinking about how Moq should generally handle co-/contravariance when matching setups for generic methods.

/cc @BrunoJuchli

While this fixes the regression documented in the previous commit, it
might be an insufficent solution in general. Perhaps it would be more
correct to take co-/contra-variance into account and select a compari-
son mode per generic argument depending on whether it appears in a
input and/or output position?
@stakx stakx added this to the 4.13.1 milestone Oct 19, 2019
@stakx
Copy link
Contributor Author

stakx commented Oct 19, 2019

Going to merge this despite my reservations that I mentioned above. In the worst case, the change being made here means some additional "cache" misses, i.e. Moq will at worst produce new return values instead of returning the previous one. Other parts of Moq shouldn't be affected by this change.

@stakx stakx merged commit 9ff1cd7 into devlooped:master Oct 19, 2019
@stakx stakx deleted the issue-932 branch October 19, 2019 12:53
@BrunoJuchli
Copy link

BrunoJuchli commented Oct 19, 2019

@stakx
Thanks a lot! I should be able to try it out on Thursday afternoon.

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.

Regression in 4.13.0
2 participants