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

Reply via email to