Skip to content

Conversation

@edparcell
Copy link

I noticed that when I created a proxy for an object with a method which takes a 2-dimensional array parameters that the proxy created ok, but when I called the method, that there was an exception due to the proxy method being called with parameter type T[] rather than T[,].

This pull request strengthens the ProxyTypeWithMultiDimentionalArrayAsParameter unit test by actually calling each of the methods of the test class. TypeUtil.GetClosedParameterType is altered so that it returns the correct type for multi-dimensional arrays and the unit test passes. I think that all other unit tests continue to pass.

Hope this helps, Ed.

edparcell@gmail.com added 3 commits March 18, 2014 20:49
@kkozmic
Copy link
Contributor

kkozmic commented Mar 19, 2014

Hmm, I think there was a bug in Reflection Emit around multidimensional arrays.

Does that mean that's been fixed now, in latest version of .NET ?

@edparcell
Copy link
Author

I'm not sure. I know that this fix works for a simple use case I set up,
similar to the unit test here.

On 18 March 2014 21:05, Krzysztof Koźmic notifications@github.com wrote:

Hmm, I think there was a bug in Reflection Emit around multidimensional
arrays.

Does that mean that's been fixed now, in latest version of .NET ?


Reply to this email directly or view it on GitHubhttps://github.com//pull/47#issuecomment-38007726
.

@kkozmic
Copy link
Contributor

kkozmic commented Mar 19, 2014

have you tried that on older versions of .NET? version 4 or 3.5 specifically?

@edparcell
Copy link
Author

I tried on .NET 4 targets with Visual Studio 2012 and 2013. Visual 2012 with the unit test here, where the test method doesn't do anything. Visual 2013 with another simple use case, where I summed 1- and 2-dimensional arrays. In either case, I think the exception was when calling the method, rather than running it or dealing with the return.

@kkozmic
Copy link
Contributor

kkozmic commented Mar 19, 2014

@kkozmic
Copy link
Contributor

kkozmic commented Mar 19, 2014

with that change, does the BasicClassProxyTestCase.ProxyTypeWithMultiDimentionalArrayAsParameters() test work?

@edparcell
Copy link
Author

I just found http://support.microsoft.com/kb/960240 which is referenced by the link you gave. It's marked as applying to .NET 2.0-3.5SP1, with a note that "Microsoft has recognized this problem, and it is currently planned that a future release of the .NET Framework will include a System.Reflection.Emit namespace that will have corrected this." but no indication whether that happened, and last review in Nov 2008.

@edparcell
Copy link
Author

Yes, with the change, all unit tests passed for me.

@edparcell
Copy link
Author

All unit tests pass with NET45-Debug target I should say.

@kkozmic
Copy link
Contributor

kkozmic commented Mar 19, 2014

nice, that likely does mean that they've fixed it... :)

@edparcell
Copy link
Author

Fingers crossed...

FWIW, I just ran with NET35-Debug from VS2012 with this pull request to see if I could replicate the bug and the test passed - I guess MS didn't favour backward compatibility for that bug.

@kkozmic
Copy link
Contributor

kkozmic commented Mar 19, 2014

ha interesting.

Do you mind unignoring the test then?

kkozmic added a commit that referenced this pull request Mar 19, 2014
Caliing DynamicProxy proxy methods with multidimensional array parameters
@kkozmic kkozmic merged commit acf533f into castleproject:master Mar 19, 2014
@edparcell
Copy link
Author

Oh, wow.. it auto-added the new commit to the pull request and you merged while I was trying to figure out how to add the commits. That's great - thanks very much. And thanks for such a useful project. All the best, Ed.

@kkozmic
Copy link
Contributor

kkozmic commented Mar 19, 2014

hehe, it just works :)

thanks for the PR. We should be doing a release either later this month, or early next month.

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.

2 participants