Re: RFR (M) 8210182: Remove macros for C compilation from vmTestBase but non jvmti

[serguei.spit...@oracle.com]

Hi Jc, The fix looks great as it looks much more simple now! One minor suggestion about this file update: http://cr.openjdk.java.net/%7Ejcbeyler/8210182/webrev.02/test/hotspot/jtreg/vmTestbase/nsk/share/jpda/libNativeMethodsTestThread.cpp.udiff.html   An example: + env->CallVoidMethod(thisObject, env->GetMethodID(klass, "log", "(Ljava/lang/String;)V"), + message);   I'd suggest either to place one call at one line, or place each argument on a different line like this: + env->CallVoidMethod(thisObject, + env->GetMethodID(klass, "log", "(Ljava/lang/String;)V"), + message); There are several similar cases in this file. No need in another webrev if you fix this. Thanks, Serguei On 8/30/18 12:03, JC Beyler wrote: Hi Alex, I've fixed those cases and found an extra one. I'll revise my scripts for the next folder so these no longer happen (bad regexp that eagerly finds a ')'. I had launched in parallel a test for vmTestBase, it finished with a few failures that might be linked to this so I have relaunched the whole vmTestBase and will post the results here. The new webrev with the fixes is here: http://cr.openjdk.java.net/~jcbeyler/8210182/webrev.02/ Thanks again for the review, Jc On Thu, Aug 30, 2018 at 10:48 AM Alex Menkov <alexey.men...@oracle.com> wrote: Hi Jc, It looks much better now. BTW did you run all the tests? test/hotspot/jtreg/vmTestbase/nsk/share/jni/JNIreferences.cpp "(V" should be  "()V":           // notify another thread that JNI local reference has been created -        JNI_ENV_PTR(env)->CallVoidMethod(JNI_ENV_ARG_3(env, createWicket, JNI_ENV_PTR(env)->GetMethodID(JNI_ENV_ARG_4(env, klass, "unlock", "()V")))); +        env->CallVoidMethod(createWicket, env->GetMethodID(klass, "unlock", "(V"));           // wait till JNI local reference can be released (it will heppen then we will leave the method) -        JNI_ENV_PTR(env)->CallVoidMethod(JNI_ENV_ARG_3(env, deleteWicket, JNI_ENV_PTR(env)->GetMethodID(JNI_ENV_ARG_4(env, klass, "waitFor", "()V")))); +        env->CallVoidMethod(deleteWicket, env->GetMethodID(klass, "waitFor", "(V"));   } --alex On 08/29/2018 22:01, JC Beyler wrote: > Hi all, > > A follow-up to Igor's work on getting tests in C++, I am working on > simplifying the macros in the tests from the vmTestBase. The full change > being a bit too large, I'm cutting it up in pieces to be easier to > review and integrate. > > Here is the first part, it changes all vmTestbase tests outside the > vmTestbase/jvmti subfolder: > > Webrev: http://cr.openjdk.java.net/~jcbeyler/8210182/webrev.01/ > <http://cr.openjdk.java.net/%7Ejcbeyler/8210182/webrev.01/> > Bug: https://bugs.openjdk.java.net/browse/JDK-8210182 > > Thanks! > Jc -- Thanks, Jc