On Mon, 8 Jan 2024 18:15:36 GMT, Roger Riggs <rri...@openjdk.org> wrote:
>> Raffaello Giulietti has updated the pull request with a new target base due >> to a merge or a rebase. The incremental webrev excludes the unrelated >> changes brought in by the merge/rebase. The pull request contains 15 >> additional commits since the last revision: >> >> - Merge branch 'master' into 8275338 >> - Simplified event messages. >> Remove ckecker allocation. >> - Corrected @Label of event and of field. >> - Removed @module from test. >> - Merge branch 'master' into 8275338 >> - Renamed an event field. >> - Minor changes. >> - Removed event kind. >> serialVersionUID must have type long. >> Test now base on keyword search in event message. >> Commented test classes about misdeclarations. >> - Changes according to reviewer's comments. >> - Better name for a label, corrected name of removed field. >> - ... and 5 more: https://git.openjdk.org/jdk/compare/966dac39...9ca1f36d > > src/java.base/share/classes/java/io/SerializationMisdeclarationChecker.java > line 53: > >> 51: private static final Class<?>[] READ_OBJECT_NO_DATA_PARAM_TYPES = {}; >> 52: private static final Class<?>[] WRITE_REPLACE_PARAM_TYPES = {}; >> 53: private static final Class<?>[] READ_RESOLVE_PARAM_TYPES = {}; > > These could share a single zero length Class<?> array. Conceptually, these are independent types. There's no logical relationship between them. Sharing a zero length array would convey a false sense of logical sharing. > src/java.base/share/classes/java/io/SerializationMisdeclarationChecker.java > line 87: > >> 85: } >> 86: if (cl.isEnum()) { >> 87: commitEvent(cl, SUID_NAME + " in an enum class is not >> effective"); > > Is there a best practice that can be included in the message? "SUID should > not be declared"? Yes, that's perhaps clearer, will do. > src/java.base/share/classes/java/io/SerializationMisdeclarationChecker.java > line 113: > >> 111: } else if (cl.isEnum()) { >> 112: commitEvent(cl, SERIAL_PERSISTENT_FIELDS_NAME + >> 113: " in an enum class is not effective"); > > Is there best practice to include in the message? "SPFN should not be > declared"? Yes, that's perhaps clearer, will do. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/17129#discussion_r1445922807 PR Review Comment: https://git.openjdk.org/jdk/pull/17129#discussion_r1445924385 PR Review Comment: https://git.openjdk.org/jdk/pull/17129#discussion_r1445924424
