255549 - JVM-dependent resolution of ambiguity when calling Java methods

Status: VERIFIED FIXED

(Rhino Graveyard :: Core, defect)

Description

[Igor Bukanov]

As of 2004-08-13 when calling overloaded Java methods Rhino does not implement
proper ambiguity resolution as specified by LC3 specs, 
http://www.mozilla.org/js/liveconnect/lc3_method_overloading.html . 

Consider the following Java class which is based on example provided by Merten
Schumann:

public class X
{
	public static class Order1
	{
		public static int test(String[] array1, String[] array2, String s)
		{
			return 1;
		}

		public static int test(String s1, String s2, String s)
		{
			return 2;
		}
	}

	public static class Order2
	{
		public static int test(String s1, String s2, String s)
		{
			return 2;
		}

		public static int test(String[] array1, String[] array2, String s)
		{
			return 1;
		}

	}

}

According to LC3 the following calls

Packages.X.Order1.test("x", null, null)
Packages.X.Order2.test("x", null, null)

should call test(String, String, String) of X.Order1 or X.Order2 correspondingly
since test(String[], String[], String) is not applicable. But depending on
compiler and JVM in Rhino either the first line or the second line produces:

js: "test.js", line 1: The choice of Java constructor test matching JavaScript
argument types (string,null,null) is ambiguous; candidate constructors are: int
test(java.lang.String[],java.lang.String[],java.lang.String), int
test(java.lang.String,java.lang.String,java.lang.String)

Comment 1

[Igor Bukanov]

Attachment: Fix: proper ambiguity resolution

The reason for the bug is that NativeJavaMethod.findFunction tested the method
applicability only if the search loop did not see any ambiguity yet. After that
it would check methods only against ambiguity. Thus if JVM returns for X.Other
classes:

test(String[],String[],String)
test(String,String,String)

then code will reject the first as inapplicable and choose the second case. If
the order would be

test(String,String,String)
test(String[],String[],String)

then code will picks up first method as applicable and then see ambiguity when
calling second and report error.

The patch fixes that and changes loop to track all currently best fitted
methods instead of doing a separated search after.

Comment 2

[Igor Bukanov]

Attachment: Fix for the last fix

The previous patch contained broken counting of unprefered methods:
   --worseCounr;

That should be 
    ++worseCount;

Comment 3

[Igor Bukanov]

I committed the fix.

Comment 4

[Merten Schumann]

Many thanx for investigating/fixing this issue, Igor!!! :-)
Will try it with next Rhino release ...
   Merten

Comment 5

[Igor Bukanov]

Marking as verified since Merten Schumann confirmed that fix resolved original
problem.