On Wed, 26 Jan 2022 16:27:24 GMT, Alex Menkov <amen...@openjdk.org> wrote:

>> Changes:
>> - ClassFileReconstituter is updated to restore "MethodParameters" attribute;
>> - handling of the attribute in VM_RedefineClasses is moved to be consistent 
>> with other code (like local variable table);
>> - copied ClassTransformer class (from test/jdk/com/sun/jdi/lib/jdb) to 
>> /test/lib as it's used by tests from hotspot and jdk (and also by test from 
>> Valhalla repo); Will file a follow up issues to updates tests and remove the 
>> class from test/jdk/com/sun/jdi/lib/jdb
>
> Alex Menkov has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Simplified the test, added comments

Thanks for adding comments and simplifying the test. Overall the changes look 
good. Just a couple of minor suggestions inlined below/

src/hotspot/share/prims/jvmtiRedefineClasses.cpp line 3692:

> 3690:                 elem[j].name_cp_index = new_cp_index;
> 3691:             }
> 3692:         }

Indentation should be 2, not 4.

test/jdk/java/lang/instrument/RetransformWithMethodParametersTest.java line 54:

> 52:  * correctly handles MethodParameter attribute:
> 53:  * - classfile bytes passed to transformers (and JVMTI ClassFileLoadHook 
> event callback) contain the attribute;
> 54:  * - the attribute is updated.

I think you could use a bit more than this. Basically summarize the 3 test 
cases in doRunTests().

-------------

Changes requested by cjplummer (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/7180

Reply via email to