Thanks for making the change. It looks good to me. Mike
On Mar 28 2013, at 17:59 , Joe Darcy wrote: > Hi Mike, > > Sure; I've checked with the latest lambda draft and changed the code to > output any default modifier after any public / private / protected modifiers. > The main change is in the printModifiersIfNonzero method: > > void printModifiersIfNonzero(StringBuilder sb, int mask, boolean > isDefault) { > int mod = getModifiers() & mask; > > if (mod != 0 && !isDefault) { > sb.append(Modifier.toString(mod)).append(' '); > } else { > int access_mod = mod & Modifier.ACCESS_MODIFIERS; > if (access_mod != 0) > sb.append(Modifier.toString(access_mod)).append(' '); > if (isDefault) > sb.append("default "); > mod = (mod & ~Modifier.ACCESS_MODIFIERS); > if (mod != 0) > sb.append(Modifier.toString(mod)).append(' '); > } > } > > Updated webrev including a "public default strictfp" test case at: > > http://cr.openjdk.java.net/~darcy/8004979.1/ > > Thanks, > > -Joe > > On 03/28/2013 03:58 PM, Mike Duigou wrote: >> Technically the changes look fine but I am a little concerned by the >> position of default in the order placement. The emerging practice is to put >> default first immediately following the access modifier. Counter to this is >> the correct preference for using the "blessed modifier order". Can we move >> default to the beginning of the order? >> >> Mike >> >> On Mar 28 2013, at 00:03 , Joe Darcy wrote: >> >>> Hello, >>> >>> Please review these changes to add support for the "default" modifier to >>> the output of Method.to[Generic]String: >>> >>> 8004979 java.lang.reflect.Modifier.toString should include "default" >>> http://cr.openjdk.java.net/~darcy/8004979.0/ >>> >>> Patch also included below. >>> >>> Thanks, >>> >>> -Joe >>> >>> diff -r d254a5f9b93f src/share/classes/java/lang/reflect/Constructor.java >>> --- a/src/share/classes/java/lang/reflect/Constructor.java Wed Mar 27 >>> 13:40:26 2013 -0400 >>> +++ b/src/share/classes/java/lang/reflect/Constructor.java Thu Mar 28 >>> 00:02:06 2013 -0700 >>> @@ -284,9 +284,13 @@ >>> * modifiers {@code public}, {@code protected} or >>> * {@code private}. Only one of these may appear, or none if the >>> * constructor has default (package) access. >>> + * >>> + * @return a string describing this {@code Constructor} >>> + * @jls 8.8.3. Constructor Modifiers >>> */ >>> public String toString() { >>> return sharedToString(Modifier.constructorModifiers(), >>> + false, >>> parameterTypes, >>> exceptionTypes); >>> } >>> @@ -328,10 +332,11 @@ >>> * include type parameters >>> * >>> * @since 1.5 >>> + * @jls 8.8.3. Constructor Modifiers >>> */ >>> @Override >>> public String toGenericString() { >>> - return sharedToGenericString(Modifier.constructorModifiers()); >>> + return sharedToGenericString(Modifier.constructorModifiers(), >>> false); >>> } >>> >>> @Override >>> diff -r d254a5f9b93f src/share/classes/java/lang/reflect/Executable.java >>> --- a/src/share/classes/java/lang/reflect/Executable.java Wed Mar 27 >>> 13:40:26 2013 -0400 >>> +++ b/src/share/classes/java/lang/reflect/Executable.java Thu Mar 28 >>> 00:02:06 2013 -0700 >>> @@ -89,20 +89,24 @@ >>> >>> } >>> >>> - void printModifiersIfNonzero(StringBuilder sb, int mask) { >>> + void printModifiersIfNonzero(StringBuilder sb, int mask, boolean >>> isDefault) { >>> int mod = getModifiers() & mask; >>> if (mod != 0) { >>> sb.append(Modifier.toString(mod)).append(' '); >>> } >>> + if (isDefault) { >>> + sb.append("default "); >>> + } >>> } >>> >>> String sharedToString(int modifierMask, >>> + boolean isDefault, >>> Class<?>[] parameterTypes, >>> Class<?>[] exceptionTypes) { >>> try { >>> StringBuilder sb = new StringBuilder(); >>> >>> - printModifiersIfNonzero(sb, modifierMask); >>> + printModifiersIfNonzero(sb, modifierMask, isDefault); >>> specificToStringHeader(sb); >>> >>> sb.append('('); >>> @@ -124,11 +128,11 @@ >>> */ >>> abstract void specificToStringHeader(StringBuilder sb); >>> >>> - String sharedToGenericString(int modifierMask) { >>> + String sharedToGenericString(int modifierMask, boolean isDefault) { >>> try { >>> StringBuilder sb = new StringBuilder(); >>> >>> - printModifiersIfNonzero(sb, modifierMask); >>> + printModifiersIfNonzero(sb, modifierMask, isDefault); >>> >>> TypeVariable<?>[] typeparms = getTypeParameters(); >>> if (typeparms.length > 0) { >>> diff -r d254a5f9b93f src/share/classes/java/lang/reflect/Field.java >>> --- a/src/share/classes/java/lang/reflect/Field.java Wed Mar 27 13:40:26 >>> 2013 -0400 >>> +++ b/src/share/classes/java/lang/reflect/Field.java Thu Mar 28 00:02:06 >>> 2013 -0700 >>> @@ -288,6 +288,9 @@ >>> * {@code protected} or {@code private} first, and then other >>> * modifiers in the following order: {@code static}, {@code final}, >>> * {@code transient}, {@code volatile}. >>> + * >>> + * @return a string describing this {@code Field} >>> + * @jls 8.3.1 Field Modifiers >>> */ >>> public String toString() { >>> int mod = getModifiers(); >>> @@ -315,6 +318,7 @@ >>> * its generic type >>> * >>> * @since 1.5 >>> + * @jls 8.3.1 Field Modifiers >>> */ >>> public String toGenericString() { >>> int mod = getModifiers(); >>> diff -r d254a5f9b93f src/share/classes/java/lang/reflect/Method.java >>> --- a/src/share/classes/java/lang/reflect/Method.java Wed Mar 27 >>> 13:40:26 2013 -0400 >>> +++ b/src/share/classes/java/lang/reflect/Method.java Thu Mar 28 >>> 00:02:06 2013 -0700 >>> @@ -343,10 +343,16 @@ >>> * {@code public}, {@code protected} or {@code private} first, >>> * and then other modifiers in the following order: >>> * {@code abstract}, {@code static}, {@code final}, >>> - * {@code synchronized}, {@code native}, {@code strictfp}. >>> + * {@code synchronized}, {@code native}, {@code strictfp}, >>> + * {@code default}. >>> + * >>> + * @return a string describing this {@code Method} >>> + * >>> + * @jls 8.4.3 Method Modifiers >>> */ >>> public String toString() { >>> return sharedToString(Modifier.methodModifiers(), >>> + isDefault(), >>> parameterTypes, >>> exceptionTypes); >>> } >>> @@ -389,16 +395,19 @@ >>> * {@code public}, {@code protected} or {@code private} first, >>> * and then other modifiers in the following order: >>> * {@code abstract}, {@code static}, {@code final}, >>> - * {@code synchronized}, {@code native}, {@code strictfp}. >>> + * {@code synchronized}, {@code native}, {@code strictfp}, >>> + * {@code default}. >>> * >>> * @return a string describing this {@code Method}, >>> * include type parameters >>> * >>> * @since 1.5 >>> + * >>> + * @jls 8.4.3 Method Modifiers >>> */ >>> @Override >>> public String toGenericString() { >>> - return sharedToGenericString(Modifier.methodModifiers()); >>> + return sharedToGenericString(Modifier.methodModifiers(), >>> isDefault()); >>> } >>> >>> @Override >>> diff -r d254a5f9b93f test/java/lang/reflect/Method/GenericStringTest.java >>> --- a/test/java/lang/reflect/Method/GenericStringTest.java Wed Mar 27 >>> 13:40:26 2013 -0400 >>> +++ b/test/java/lang/reflect/Method/GenericStringTest.java Thu Mar 28 >>> 00:02:06 2013 -0700 >>> @@ -23,7 +23,7 @@ >>> >>> /* >>> * @test >>> - * @bug 5033583 6316717 6470106 >>> + * @bug 5033583 6316717 6470106 8004979 >>> * @summary Check toGenericString() and toString() methods >>> * @author Joseph D. Darcy >>> */ >>> @@ -39,6 +39,7 @@ >>> classList.add(TestClass1.class); >>> classList.add(TestClass2.class); >>> classList.add(Roebling.class); >>> + classList.add(TestInterface1.class); >>> >>> >>> for(Class<?> clazz: classList) >>> @@ -129,6 +130,20 @@ >>> void varArg(Object ... arg) {} >>> } >>> >>> +interface TestInterface1 { >>> + @ExpectedGenericString( >>> + "public default void TestInterface1.foo()") >>> + @ExpectedString( >>> + "public default void TestInterface1.foo()") >>> + public default void foo(){;} >>> + >>> + @ExpectedString( >>> + "public default java.lang.Object TestInterface1.bar()") >>> + @ExpectedGenericString( >>> + "public default <A> A TestInterface1.bar()") >>> + default <A> A bar(){return null;} >>> +} >>> + >>> @Retention(RetentionPolicy.RUNTIME) >>> @interface ExpectedGenericString { >>> String value(); >>> >