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();
>>>
>
