Re: Code review request for 6543593 "(reflect) Clarify private final field mutability"

Mon, 04 Apr 2011 00:48:56 -0700 

Hi David.

David Holmes wrote:

Hi Joe,

I'm not sure exactly what Bill was complaining about here,


His specific issue was addressed by the diff

    * <p>If the underlying field is final, the method throws an
-     * {@code IllegalAccessException} unless
-     * {@code setAccessible(true)} has succeeded for this field
-     * and this field is non-static. Setting a final field in this way
+     * {@code IllegalAccessException} unless {@code setAccessible(true)}
+     * has succeeded for this {@code Field} object
+     * and the field is non-static. Setting a final field in this way
    * is meaningful only during deserialization or reconstruction of
    * instances of classes with blank final fields, before they are
    * made available for access by other parts of a program. Use in
@@ -658,8 +667,9 @@
    * @param value the new value for the field of {@code obj}
    * being modified
In other words, setAccessible(true) can succeed on a java.lang.reflect.Field object but not on a field.
but using the same phrasing as Method and Constructor is a good thing. However, as the CR pertains in particular to the issue of final fields, I think for the setXXX cases the "@exception** IllegalAccessException" javadoc should also mention that it can be thrown because the field is final, not just because it is inaccessible. 

Fair enough.  How about just for the setter methods

+     * @exception IllegalAccessException    if this {@code Field} object
+ * enforces Java language access control and the underlying 
+     *              field is inaccessible or is final.



** shouldn't all those @exception tags get converted to @throws?
That would certainly be my style recommendation, but I wasn't looking to make that change at the moment. 

Thanks,

-Joe



David

Joe Darcy said the following on 04/04/11 16:32:

Hello.
Please review the more precise wording added to java.lang.reflect.Field to address bug 

   6543593 "(reflect) Clarify private final field mutability"
   http://cr.openjdk.java.net/~darcy/6543593.0/

Full patch below.
In describing when IllegalAccessException is thrown, Field now uses phrasing consistent with that used by its sibling classes Method and Constructor:
"if this Method object enforces Java language access control and the underlying method is inaccessible." "if this Constructor object enforces Java language access control and the underlying constructor is inaccessible." 

Thanks,

-Joe
--- old/src/share/classes/java/lang/reflect/Field.java 2011-04-03 23:25:40.000000000 -0700 +++ new/src/share/classes/java/lang/reflect/Field.java 2011-04-03 23:25:39.000000000 -0700 
@@ -360,8 +360,9 @@
     * {@code obj}; primitive values are wrapped in an appropriate
     * object before being returned
     *
-     * @exception IllegalAccessException    if the underlying field
-     *              is inaccessible.
+ * @exception IllegalAccessException if this {@code Field} object + * enforces Java language access control and the underlying 
+     *              field is inaccessible.
* @exception IllegalArgumentException if the specified object is not an * instance of the class or interface declaring the underlying 
     *              field (or a subclass or implementor thereof).
@@ -383,8 +384,9 @@
     * from
     * @return the value of the {@code boolean} field
     *
-     * @exception IllegalAccessException    if the underlying field
-     *              is inaccessible.
+ * @exception IllegalAccessException if this {@code Field} object + * enforces Java language access control and the underlying 
+     *              field is inaccessible.
* @exception IllegalArgumentException if the specified object is not 
     *              an instance of the class or interface declaring the
     *              underlying field (or a subclass or implementor
@@ -410,8 +412,9 @@
     * from
     * @return the value of the {@code byte} field
     *
-     * @exception IllegalAccessException    if the underlying field
-     *              is inaccessible.
+ * @exception IllegalAccessException if this {@code Field} object + * enforces Java language access control and the underlying 
+     *              field is inaccessible.
* @exception IllegalArgumentException if the specified object is not 
     *              an instance of the class or interface declaring the
     *              underlying field (or a subclass or implementor
@@ -439,8 +442,9 @@
     * from
     * @return the value of the field converted to type {@code char}
     *
-     * @exception IllegalAccessException    if the underlying field
-     *              is inaccessible.
+ * @exception IllegalAccessException if this {@code Field} object + * enforces Java language access control and the underlying 
+     *              field is inaccessible.
* @exception IllegalArgumentException if the specified object is not 
     *              an instance of the class or interface declaring the
     *              underlying field (or a subclass or implementor
@@ -468,8 +472,9 @@
     * from
     * @return the value of the field converted to type {@code short}
     *
-     * @exception IllegalAccessException    if the underlying field
-     *              is inaccessible.
+ * @exception IllegalAccessException if this {@code Field} object + * enforces Java language access control and the underlying 
+     *              field is inaccessible.
* @exception IllegalArgumentException if the specified object is not 
     *              an instance of the class or interface declaring the
     *              underlying field (or a subclass or implementor
@@ -497,8 +502,9 @@
     * from
     * @return the value of the field converted to type {@code int}
     *
-     * @exception IllegalAccessException    if the underlying field
-     *              is inaccessible.
+ * @exception IllegalAccessException if this {@code Field} object + * enforces Java language access control and the underlying 
+     *              field is inaccessible.
* @exception IllegalArgumentException if the specified object is not 
     *              an instance of the class or interface declaring the
     *              underlying field (or a subclass or implementor
@@ -526,8 +532,9 @@
     * from
     * @return the value of the field converted to type {@code long}
     *
-     * @exception IllegalAccessException    if the underlying field
-     *              is inaccessible.
+ * @exception IllegalAccessException if this {@code Field} object + * enforces Java language access control and the underlying 
+     *              field is inaccessible.
* @exception IllegalArgumentException if the specified object is not 
     *              an instance of the class or interface declaring the
     *              underlying field (or a subclass or implementor
@@ -555,8 +562,9 @@
     * from
     * @return the value of the field converted to type {@code float}
     *
-     * @exception IllegalAccessException    if the underlying field
-     *              is inaccessible.
+ * @exception IllegalAccessException if this {@code Field} object + * enforces Java language access control and the underlying 
+     *              field is inaccessible.
* @exception IllegalArgumentException if the specified object is not 
     *              an instance of the class or interface declaring the
     *              underlying field (or a subclass or implementor
@@ -584,8 +592,9 @@
     * from
     * @return the value of the field converted to type {@code double}
     *
-     * @exception IllegalAccessException    if the underlying field
-     *              is inaccessible.
+ * @exception IllegalAccessException if this {@code Field} object + * enforces Java language access control and the underlying 
+     *              field is inaccessible.
* @exception IllegalArgumentException if the specified object is not 
     *              an instance of the class or interface declaring the
     *              underlying field (or a subclass or implementor
@@ -626,9 +635,9 @@
     * {@code IllegalAccessException}.
     *
     * <p>If the underlying field is final, the method throws an
-     * {@code IllegalAccessException} unless
-     * {@code setAccessible(true)} has succeeded for this field
-     * and this field is non-static. Setting a final field in this way
+ * {@code IllegalAccessException} unless {@code setAccessible(true)} 
+     * has succeeded for this {@code Field} object
+     * and the field is non-static. Setting a final field in this way
     * is meaningful only during deserialization or reconstruction of
     * instances of classes with blank final fields, before they are
     * made available for access by other parts of a program. Use in
@@ -658,8 +667,9 @@
     * @param value the new value for the field of {@code obj}
     * being modified
     *
-     * @exception IllegalAccessException    if the underlying field
-     *              is inaccessible.
+ * @exception IllegalAccessException if this {@code Field} object + * enforces Java language access control and the underlying 
+     *              field is inaccessible.
* @exception IllegalArgumentException if the specified object is not an * instance of the class or interface declaring the underlying 
     *              field (or a subclass or implementor thereof),
@@ -686,8 +696,9 @@
     * @param z   the new value for the field of {@code obj}
     * being modified
     *
-     * @exception IllegalAccessException    if the underlying field
-     *              is inaccessible.
+ * @exception IllegalAccessException if this {@code Field} object + * enforces Java language access control and the underlying 
+     *              field is inaccessible.
* @exception IllegalArgumentException if the specified object is not an * instance of the class or interface declaring the underlying 
     *              field (or a subclass or implementor thereof),
@@ -715,8 +726,9 @@
     * @param b   the new value for the field of {@code obj}
     * being modified
     *
-     * @exception IllegalAccessException    if the underlying field
-     *              is inaccessible.
+ * @exception IllegalAccessException if this {@code Field} object + * enforces Java language access control and the underlying 
+     *              field is inaccessible.
* @exception IllegalArgumentException if the specified object is not an * instance of the class or interface declaring the underlying 
     *              field (or a subclass or implementor thereof),
@@ -744,8 +756,9 @@
     * @param c   the new value for the field of {@code obj}
     * being modified
     *
-     * @exception IllegalAccessException    if the underlying field
-     *              is inaccessible.
+ * @exception IllegalAccessException if this {@code Field} object + * enforces Java language access control and the underlying 
+     *              field is inaccessible.
* @exception IllegalArgumentException if the specified object is not an * instance of the class or interface declaring the underlying 
     *              field (or a subclass or implementor thereof),
@@ -773,8 +786,9 @@
     * @param s   the new value for the field of {@code obj}
     * being modified
     *
-     * @exception IllegalAccessException    if the underlying field
-     *              is inaccessible.
+ * @exception IllegalAccessException if this {@code Field} object + * enforces Java language access control and the underlying 
+     *              field is inaccessible.
* @exception IllegalArgumentException if the specified object is not an * instance of the class or interface declaring the underlying 
     *              field (or a subclass or implementor thereof),
@@ -802,8 +816,9 @@
     * @param i   the new value for the field of {@code obj}
     * being modified
     *
-     * @exception IllegalAccessException    if the underlying field
-     *              is inaccessible.
+ * @exception IllegalAccessException if this {@code Field} object + * enforces Java language access control and the underlying 
+     *              field is inaccessible.
* @exception IllegalArgumentException if the specified object is not an * instance of the class or interface declaring the underlying 
     *              field (or a subclass or implementor thereof),
@@ -831,8 +846,9 @@
     * @param l   the new value for the field of {@code obj}
     * being modified
     *
-     * @exception IllegalAccessException    if the underlying field
-     *              is inaccessible.
+ * @exception IllegalAccessException if this {@code Field} object + * enforces Java language access control and the underlying 
+     *              field is inaccessible.
* @exception IllegalArgumentException if the specified object is not an * instance of the class or interface declaring the underlying 
     *              field (or a subclass or implementor thereof),
@@ -860,8 +876,9 @@
     * @param f   the new value for the field of {@code obj}
     * being modified
     *
-     * @exception IllegalAccessException    if the underlying field
-     *              is inaccessible.
+ * @exception IllegalAccessException if this {@code Field} object + * enforces Java language access control and the underlying 
+     *              field is inaccessible.
* @exception IllegalArgumentException if the specified object is not an * instance of the class or interface declaring the underlying 
     *              field (or a subclass or implementor thereof),
@@ -889,8 +906,9 @@
     * @param d   the new value for the field of {@code obj}
     * being modified
     *
-     * @exception IllegalAccessException    if the underlying field
-     *              is inaccessible.
+ * @exception IllegalAccessException if this {@code Field} object + * enforces Java language access control and the underlying 
+     *              field is inaccessible.
* @exception IllegalArgumentException if the specified object is not an * instance of the class or interface declaring the underlying 
     *              field (or a subclass or implementor thereof),

Reply via email to