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

C# 7.2 in ValueType parameters are not supported with .NET Standard / Core #605

Closed
4 tasks done
stakx opened this issue Mar 13, 2018 · 3 comments · Fixed by #625
Closed
4 tasks done

C# 7.2 in ValueType parameters are not supported with .NET Standard / Core #605

stakx opened this issue Mar 13, 2018 · 3 comments · Fixed by #625

Comments

@stakx
Copy link
Contributor

stakx commented Mar 13, 2018

See this discussion over at DynamicProxy's repo: castleproject/Core#339 (comment).

What needs to be done to add C# 7.2 reference semantics support in Moq for value types is this:

@zvirja
Copy link

zvirja commented Mar 15, 2018

BTW, just noticed that probably you and NSubstitute should also update our delegate generators. We don't copy the required modifiers as well, so in theory if you have delegate with in ValueType, it will fail.

@stakx
Copy link
Contributor Author

stakx commented Mar 15, 2018

@zvirja - Thanks a lot for that hint! True, this needs to be tested as well.

Another thing that comes to mind (which may or may not be relevant to NSubstitute) is that Moq does a lot of DynamicInvoke-ing on delegates, e.g. when you configure a Callback or Returns callback for a method call setup. Not sure at this point whether DynamicInvoke is flexible enough to glance over small differences in the delegate signatures, i.e. when the method being set up has an in parameter but the configured callback doesn't have the in modifier on the parameter. Will need to be tested, too.

Adding these two points to the list above.

@stakx
Copy link
Contributor Author

stakx commented Jun 9, 2018

Add .NET Standard 2.0 as a target framework. As it turns out, it appears that Moq doesn't need to add an additional TFM. The upgrade of Castle.Core to version 4.3.0 should be enough. If user code targets a platform that supports .NET Standard 1.5 (or newer), then the .NET Standard 1.5 version of Castle.Core will be selected even though Moq (which sits between Castle.Core and the targeted platform) only has a .NET Standard 1.3 assembly.

Possibly update delegate generator logic. Added some tests to verify delegate mocking works in the presence of in parameters. It seems for once, System.Reflection.Emit just does what it's supposed to.

Check whether DynamicInvoke e.g. for Callback and Returns is already lenient enough given differences in delegate signatures. Ran some quick successful tests; given that DynamicInvoke already worked with differences in ref / no-ref, perhaps it's not too surprising that in appears to be supported just fine, too.

@stakx stakx closed this as completed in #625 Jun 9, 2018
@devlooped devlooped locked and limited conversation to collaborators Sep 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants