On Fri, 11 Oct 2024 15:57:50 GMT, Adam Sotona <[email protected]> wrote:
>> Please review this patch that adds a test by @nizarbenalla to perform null
>> checks across the ClassFile API. This is an updated version of #20556 that
>> minimizes impact on our implementation code.
>>
>> Notes:
>> 1. There's one change in `MethodHandleProxies` to explicitly use platform
>> class loader instead of `null` for boot class loader. Tests work fine.
>> 2. The null check test uses the same set of classes as CorpusTest to make it
>> suitable for tier 1.
>> 3. The test is not comprehensive; currently it is permissive toward IAE,
>> because sometimes some substitution arguments are invalid, especially
>> primitive ClassDesc or Opcode with wrong types.
>
> src/java.base/share/classes/jdk/internal/classfile/impl/ClassRemapperImpl.java
> line 276:
>
>> 274: @Override
>> 275: public ClassDesc map(ClassDesc desc) {
>> 276: if (desc == null) return null;
>
> I'm not sure this won't break some use case.
> Here I would say null -> null mapping might be OK.
Makes sense. I will push the changes when you finish the review.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21458#discussion_r1797158151
