376792 - Patch for array conversion

Status: RESOLVED INACTIVE

(Rhino Graveyard :: Core, enhancement)

Description

[Jürg Lehni]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en) AppleWebKit/419 (KHTML, like Gecko) Safari/419.3
Build Identifier: 

This patch adds more distinction when determining the conversion weight of arrays in NativeJavaObject. Basically it looks at the first element of the array, and compares ith with the component type of the java array.

This decreases the amount of cases where the exact version of the method needs to be picked. The only situation where it would produce wrong results is with arrays containing null or undefined at the first index, or arrays containing values of different types.


Reproducible: Always

Comment 1

[Jürg Lehni]

Attachment: array conversion patch

Comment 2

[Attila Szegedi]

This looks okay to me -- Norris might provide additional insight into whether there are any pitfalls with this approach; if not, I'll commit it soon.

Comment 3

[Norris Boyd]

This looks like a nice addition we should make to LiveConnect. However, it is at odds with the LC3 spec. I'd like to consider a broader examination of LiveConnect limitations for the next release, and as a major version change it would be a better time to land a change to the overloading behavior.

Comment 4

[Jürg Lehni]

Thank you for your feedback, Norris. I thought that it would break LC3, but was not sure if that would be an issue. No problem with waiting for 1.7 on my side. Until then I will continue to use my patched version.

Comment 5

[Attila Szegedi]

I think it's safe to go beyond LC3 -- it's not exactly set in stone, and there's no real reason to keep to it indefinitely. Actually, we've already done that. The recently committed patch from another issue (don't have time to lookup the number now) added support for variable argument methods (which didn't exist at the time LC3 was established) and the much needed treatment of boolean method parameters as equal to other primitive types.

Comment 7

[Thomas Chust]

(In reply to comment #6)
> *** Bug 380032 has been marked as a duplicate of this bug. ***

I just pulled a fresh CVS version of Rhino, applied the above patch and bug 380032 unfortunately still persists :-(

Comment 8

[Sylvestre Ledru [:Sylvestre]]

Closing. Bug management is now done here:
https://github.com/mozilla/rhino