On Tue, 12 Sep 2023 16:37:28 GMT, Mandy Chung <mch...@openjdk.org> wrote:
>> This reimplements >> `sun.reflect.ReflectionFactory::newConstructorForSerialization` with method >> handles. >> >> This API currently generates the bytecode which fails the verification >> because `new C; invokespecial A()` where the given class `C` and invoke a >> no-arg constructor of `C`'s first non-`Serializable` superclass `A` is not a >> valid operation per the VM specification. VM special cases the classes >> generated for reflection to skip verification for the constructors generated >> for serialization and externalization. This change will allow such VM hack >> to be removed. >> >> A `jdk.reflect.useOldSerializableConstructor` system property can be set to >> use the old implementation in case if customers run into any compatibility >> issue. I expect this change has very low compatibility risk. This system >> property is undocumented and will be removed in a future release. > > Mandy Chung 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 three additional > commits since the last revision: > > - Merge branch 'master' of https://github.com/openjdk/jdk into > reflect-serializable-ctor > - minor cleanup > - Reimplement ReflectionFactory::newConstructorForSerialization with method > handle src/java.base/share/classes/java/lang/invoke/MethodHandles.java line 3552: > 3550: } > 3551: } > 3552: return false; Does it break encapsulation too much to export and use the same method from jdk.internal.reflect.MethodHandleAccessorFactory. Maybe not worth it for a small utility method. src/java.base/share/classes/jdk/internal/reflect/ReflectionFactory.java line 384: > 382: > getConstructorAnnotations(constructorToCall), > 383: langReflectAccess. > 384: > getConstructorParameterAnnotations(constructorToCall)); This would be easier to read if either the line length or the indentation was modified. src/java.base/share/classes/jdk/internal/reflect/ReflectionFactory.java line 400: > 398: } > 399: > 400: Extra blank line. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/15600#discussion_r1323453214 PR Review Comment: https://git.openjdk.org/jdk/pull/15600#discussion_r1318801680 PR Review Comment: https://git.openjdk.org/jdk/pull/15600#discussion_r1318803452