On Wed, 20 Dec 2023 14:28:39 GMT, Roger Riggs <rri...@openjdk.org> wrote:

>> Raffaello Giulietti has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Changes according to reviewer's comments.
>
> It would also be useful/interesting to include a test that checks every 
> Serializable class (by  Invoking `ObjectStreamClass.lookup(clazz)`) in the 
> Java runtime and reports any jfr events.  
> Fixing them would be a separate task.  The compiler warnings from last year 
> should have fixed most/many non-conforming classes.

@RogerRiggs Do you mean a permanent test in the codebase, or just a throwaway 
run? Anyway, interesting suggestion.

> src/java.base/share/classes/java/io/SerializationMisdeclarationChecker.java 
> line 113:
> 
>> 111:             if (longFromStatic(f) == null) {
>> 112:                 commitEvent(SUID_CONVERTIBLE_TO_LONG,
>> 113:                         SUID_NAME + " must be convertible to long via 
>> widening to be effective");
> 
> The serialization spec only shows using long.  If any recommendation is made 
> it should be to declare the field as a `long`

There's a check on the type at L.104 which is about the "should" 
recommendation, since serialization does not care about the type of the field 
being `long`.

This check is about the value at runtime, which is a "must" because 
serialization expects it to be convertible to long by widening.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/17129#issuecomment-1864628102
PR Review Comment: https://git.openjdk.org/jdk/pull/17129#discussion_r1432827176

Reply via email to