RFR 8231314: java.time serialization warning cleanup

Roger Riggs Roger.Riggs at oracle.com
Mon Sep 23 14:56:14 UTC 2019


Updated to  wrap long lines and remove unneeded @SuppressWarnings.


On 9/23/19 4:44 AM, Peter Levart wrote:
> Once more, for the list (sorry)...
> Hi,
> On 9/21/19 12:31 PM, Chris Hegarty wrote:
>> Roger,
>>> On 20 Sep 2019, at 19:51, Roger Riggs <Roger.Riggs at oracle.com> wrote:
>>> Please review code cleanup that will remove the need to suppress 
>>> soon to be introduced
>>> warnings [1] about static typing of serialization related fields.
>>> A few of the implementation Ser classes that serialize java.time 
>>> types are affected.
>>> Webrev:
>>> http://cr.openjdk.java.net/~rriggs/webrev-warn-serializable-8231314/
>> I think the change is fine, just a few comments..
>> - AbstractChronology is not necessarily is not always Serializable, 
>> but writeReplace expects that it is.
>> Since this is a package-private method, then I assume that it will 
>> only ever be called by Serializable subtypes or the serialization 
>> framework itself.
Correct, chrono/Ser is only used for built-in subtypes of 

I think there is a separate design choice that should be questioned.
In retrsospect I think it should required subclasses of 
AbstractChronology be Serializable.
A number of java.time. serializable types can refer to a Chronology and 
if Chronology is not
serializable, then they won't be either.  If AbstractChronology extended 
other implementations would be aware of the expectation and have to make 
a deliberate choice.
But this is a separate issue.

>> - time/chrono/Ser.java : it’s kinda awkward to have to cast the 
>> return of readExternal, especially since the set of types is locked 
>> down, but without these readExternal methods returning xxxImpl types 
>> there is little choice.
> readExternal package-private methods could return Serializable, but 
> then those methods would have to cast. Perhaps this would be better as 
> it is "closer" to the real impl.
That would be better, but except for lambdas, I don't know of a way to 
cast to multiple interfaces.
> Another option (as I mentioned in previous message) would be to simply 
> mark the Ser field as transient. The fact that currently it always 
> holds an instance that *is* Serializable is just a coincidence, 
> because it acts as a serialization proxy for Serializable 
> implementations. Nothing in the logic of Ser class "requires" the 
> instance assigned  to the field to be in fact Serializable.
That makes sense for the general non-serializable case, but in the case 
of chrono/Ser, all of the implementations
are known to be Serializable.

Thanks, Roger

> Regards, Peter

More information about the core-libs-dev mailing list