RFR 9 and 8u: JDK-8029674: (reflect) getMethods returns default methods that are not members of the class (original) (raw)
Joe Darcy joe.darcy at oracle.com
Mon Jun 9 23:09:13 UTC 2014
- Previous message: RFR 9 and 8u: JDK-8029674: (reflect) getMethods returns default methods that are not members of the class
- Next message: RFR 9 and 8u: JDK-8029674: (reflect) getMethods returns default methods that are not members of the class
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
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* methodname = method->name(); oop nameoop = StringTable::intern(methodname, CHECKNULL); Handle name = Handle(THREAD, nameoop); And later at the line 798 we have this code: javalangreflectMethod::setname(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
- Previous message: RFR 9 and 8u: JDK-8029674: (reflect) getMethods returns default methods that are not members of the class
- Next message: RFR 9 and 8u: JDK-8029674: (reflect) getMethods returns default methods that are not members of the class
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]