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

Reply via email to