[cp-patches] Fix for Class.getSimpleName()
Hi, attached is a fix for the method Class.getSimpleName(), together with a small testcase. The old implementation failed on usual class names, as well as on inner classes. Please comment/commit. This is my first direct classpath contribution. Do I need to sign any copyright assignment? Regards, Sebastian -- tarent Gesellschaft für Softwareentwicklung und IT-Beratung mbH Heilsbachstr. 24, 53123 Bonn| Poststr. 4-5, 10178 Berlin fon: +49(228) / 52675-0 | fon: +49(30) / 27594853 fax: +49(228) / 52675-25| fax: +49(30) / 78709617 durchwahl: +49(228) / 52675-17 | mobil: +49(171) / 7673249 Geschäftsführer: Boris Esser, Elmar Geese, Thomas Müller-Ackermann HRB AG Bonn 5168 Ust-ID: DE122264941 Index: vm/reference/java/lang/VMClass.java === RCS file: /sources/classpath/classpath/vm/reference/java/lang/VMClass.java,v retrieving revision 1.20 diff -u -r1.20 VMClass.java --- vm/reference/java/lang/VMClass.java 18 Sep 2007 21:52:38 - 1.20 +++ vm/reference/java/lang/VMClass.java 15 Apr 2008 21:09:51 - @@ -304,19 +304,11 @@ } String fullName = getName(klass); int pos = fullName.lastIndexOf("$"); -if (pos == -1) - pos = 0; -else - { - ++pos; - while (Character.isDigit(fullName.charAt(pos))) - ++pos; - } -int packagePos = fullName.lastIndexOf(".", pos); -if (packagePos == -1) - return fullName.substring(pos); -else - return fullName.substring(packagePos + 1); +if (pos == -1) { +pos = fullName.lastIndexOf("."); +} +pos++; +return fullName.substring(pos); } /** Index: testsuite/java.lang/SimpleNameTest.java === RCS file: testsuite/java.lang/SimpleNameTest.java diff -N testsuite/java.lang/SimpleNameTest.java --- /dev/null 1 Jan 1970 00:00:00 - +++ testsuite/java.lang/SimpleNameTest.java 15 Apr 2008 21:09:51 - @@ -0,0 +1,40 @@ +package simplename; + +public class SimpleNameTest +{ +public static void main(final String[] args) { +new SimpleNameTest(); +} + +private SimpleNameTest() { +if ("SimpleNameTest".equals(SimpleNameTest.class.getSimpleName())) +passed("SimpleNameTest.class.getSimpleName() is \"SimpleNameTest\""); +else +failed("SimpleNameTest.class.getSimpleName() should be \"SimpleNameTest\", but is "+SimpleNameTest.class.getSimpleName()); + +Object anonymous = new Object(){}; +if ("".equals(anonymous.getClass().getSimpleName())) +passed("anonymous.getClass().getSimpleName() is \"\""); +else +failed("anonymous.getClass().getSimpleName() should be \"\", but is "+anonymous.getClass().getSimpleName()); + + +if ("Inner".equals(Inner.class.getSimpleName())) +passed("Inner.class.getSimpleName() is \"Inner\""); +else +failed("Inner.class.getSimpleName() should be \"Inner\", but is "+Inner.class.getSimpleName()); + +} + +public class Inner { + +} + +static void passed(String msg) { +System.out.println("PASSED: "+msg); +} + +static void failed(String msg) { +System.out.println("FAILED: "+msg); +} +}
Re: [cp-patches] Fix for Class.getSimpleName()
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Christian Thalinger schrieb: > On Tue, 2008-04-15 at 23:28 +0200, Sebastian Mancke wrote: >> Hi, >> >> attached is a fix for the method Class.getSimpleName(), together with a >> small testcase. The old implementation failed on usual class names, as >> well as on inner classes. >> >> Please comment/commit. > > This reminds me of this patch: > > http://www.mail-archive.com/classpath-patches@gnu.org/msg10378.html > > And I think your changes will break it (but I haven't tested). I saw this patch and think it broke the behaviour, because of the wrong usage of fullName.lastIndexOf(".", pos): The pos argument, counted from left makes no sense in this method. Also, I think, that advancing 'pos', dependent on Character.isDigit(fullName.charAt(pos)) make no sense, because we return "" for anonymous classes already. - --Sebastian - -- tarent Gesellschaft für Softwareentwicklung und IT-Beratung mbH Heilsbachstr. 24, 53123 Bonn| Poststr. 4-5, 10178 Berlin fon: +49(228) / 52675-0 | fon: +49(30) / 27594853 fax: +49(228) / 52675-25| fax: +49(30) / 78709617 durchwahl: +49(228) / 52675-17 | mobil: +49(171) / 7673249 Geschäftsführer: Boris Esser, Elmar Geese, Thomas Müller-Ackermann HRB AG Bonn 5168 Ust-ID: DE122264941 -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.6 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iD8DBQFIBb+lsMJ2Yk81wE0RAjE8AJ9FsNX2ApT6kU3eZz3RLdYWFasZDACg9vhk 4aiBh4P6d5bce9jFqisJ5mU= =aKq8 -END PGP SIGNATURE-
Re: [cp-patches] Fix for Class.getSimpleName()
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Christian Thalinger schrieb: > On Wed, 2008-04-16 at 10:58 +0200, Sebastian Mancke wrote: >> I saw this patch and think it broke the behaviour, because of the wrong >> usage of fullName.lastIndexOf(".", pos): The pos argument, counted from >> left makes no sense in this method. >> >> Also, I think, that advancing 'pos', dependent on >> Character.isDigit(fullName.charAt(pos)) make no sense, because we return >> "" for anonymous classes already. > > I see. Can you try the CACAO testcase (I hope this is the right one): > tests/regression/TestAnnotations.java TestAnnotations hangs the current cacao. Dump is attached. But maybe the testcase you mean is MinimalClassReflection, right? I have diff'ed cacaos output to the jdks output. In fact, I have missed one case: The Digits after the '$' exist for local classes and have to be skipped for the simple name. I will overwork my patch. - --Sebastian - -- tarent Gesellschaft für Softwareentwicklung und IT-Beratung mbH Heilsbachstr. 24, 53123 Bonn| Poststr. 4-5, 10178 Berlin fon: +49(228) / 52675-0 | fon: +49(30) / 27594853 fax: +49(228) / 52675-25| fax: +49(30) / 78709617 durchwahl: +49(228) / 52675-17 | mobil: +49(171) / 7673249 Geschäftsführer: Boris Esser, Elmar Geese, Thomas Müller-Ackermann HRB AG Bonn 5168 Ust-ID: DE122264941 -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.6 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iD8DBQFIBdTQsMJ2Yk81wE0RAiSsAKD6Oat3rw6HGWzV0jTiPBbrWD2cQgCeMnJa gQYpAZmnkZMwxFmqAegbNVA= =AgQ5 -END PGP SIGNATURE- == Testing EnumA == LOG: [0x401b96b0] We received a SIGSEGV and tried to handle it, but we were LOG: [0x401b96b0] unable to find a Java method at: LOG: [0x401b96b0] LOG: [0x401b96b0] PC=0x4024ff33 LOG: [0x401b96b0] LOG: [0x401b96b0] Dumping the current stacktrace: at java.lang.reflect.VMMethod.getDefaultValue()Ljava/lang/Object;(Native Method) at java.lang.reflect.Method.getDefaultValue()Ljava/lang/Object;(Method.java:440) at sun.reflect.annotation.AnnotationType.(Ljava/lang/Class;)V(AnnotationType.java:132) at sun.reflect.annotation.AnnotationType.getInstance(Ljava/lang/Class;)Lsun/reflect/annotation/AnnotationType;(AnnotationType.java:99) at sun.reflect.annotation.AnnotationParser.parseAnnotation(Ljava/nio/ByteBuffer;Lsun/reflect/ConstantPool;Ljava/lang/Class;Z)Ljava/lang/annotation/Annotation;(AnnotationParser.java:300) at sun.reflect.annotation.AnnotationParser.parseAnnotations2([BLsun/reflect/ConstantPool;Ljava/lang/Class;)Ljava/util/Map;(AnnotationParser.java:167) at sun.reflect.annotation.AnnotationParser.parseAnnotations([BLsun/reflect/ConstantPool;Ljava/lang/Class;)Ljava/util/Map;(AnnotationParser.java:150) at sun.reflect.annotation.AnnotationParser.parseAnnotationsIntoArray([BLsun/reflect/ConstantPool;Ljava/lang/Class;)[Ljava/lang/annotation/Annotation;(AnnotationParser.java:63) at java.lang.VMClass.getDeclaredAnnotations(Ljava/lang/Class;)[Ljava/lang/annotation/Annotation;(Native Method) at java.lang.Class.getDeclaredAnnotations()[Ljava/lang/annotation/Annotation;(Class.java:1575) at AnnotatedElementAnnotationTester.testGetDeclaredAnnotations()Z(TestAnnotations.java:862) at AnnotatedElementAnnotationTester.test()Z(TestAnnotations.java:851) at ClassAnnotationTester.test()Z(TestAnnotations.java:1268) at TestAnnotations.main([Ljava/lang/String;)V(TestAnnotations.java:1773) LOG: [0x401b96b0] Exiting...
Re: [cp-patches] Fix for Class.getSimpleName()
Sebastian Mancke schrieb: > > > Christian Thalinger schrieb: >> On Wed, 2008-04-16 at 10:58 +0200, Sebastian Mancke wrote: >>> I saw this patch and think it broke the behaviour, because of the wrong >>> usage of fullName.lastIndexOf(".", pos): The pos argument, counted from >>> left makes no sense in this method. >>> >>> Also, I think, that advancing 'pos', dependent on >>> Character.isDigit(fullName.charAt(pos)) make no sense, because we return >>> "" for anonymous classes already. >> I see. Can you try the CACAO testcase (I hope this is the right one): >> tests/regression/TestAnnotations.java > TestAnnotations hangs the current cacao. Dump is attached. > > But maybe the testcase you mean is MinimalClassReflection, right? > I have diff'ed cacaos output to the jdks output. In fact, I have missed > one case: The Digits after the '$' exist for local classes and have to > be skipped for the simple name. I will overwork my patch. So, here is my patch again, now handling local classes as well. I have tested it against the attached testcase, as well as against cacaos MinimalClassReflection.java. The following class name situations are covered: Class in default package Class in package Inner Class (e.g. xyz.Abc$Inner -> "Inner") Local Class (e.g. xyz.Abc$1Local -> "Local") Anonymous (e.g. xyz.Abc$2 -> "") --Sebastian -- tarent Gesellschaft für Softwareentwicklung und IT-Beratung mbH Heilsbachstr. 24, 53123 Bonn| Poststr. 4-5, 10178 Berlin fon: +49(228) / 52675-0 | fon: +49(30) / 27594853 fax: +49(228) / 52675-25| fax: +49(30) / 78709617 durchwahl: +49(228) / 52675-17 | mobil: +49(171) / 7673249 Geschäftsführer: Boris Esser, Elmar Geese, Thomas Müller-Ackermann HRB AG Bonn 5168 Ust-ID: DE122264941 Index: vm/reference/java/lang/VMClass.java === RCS file: /sources/classpath/classpath/vm/reference/java/lang/VMClass.java,v retrieving revision 1.20 diff -u -r1.20 VMClass.java --- vm/reference/java/lang/VMClass.java 18 Sep 2007 21:52:38 - 1.20 +++ vm/reference/java/lang/VMClass.java 16 Apr 2008 11:12:18 - @@ -304,19 +304,14 @@ } String fullName = getName(klass); int pos = fullName.lastIndexOf("$"); -if (pos == -1) - pos = 0; -else - { - ++pos; - while (Character.isDigit(fullName.charAt(pos))) - ++pos; - } -int packagePos = fullName.lastIndexOf(".", pos); -if (packagePos == -1) - return fullName.substring(pos); -else - return fullName.substring(packagePos + 1); +if (pos != -1) { //inner class or local class +// skip digits of local classes +while (Character.isDigit(fullName.charAt(pos+1))) +pos++; +} else { +pos = fullName.lastIndexOf("."); +} +return fullName.substring(pos+1); } /** Index: testsuite/java.lang/SimpleNameTest.java === RCS file: testsuite/java.lang/SimpleNameTest.java diff -N testsuite/java.lang/SimpleNameTest.java --- /dev/null 1 Jan 1970 00:00:00 - +++ testsuite/java.lang/SimpleNameTest.java 16 Apr 2008 11:12:18 - @@ -0,0 +1,54 @@ +package simplename; + +public class SimpleNameTest +{ +public static void main(final String[] args) { +new SimpleNameTest(); +} + +private SimpleNameTest() { + +if ("Object".equals(Object.class.getSimpleName())) +passed("Object.class.getSimpleName() is \"Object\""); +else +failed("Object.class.getSimpleName() should be \"Object\", but is "+Object.class.getSimpleName()); + + +if ("SimpleNameTest".equals(SimpleNameTest.class.getSimpleName())) +passed("SimpleNameTest.class.getSimpleName() is \"SimpleNameTest\""); +else +failed("SimpleNameTest.class.getSimpleName() should be \"SimpleNameTest\", but is "+SimpleNameTest.class.getSimpleName()); + +Object anonymous = new Object(){}; +if ("".equals(anonymous.getClass().getSimpleName())) +passed("anonymous.getClass().getSimpleName() is \"\""); +else +failed("anonymous.getClass().getSimpleName() should be \"\", but is "+anonymous.getClass().getSimpleName()); + + +if ("Inner".equals(Inner.class.getSimpleName())) +passed("Inner.class.getSimpleName() is \"Inner\""); +else +failed("Inner.class.getSimpleName() should be \"Inner\", but is "+Inner.class.getSimpleName()); +
Re: [cp-patches] Fix for Class.getSimpleName()
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Mark Wielaard schrieb: > Hi, > > On Thu, 2008-04-17 at 00:50 +0100, Andrew John Hughes wrote: >> As to your contribution, the Classpath part (i.e. the java.lang.Class >> changes, Mauve has different contribution rules) looks minor enough to >> not require an assignment, but I'll let Mark (CCed) answer that for >> definite. However, if you intend to do further Classpath >> contributions, I'd suggest sorting out the necessary paperwork with >> the FSF; you need to assign copyright to them. Either Mark or I >> should be able to send you the form. > > Yes, you are right. This is small enough to be "obvious", but it would > be nice to have paperwork on file with the FSF stating your willingness > to contribute (larger) patches in the future. I'll send you the request > form. I have sent the request to the FSF. > BTW. libgcj has a slightly different implementation of > Class.getSimpleName() that might be worth merging with the classpath > version: http://gcc.gnu.org/ml/java-patches/2006-q3/msg00192.html Yes, looks good. - --Sebastian - -- tarent Gesellschaft für Softwareentwicklung und IT-Beratung mbH Heilsbachstr. 24, 53123 Bonn| Poststr. 4-5, 10178 Berlin fon: +49(228) / 52675-0 | fon: +49(30) / 27594853 fax: +49(228) / 52675-25| fax: +49(30) / 78709617 durchwahl: +49(228) / 52675-17 | mobil: +49(171) / 7673249 Geschäftsführer: Boris Esser, Elmar Geese, Thomas Müller-Ackermann HRB AG Bonn 5168 Ust-ID: DE122264941 -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.6 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iD8DBQFIB1mgsMJ2Yk81wE0RAkNSAJwLyLs+lOg/UfkloI7rHe1V3goYtgCg+xj+ zpAvV2K7It3KZ9u2nt1TTX0= =SOH2 -END PGP SIGNATURE-
Re: [cp-patches] Fix for Class.getSimpleName()
Hi. Now, the method getSimpleName() is merged with the one of glibgcj. (Not changing behaviour, but avoiding recursion) The Testcase has additional checks for array types and is converted to mauve, now. I could not find regression due to my changes --Sebastian Sebastian Mancke schrieb: > > > Mark Wielaard schrieb: >> Hi, > >> On Thu, 2008-04-17 at 00:50 +0100, Andrew John Hughes wrote: >>> As to your contribution, the Classpath part (i.e. the java.lang.Class >>> changes, Mauve has different contribution rules) looks minor enough to >>> not require an assignment, but I'll let Mark (CCed) answer that for >>> definite. However, if you intend to do further Classpath >>> contributions, I'd suggest sorting out the necessary paperwork with >>> the FSF; you need to assign copyright to them. Either Mark or I >>> should be able to send you the form. >> Yes, you are right. This is small enough to be "obvious", but it would >> be nice to have paperwork on file with the FSF stating your willingness >> to contribute (larger) patches in the future. I'll send you the request >> form. > I have sent the request to the FSF. > >> BTW. libgcj has a slightly different implementation of >> Class.getSimpleName() that might be worth merging with the classpath >> version: http://gcc.gnu.org/ml/java-patches/2006-q3/msg00192.html > Yes, looks good. > > --Sebastian > -- tarent Gesellschaft für Softwareentwicklung und IT-Beratung mbH Heilsbachstr. 24, 53123 Bonn| Poststr. 4-5, 10178 Berlin fon: +49(228) / 52675-0 | fon: +49(30) / 27594853 fax: +49(228) / 52675-25| fax: +49(30) / 78709617 durchwahl: +49(228) / 52675-17 | mobil: +49(171) / 7673249 Geschäftsführer: Boris Esser, Elmar Geese, Thomas Müller-Ackermann HRB AG Bonn 5168 Ust-ID: DE122264941 Index: vm/reference/java/lang/VMClass.java === RCS file: /sources/classpath/classpath/vm/reference/java/lang/VMClass.java,v retrieving revision 1.20 diff -u -r1.20 VMClass.java --- vm/reference/java/lang/VMClass.java 18 Sep 2007 21:52:38 - 1.20 +++ vm/reference/java/lang/VMClass.java 19 Apr 2008 15:19:00 - @@ -296,27 +296,43 @@ */ static String getSimpleName(Class klass) { +int arrayCount = 0; +while (klass.isArray()) + { +klass = klass.getComponentType(); +++arrayCount; + } +// now klass is the component type + +String simpleComponentName = null; if (isAnonymousClass(klass)) - return ""; -if (isArray(klass)) { - return getComponentType(klass).getSimpleName() + "[]"; +simpleComponentName = ""; } -String fullName = getName(klass); -int pos = fullName.lastIndexOf("$"); -if (pos == -1) - pos = 0; else { - ++pos; - while (Character.isDigit(fullName.charAt(pos))) - ++pos; +String fullName = getName(klass); +int pos = fullName.lastIndexOf("$"); +if (pos != -1) + { //inner class or local class +// skip digits of local classes +while (Character.isDigit(fullName.charAt(pos+1))) + pos++; + } +else + { +pos = fullName.lastIndexOf("."); + } +simpleComponentName = fullName.substring(pos+1); } -int packagePos = fullName.lastIndexOf(".", pos); -if (packagePos == -1) - return fullName.substring(pos); -else - return fullName.substring(packagePos + 1); + +if (arrayCount == 0) + return simpleComponentName; + +StringBuffer sb = new StringBuffer(simpleComponentName); +while (arrayCount-- > 0) + sb.append("[]"); +return sb.toString(); } /** Index: gnu/testlet/java/lang/Class/SimpleNameTest.java === RCS file: gnu/testlet/java/lang/Class/SimpleNameTest.java diff -N gnu/testlet/java/lang/Class/SimpleNameTest.java --- /dev/null 1 Jan 1970 00:00:00 - +++ gnu/testlet/java/lang/Class/SimpleNameTest.java 19 Apr 2008 15:18:25 - @@ -0,0 +1,82 @@ +// Copyright (C) 2008 Sebastian Mancke, Tarent GmbH <[EMAIL PROTECTED]> + +// This file is part of Mauve. + +// Mauve is free software; you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation; either version 2, or (at your option) +// any later version. + +// Mauve is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +//