On Wed, 5 Feb 2025 05:35:58 GMT, David Holmes <dhol...@openjdk.org> wrote:
>> Coleen Phillimore has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Fix test that knows which fields are hidden from reflection in jvmci.
>
> src/java.base/share/classes/java/lang/Class.java line 239:
>
>> 237: * generated.
>> 238: */
>> 239: private Class(ClassLoader loader, Class<?> arrayComponentType,
>> ProtectionDomain pd) {
>
> If this constructor is not used then why do we need to add the PD argument,
> rather than just set it to null? For that matter why do we even need the
> field if nothing is ever setting it? I'm missing something here.
@DanHeidinga suggested this for my other PR as a convention that's used for the
j.l.Class constructor.
> src/java.base/share/classes/java/lang/Class.java line 2701:
>
>> 2699:
>> 2700: @Stable
>> 2701: private transient final ProtectionDomain protectionDomain;
>
> Isn't `@Stable` superfluous with a final field?
Yes, I thought I removed it but that was probably the other PR.
> src/java.base/share/classes/java/lang/Class.java line 2722:
>
>> 2720: */
>> 2721: public ProtectionDomain getProtectionDomain() {
>> 2722: if (protectionDomain == null) {
>
> Does this imply the class is a primitive class?
No I believe classes that are bootstrap classes don't generally have a
protection domain.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23396#discussion_r1943394960
PR Review Comment: https://git.openjdk.org/jdk/pull/23396#discussion_r1943393526
PR Review Comment: https://git.openjdk.org/jdk/pull/23396#discussion_r1943394193
