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

2016-10-11 Thread 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@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@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@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
) that also fixes a
conformance (P3) bug (8029459
) and a performance
issue (8061950 ):

http://cr.openjdk.java.net/~plevart/jdk9-dev/Class.getMethods.new/webrev.04/ 




As Daniel Smith writes in 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 

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

2016-10-11 Thread Daniel Fuchs

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?

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
) that also fixes a
conformance (P3) bug (8029459
) and a performance
issue (8061950 ):

http://cr.openjdk.java.net/~plevart/jdk9-dev/Class.getMethods.new/webrev.04/


As Daniel Smith writes in 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
, getMethod() and
getMethods() share the same consolidation logic that results in a 

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

2016-10-11 Thread Alan Bateman



On 10/10/2016 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.
I've taken a first pass over this and it looks quite good. I do intend 
to do a second pass and I hope others will help review this. One things 
we'll probably need to do is write a release note so that the behavioral 
change is documented, I'm trying to think if there are scenarios where 
the historical inconsistency could cause something to work that won't 
work now.


-Alan


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

2016-10-10 Thread Martin Buchholz
Does core reflection have an active maintainer?  Or is Peter that
maintainer?

On Mon, Oct 10, 2016 at 1:04 AM, 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 > /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.getMethod
>> s.new/webrev.04/
>>
>> As Daniel Smith writes in 8029459 > /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 > /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
>> 

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

2016-10-10 Thread Peter Levart

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 
) that also fixes a 
conformance (P3) bug (8029459 
) and a performance 
issue (8061950 ):


http://cr.openjdk.java.net/~plevart/jdk9-dev/Class.getMethods.new/webrev.04/

As Daniel Smith writes in 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 
, 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