On Wed, 20 Dec 2023 14:28:39 GMT, Roger Riggs <[email protected]> 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
