On Wed, 26 Jan 2022 16:27:24 GMT, Alex Menkov <[email protected]> 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
