RFR (M) 8213003: Remove more assignments in ifs for vmTestbase/[a-s]

[serguei.spitsyn at oracle.com]

It prints the FILE/LINE which is enough to identify the error point.
But I agree that doing the assignments within the NSK_JNI_VERIFY was 
intentional as it gives more context.
 From the other hand it make the code ugly and less readable.
Not sure, what direction is better to go.

Thanks,
Serguei


On 10/31/18 11:21, Chris Plummer wrote:
> Hi Claes,
>
> It's good that you brought this up. NSK_JNI_VERIFY is always "on", but 
> moving the assignment outside of it does slightly change the behavior 
> of the macro (although the code still executes "correctly"):
>
> /**
>  * Execute action with JNI call, check result for true and
>  * pending exception and complain error if required.
>  * Also trace action execution if tracing mode enabled.
>  */
> #define NSK_JNI_VERIFY(jni, action)  \
> (nsk_ltrace(NSK_TRACE_BEFORE,__FILE__,__LINE__,"%s\n",#action), \
> nsk_jni_lverify(NSK_TRUE,jni,action,__FILE__,__LINE__,"%s\n",#action))
>
> So you will no longer see the JNI call in the trace or error output. 
> You will just see the comparison to the JNI result, which gives no 
> context as to the source of the result. So I'm now starting to think 
> that doing the assignments within the NSK_JNI_VERIFY was intentional 
> so we would see the JNI call in the trace output.
>
> Chris
>
> On 10/31/18 3:16 AM, Claes Redestad wrote:
>> Hi,
>>
>> here's a case that your script seems to miss:
>>
>> http://cr.openjdk.java.net/%7Ejcbeyler/8213003/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/GetClassLoaderClasses/clsldrclss002/clsldrclss002.cpp.udiff.html 
>>
>>
>>      if (!NSK_JNI_VERIFY(jni, (testedFieldID =
>>              jni->GetStaticFieldID(testedClass, FIELD_NAME, 
>> FIELD_SIGNATURE)) != NULL))
>>
>> I'd note that with some of your changes here you're unconditionally 
>> executing logic outside of NSK_JNI_VERIFY macros. Does care need be 
>> taken to wrap the code blocks in equivalent macros/ifdefs? Or are the 
>> NSK_JNI_VERIFY macros always-on in these tests (and essentially 
>> pointless)?
>>
>> /Claes
>>
>> On 2018-10-31 05:42, JC Beyler wrote:
>>> Hi all,
>>>
>>> I worked on updating my script and handling more assignments so I 
>>> redid a second pass on the same files to get all the cases. Now, on 
>>> those files the regex "if.* = " no longer shows any cases we would 
>>> want to fix.
>>>
>>> Could I get a review for this webrev:
>>> Webrev: http://cr.openjdk.java.net/~jcbeyler/8213003/webrev.00/
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8213003
>>>
>>> I tested this on my dev machine by passing all the tests that were 
>>> modified.
>>>
>>> Thanks!
>>> Jc
>>
>
>