RFR [9] 8071472: Add field access to support setting final fields in readObject

[Chris Hegarty]

On 17/03/15 11:52, Peter Levart wrote:
> On 03/17/2015 10:57 AM, Chris Hegarty wrote:
>> Peter, Alan,
>>
>> After further thought I think that requiring all final fields to be set is reasonable behaviour. Since there is no compile time checking, this is a reasonable runtime effort to catch what is likely to be developer errors. To address Alan’s comments, I beefed up the API docs and added examples to typical usage.
>>
>> Updated specdiff (and webrev):
>>     http://cr.openjdk.java.net/~chegar/8071472/01/  <http://cr.openjdk.java.net/~chegar/8071472/01/>
>>
>> This version also includes a number of changes to readObject implementations in the base module, to replace unsafe usage with this field setter API.
>
> Hi Chris,
>
> It's good that you included changes to some readObject() implementations
> in base module as these are the 1st real-world examples of API use. This
> way we can get some experience as to whether the API is adequate. This
> experience includes subtleties of assignment checking on finals. I think
> it would be desirable that if this API is included in JDK9, more
> migration from Unsafe to this API is gradually attempted before the API
> is frozen for JDK9.

I think we are mainly in agreement with regard to the level of the API. 
As with all new API's, the more bake time in the release the better. If 
we accept the general approach, then further tweaks can be done before 9 
ships.

I plan to file bugs against specific component areas, or by module, to 
remove unsafe usage and replace it with this API. There should be enough 
time to do this in 9, and the experience from doing so can feedback into 
the API, if needed.

> As for the changes, I went through them and I think they are OK. Just
> one nit: private static FieldSetterContext.finalInstanceFieldCount()
> helper method could be moved to inside the FieldSetterContext.FieldsMap
> nested class as it is only used from it's constructor. This way you
> avoid generating synthetic access methods.

Thanks, done.

> Also in ObjectInputStream.fieldSetter() javadoc, in the following statement:
>
> Returns the |FieldSetter| instance associated with current object and
> type being deserialized, which gives write access to the *types*
> declared instance fields, including final, during the |readObject| callback.

Fixed.

> I think it should be changed from: "types" -> "type's" (or class' for
> that matter as only classes can have instance fields).
>
> Regards, Peter

-Chris.