RFR: 8062389,8029459,8061950: Class.getMethod() is inconsistent with Class.getMethods() results + more

[Peter Levart]

Hi Daniel.


On 10/11/2016 05:19 PM, Daniel Fuchs wrote:
> Hi Peter,
>
> I was looking at the test - I didn't see a case where the
> same method would be declared by two unrelated interfaces
> later implemented by the same class.
> Do we have test cases to verify that:
>
> public interface I1 {
>     public Object test(String blah);
> }
> public interface I2 {
>     public Object test(String blah);
> }
> public abstract class A implements I2 {}
> public abstract class A1 extends A implements I1, I2 {}
> public abstract class A2 implements I1, I2 {}
> public abstract class A3 implements I2, I1 {}
>
> A1.getMethods(), A2.getMethods(), A3.getMethods() return
> the expected result?
> In particular what should A2.class.getMethods()
> and A3.class.getMethods() return?

import java.lang.reflect.Method;

public class Test {

     static String toMethodsString(Class<?> clazz) {
         StringBuilder sb = new StringBuilder();
         sb.append(clazz.getName()).append(": [");
         boolean first = true;
         for (Method m : clazz.getMethods()) {
             if (m.getDeclaringClass() != Object.class) {
                 if (first)
                     first = false;
                 else
                     sb.append(", ");
                 sb.append(m);
             }
         }
         sb.append("]");
         return sb.toString();
     }

     public static void main(String[] args) throws Exception {
         for (Class<?> clazz : new Class[]{A1.class, A2.class, A3.class}) {
             System.out.println(toMethodsString(clazz));
         }
     }
}

interface I1 {
     Object test(String blah);
}

interface I2 {
     Object test(String blah);
}

abstract class A implements I2 {}

abstract class A1 extends A implements I1, I2 {}

abstract class A2 implements I1, I2 {}

abstract class A3 implements I2, I1 {}



Above program, ignoring Object methods, executed with JDK 7, 8 and 
9+patch, returns the same results:

[peter at cube jdk9-dev-test]$ ~/Apps64/jdk1.7.0/bin/java -cp out Test

A1: [public abstract java.lang.Object I2.test(java.lang.String), public 
abstract java.lang.Object I1.test(java.lang.String)]
A2: [public abstract java.lang.Object I1.test(java.lang.String), public 
abstract java.lang.Object I2.test(java.lang.String)]
A3: [public abstract java.lang.Object I2.test(java.lang.String), public 
abstract java.lang.Object I1.test(java.lang.String)]

[peter at cube jdk9-dev-test]$ ~/Apps64/jdk1.8.0/bin/java -cp out Test

A1: [public abstract java.lang.Object I2.test(java.lang.String), public 
abstract java.lang.Object I1.test(java.lang.String)]
A2: [public abstract java.lang.Object I1.test(java.lang.String), public 
abstract java.lang.Object I2.test(java.lang.String)]
A3: [public abstract java.lang.Object I2.test(java.lang.String), public 
abstract java.lang.Object I1.test(java.lang.String)]

[peter at cube jdk9-dev-test]$ 
~/work/hg/jdk9-dev/build/linux-x86_64-normal-server-release/images/jdk/bin/java 
-cp out Test

A1: [public abstract java.lang.Object I2.test(java.lang.String), public 
abstract java.lang.Object I1.test(java.lang.String)]
A2: [public abstract java.lang.Object I1.test(java.lang.String), public 
abstract java.lang.Object I2.test(java.lang.String)]
A3: [public abstract java.lang.Object I2.test(java.lang.String), public 
abstract java.lang.Object I1.test(java.lang.String)]

...which is expected.

In above example, two abstract methods with same signature are inherited 
from two unrelated interfaces, but they are not later implemented by a 
class. If it was implemented (or overridden by an abstract class 
method), only the class method would show.

I agree that the test is not very systematic in covering all possible 
combinations. Maybe it is time to write new test. Let me see...

Regards, Peter

>
> best regards,
>
> -- daniel
>
> On 10/10/16 09:04, Peter Levart wrote:
>> Just a note that this is still ready to be reviewed.
>>
>> I was also told that JCK tests pass with the patch applied.
>>
>> Regards, Peter
>>
>> On 10/04/2016 03:40 PM, Peter Levart wrote:
>>> Hi,
>>>
>>> I have a fix for conformance (P2) bug (8062389
>>> <https://bugs.openjdk.java.net/browse/JDK-8062389>) that also fixes a
>>> conformance (P3) bug (8029459
>>> <https://bugs.openjdk.java.net/browse/JDK-8029459>) and a performance
>>> issue (8061950 <https://bugs.openjdk.java.net/browse/JDK-8061950>):
>>>
>>> http://cr.openjdk.java.net/~plevart/jdk9-dev/Class.getMethods.new/webrev.04/ 
>>>
>>>
>>>
>>> As Daniel Smith writes in 8029459
>>> <https://bugs.openjdk.java.net/browse/JDK-8029459>, the following
>>> situation is as expected:
>>>
>>> interface I { void m(); }
>>> interface J extends I { void m(); }
>>> interface K extends J {}
>>> K.class.getMethods() = [ J.m ]
>>>
>>> But the following has a different result although it should probably
>>> have the same:
>>>
>>> interface I { void m(); }
>>> interface J extends I { void m(); }
>>> interface K extends I, J {}
>>> K.class.getMethods() = [ I.m, J.m ]
>>>
>>> He then suggests the following algorithm:
>>>
>>> An implementation of getMethods consistent with JLS 8 would include
>>> the following (see Lambda Spec, Part H, 9.4.1 and 8.4.8):
>>> 1) The class's/interface's declared (public) methods
>>> 2) The getMethods() of the superclass (if this is a class), minus any
>>> that have a match in (1)
>>> 3) The getMethods() of each superinterface, minus any that have a
>>> match in (1) or a _concrete_ match in (2) or a match from a
>>> more-specific class/interface in (2) or (3)
>>>
>>> But even that is not sufficient for the following situation:
>>>
>>> interface E { void m(); }
>>> interface F extends E { void m(); }
>>> abstract class G implements E {}
>>> abstract class H extends G implements F {}
>>> H.class.getMethods() = [ E.m, F.m ]
>>>
>>> The desired result of H.class.getMethods() = [ F.m ]
>>>
>>> So a more precise definition is required which is implemented in the 
>>> fix.
>>>
>>> The getMethods() implementation partitions the union of the following
>>> methods:
>>>
>>> 1) The class's/interface's declared public methods
>>> 2) The getMethods() of the superclass (if this is a class)
>>> 3) The non-static getMethods() of each direct superinterface
>>>
>>> ...into equivalence classes (sets) of methods with same signature
>>> (return type, name, parameter types). Within each such set, only the
>>> "most specific" methods are kept and others are discarded. The union
>>> of the kept methods is the result.
>>>
>>> The "more specific" is defined as a partial order within a set of
>>> methods of same signature:
>>>
>>> Method A is more specific than method B if:
>>> - A is declared by a class and B is declared by an interface; or
>>> - A is declared by the same type as or a subtype of B's declaring type
>>> and both are either declared by classes or both by interfaces (clearly
>>> if A and B are declared by the same type and have the same signature,
>>> they are the same method)
>>>
>>> If A and B are distinct elements from the set of methods with same
>>> signature, then either:
>>> - A is more specific than B; or
>>> - B is more specific than A; or
>>> - A and B are incomparable
>>>
>>> A sub-set of "most specific" methods are the methods from the set
>>> where for each such method M, there is no method N != M such that N is
>>> "more specific" than M.
>>>
>>> There can be more than one "most specific" method for a particular
>>> signature as they can be inherited from multiple unrelated interfaces,
>>> but:
>>> - javac prevents compilation when among multiply inherited methods
>>> with same signature there is at least one default method, so in
>>> practice, getMethods() will only return multiple methods with same
>>> signature if they are abstract interface methods. With one exception:
>>> bridge methods that are created by javac for co-variant overrides are
>>> default methods in interfaces. For example:
>>>
>>>     interface I { Object m(); }
>>>     interface J1 extends I { Map m(); }
>>>     interface J2 extends I { HashMap m(); }
>>>     interface K extends J1, J2 {}
>>>
>>> K.class.getMethods() = [ default Object j1.m, abstract Map j1.m,
>>> default Object j2.m, abstract HashMap j2.m ]
>>>
>>> But this is an extreme case where one can still expect multiple
>>> non-abstract methods with same signature, but different declaring
>>> type, returned from getMethods().
>>>
>>> In order to also fix 8062389
>>> <https://bugs.openjdk.java.net/browse/JDK-8062389>, getMethod() and
>>> getMethods() share the same consolidation logic that results in a set
>>> of "most specific" methods. The partitioning in getMethods() is
>>> partially performed by collecting Methods into a HashMap where the
>>> keys are (name, parameter types) tuples and values are linked lists of
>>> Method objects with possibly varying return and declaring types. The
>>> consolidation, i.e. keeping only the set of most specific methods as
>>> new methods are encountered, is performed only among methods in the
>>> list that share same return type and also removes duplicates.
>>> getMethod() uses only one such list, consolidates methods that match
>>> given name and parameter types and returns the 1st method from the
>>> resulting list that has the most specific return type.
>>>
>>> That's it for algorithms used. As partitioning partially happens using
>>> HashMap with (name, parameter types) keys, lists of methods that form
>>> values are typically kept short with most of them containing only a
>>> single method, so this fix also fixes performance issue 8061950
>>> <https://bugs.openjdk.java.net/browse/JDK-8061950>.
>>>
>>> The patch also contains some optimizations around redundant copying of
>>> arrays and reflective objects.
>>>
>>> The FilterNotMostSpecific jtreg test has been updated to accommodate
>>> for changed behavior. Both of the above extreme cases have been added
>>> to the test.
>>>
>>> So, comments, please.
>>>
>>> Regards, Peter
>>>
>>
>