JDK 8 code review request for 8005042 Add Method.isDefault to core reflection

[Peter Levart]

Hi Joe,

Also, now studying the logic of Class.privateGetPublicMethods(), I don't 
quite get the reasoning behind current implementation regardless of 
default interface methods. The javadoc for Class.getMethods() is not 
very precise about what happens with multiple public methods with same 
signature coming from superclass and interfaces. Take for example the 
following program:

public class MethodsTest {
     public interface I {
         void m();
     }

     public interface J {
         void m();
     }

     public abstract static class S implements I, J {
         public abstract void m();
     }

     public abstract static class C extends S {
     }

     public abstract static class D extends S implements I, J {
     }

     static String toString(Class<?> clazz, Method[] methods) {
         StringBuilder sb = new 
StringBuilder(clazz.getSimpleName()).append(":");
         for (Method m : methods) {
             if (m.getDeclaringClass() != Object.class) {
                 sb.append(" ")
.append(m.getDeclaringClass().getSimpleName())
                     .append(".")
                     .append(m.getName());
             }
         }
         return sb.toString();
     }

     public static void main(String[] args) {
         System.out.println("\ndeclaredMethods:\n");
         System.out.println(toString(I.class, 
I.class.getDeclaredMethods()));
         System.out.println(toString(J.class, 
J.class.getDeclaredMethods()));
         System.out.println(toString(S.class, 
S.class.getDeclaredMethods()));
         System.out.println(toString(C.class, 
C.class.getDeclaredMethods()));
         System.out.println(toString(D.class, 
D.class.getDeclaredMethods()));
         System.out.println("\nmethods:\n");
         System.out.println(toString(I.class, I.class.getMethods()));
         System.out.println(toString(J.class, J.class.getMethods()));
         System.out.println(toString(S.class, S.class.getMethods()));
         System.out.println(toString(C.class, C.class.getMethods()));
         System.out.println(toString(D.class, D.class.getMethods()));
     }
}

which prints:

declaredMethods:

I: I.m
J: J.m
S: S.m
C:
D:

methods:

I: I.m
J: J.m
S: S.m
C: S.m
D: S.m I.m J.m

So current logic is such that abstract methods on this Class knock-out 
interface (abstract) methods, but abstract methods on superclass don't. 
In particular, I don't understand the reasoning behind the difference of 
results of C.class.getMethods() and D.class.getMethods(). I think that 
the consistent logic would be to play the class-always-wins rule for 
abstract methods too and always knock-out any interface methods 
(abstract or default) that have the same signature as a method, coming 
from a superclass.privateGetPublicMethods() and which is declared in a 
class (not interface) regardless of whether it is abstract or not. Such 
logic would still be in line with the search logic specified by 
Class.getMethod(name, parameterTypes).

Regards, Peter

On 12/19/2012 10:34 AM, Peter Levart wrote:
> Hi Joe,
>
> I think that besides this bug, also the 
> Class.privateGetPublicMethods() will need to be revised. Currently it 
> suppresses any methods from direct interfaces that either:
> - have a non-abstract method with same signature in a superclass
> - have an abstract or non-abstract method with same signature in this 
> class
>
> Now because of the class-always-wins rule, an abstract method from a 
> superclass that is declared in a class (not interface) also has to 
> suppress any default method coming from any direct interface, don't 
> you think?
>
> Regards, Peter
>
> On 12/19/2012 10:01 AM, Peter Levart wrote:
>> On 12/19/2012 01:35 AM, David Holmes wrote:
>>> On 19/12/2012 10:24 AM, Joe Darcy wrote:
>>>> On 12/18/2012 04:20 PM, David Holmes wrote:
>>>>> On 19/12/2012 10:16 AM, Joe Darcy wrote:
>>>>>> On 12/18/2012 04:12 PM, David Holmes wrote:
>>>>>>> On 19/12/2012 8:40 AM, Louis Wasserman wrote:
>>>>>>>> It's not 100% obvious to me whether this refers to a default
>>>>>>>> implementation
>>>>>>>> in an interface, a class which inherits that default 
>>>>>>>> implementation
>>>>>>>> and does not override it, or both. Is that worth clarifying in 
>>>>>>>> the doc,
>>>>>>>> rather than forcing readers to check the JLS citation?
>>>>>>>
>>>>>>> The issue is where you obtained this Method reference from:
>>>>>>>
>>>>>>> - from the Interface? then it is a default method
>>>>>>> - from a class implementing the interface but not redefining the
>>>>>>> method? then it is a default method
>>>>>>
>>>>>> Actually, that is *now* how HotSpot represents this case in core
>>>>>> reflection at the moment. HotSpot uses a new method object to 
>>>>>> represent
>>>>>> the default method woven into an implementing class.
>>>>>
>>>>> *now* -> *not* ??
>>>>
>>>> Correct.
>>>>
>>>>>
>>>>> It may be a new Method object but getDeclaringClass() should give the
>>>>> interface class NOT the concrete class. That is currently the case 
>>>>> for
>>>>> abstract interface methods. I hope it is the same for default 
>>>>> methods!
>>>>
>>>> It is not at the moment, which is a bit surprising.
>>>
>>> Very surprising! I'd call that a major bug.
>>
>> Not only default methods, also abstract interface methods show-up in 
>> the implementing class's declared methods. For example the following 
>> test:
>>
>> public class DefaultMethodsTest {
>>     public interface I {
>>         void i();
>>         default void d() { }
>>     }
>>
>>     public abstract static class S {
>>         public abstract void a();
>>         public void s() { }
>>     }
>>
>>     public abstract static class C extends S implements I {
>>         public void c() { }
>>     }
>>
>>     public static void main(String[] args) {
>>         for (Method m : C.class.getDeclaredMethods())
>>             System.out.println(m.getName());
>>     }
>> }
>>
>>
>> prints:
>>
>> c
>> i
>> d
>>
>>
>> Regards, Peter
>>
>>>
>>> David
>>> -----
>>>
>>>
>>>> -Joe
>>
>