RFR (M) 8213003: Remove more assignments in ifs for vmTestbase/[a-s]
chris.plummer at oracle.com
Wed Oct 31 18:52:38 UTC 2018
On 10/31/18 11:30 AM, serguei.spitsyn at oracle.com wrote:
> It prints the FILE/LINE which is enough to identify the error point.
Yes, but it also prints the JNI calls.
> 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.
Agreed. Somewhat of a tossup now as to whether or not this change should
be done. I'm fine either way.
> 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), \
>> 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.
>> On 10/31/18 3:16 AM, Claes Redestad wrote:
>>> here's a case that your script seems to miss:
>>> 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
>>> 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
More information about the serviceability-dev