JDK 8 code review request for JDK-8004979 java.lang.reflect.Modifier.toString should include "default"
Joe Darcy
joe.darcy at oracle.com
Fri Mar 29 00:59:17 UTC 2013
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();
>>
More information about the core-libs-dev
mailing list