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