Fix for issue #87 #91
Conversation
…d parameter of type decimal or decimal? causes an exception).
| * A decimal value may not be passed to SetConstant(), so omit the call | ||
| * in this instance. | ||
| * | ||
| * See https://github.com/castleproject/Core/issues/87 and https://bit.ly/1eXx6fF |
There was a problem hiding this comment.
I know the background on the issue, but it'd be nicer if the comment was a bit clearer that this is a defect/omission in .NET so in the future when the .NET defect is fixed someone can remove this code without worry in case our GitHub issues history isn't around anymore.
Could you also include the full URL for the MSDN link rather than a bit.ly link:
https://msdn.microsoft.com/en-au/library/system.reflection.emit.parameterbuilder.setconstant(v=vs.110).aspx
|
Looks good, thanks. Did you try compiling this with .NET 3.5 because I suspect it'll fail because optional parameters weren't supported then? You'll likely need to wrap the new code in an |
- Changed comment to clarify that the fix is necessary because of a shortcoming of the .NET Framework. - Added compiler flags to make sure the new tests (and classes & interfaces) are only included in case the .NET Framework version is not set to 3.5.
|
I was able to compile the code using Visual Studio even with the NET35 configurations, but according to this link this may not work using a command-line compiler, so I have wrapped the test code in an Additionally, I modified the comment as you suggested. |
|
@fknx thanks |
|
The Silverlight builds failed in the same way the .NET 3.5 one would have, but don't worry I've fixed them in master. |
I have added an
ifstatement which checks whether the parameter type is adecimalordecimal?beforeSetConstantis called. I also added two simple test cases where anInterfaceandClassproxy is created for anInterface(orClass) containing methods with all kinds of optional parameters.