RFR 9 and 8u: JDK-8029674: (reflect) getMethods returns default methods that are not members of the class

[Joe Darcy]

Thanks for the investigation Andrej.

In any case, I'd prefer a comment noting the interned-ness (or mostly 
interned-ness, etc.) of the name to let other readers of the code using 
== rather than .equals is intentional and not a bug.

Thanks,

-Joe

On 06/09/2014 12:56 PM, Andrej Golovnin wrote:
> Hi Joel,
>
>> IIRC name isn’t actually interned with String.intern() but in the VM:s Symbol table as a name representing a (Java) method. Should be equivalent, as long as we don’t start comparing it with == with interned Strings.
> I think that's not true.
>
> The following small test program prints true on the console:
>
> public class Test {
>
>      public static void main(String... argv) throws Exception {
>          Method m = Test.class.getMethod("main", String[].class);
>          System.out.println(m.getName() == new String("main").intern());
>      }
>
> }
>
> And if you look into reflection.cpp at lines 787-789, you will see following code:
>
>    Symbol*  method_name = method->name();
>    oop name_oop = StringTable::intern(method_name, CHECK_NULL);
>    Handle name = Handle(THREAD, name_oop);
>
> And later at the line 798 we have this code:
>
>    java_lang_reflect_Method::set_name(mh(), name());
>
> Therefore I would say the name is actually interned in terms of String.intern().
> And you can compare the name of a method with == with interned Strings.
>
> The same applies to a name of a class and to a name of a field too.
> Please feel free to correct me.
>
> Best regards,
> Andrej Golovnin
>
>
>> cheers
>> /Joel
>>
>> On 07 Jun 2014, at 22:34, Andrej Golovnin <andrej.golovnin at gmail.com> wrote:
>>
>>> Hi Joe,
>>>
>>>> Sorry for the belated review.
>>>>
>>>> Generally the change looks good. One question, in
>>>>
>>>> 2803         private boolean matchesNameAndDescriptor(Method m1, Method m2) {
>>>> 2804             return m1.getReturnType() == m2.getReturnType() &&
>>>> 2805                    m1.getName() == m2.getName() &&
>>>> 2806                    arrayContentsEq(m1.getParameterTypes(),
>>>> 2807                            m2.getParameterTypes());
>>>> 2808         }
>>>>
>>>> Should the equality check on 2805 be .equals rather than == ?
>>>>
>>> "==" can be used in this case as the method name is interned by JVM.
>>> Here is the comment for the field "name" from java.lang.reflect.Method:
>>>
>>>   // This is guaranteed to be interned by the VM in the 1.4
>>>   // reflection implementation
>>>   private String              name;
>>>
>>> BTW, in the old version of Class in the line 2766 there was already a similar check:
>>>
>>> 2764                 if (m != null &&
>>> 2765                     m.getReturnType() == toRemove.getReturnType() &&
>>> 2766                     m.getName() == toRemove.getName() &&
>>> 2767                     arrayContentsEq(m.getParameterTypes(),
>>> 2768                                     toRemove.getParameterTypes())) {
>>> 2769                     methods[i] = null;
>>>
>>>
>>> Best regards,
>>> Andrej Golovnin
>>>
>>>