RFR (L) 8214417: Add space after/before {} in vmTestbase/nsk/jvmti/[A-I] tests

[serguei.spitsyn at oracle.com]

Hi Alex,


On 11/28/18 18:18, Alex Menkov wrote:
> Hi Serguei,
>
> On 11/28/2018 16:44, serguei.spitsyn at oracle.com wrote:
>> There are some implicit rules, like unification and consistency.
>> We want a space or new line after '{' and before '}'.
>
> I believe this rule is about braces for code blocks (new line after 
> "{", "}" on a separate line), but this are array initializers.

Array initializers are blocs as well.
It make it a base for unification.


> And to me for 1-line array initializers spaces do not improve readability.

I politely disagree, at least, they improve it for me. :)

We can do one simple test.
What suggestion would make the Jc's awk script simpler?
If yours then I'm out.
Otherwise, why does it make simpler for script but not for humans?

Also, we can wait for one more opinion.


> So to me this looks like the last item from "Whitespace" is applicable 
> here:
> <cite>
> Try not to change whitespace unless it improves readability or 
> consistency. (Different editors indent differently, and spurious 
> indentation changes will just make integrations more difficult.)
> </cite>

It was Okay to break this rule when we decided to fix spacing over all 
the tests.

Thanks,
Serguei


> --alex
>
>> Why this case needs to have an exception?
>>
>> Thanks,
>> Serguei
>>
>>
>> On 11/28/18 16:18, Alex Menkov wrote:
>>> I don't see such rule (I suppose 
>>> https://wiki.openjdk.java.net/display/HotSpot/StyleGuide is correct 
>>> link?)
>>>
>>> --alex
>>>
>>> On 11/28/2018 16:05, serguei.spitsyn at oracle.com wrote:
>>>> On 11/28/18 15:43, Alex Menkov wrote:
>>>>> Hi Jc,
>>>>>
>>>>> In the JDK-8212771 review thread cleanup for "{}" was requested 
>>>>> for statements like
>>>>> test/hotspot/jtreg/vmTestbase/nsk/jvmti/unit/MethodBind/JvmtiTest/JvmtiTest.cpp, 
>>>>>
>>>>>
>>>>> +#define JVMTI_ERROR_CHECK(str,res) if (res != JVMTI_ERROR_NONE) { 
>>>>> printf(str); printf("%d\n",res); return res;}
>>>>>
>>>>> +#define JVMTI_ERROR_CHECK_EXPECTED_ERROR(str,res,err) if (res != 
>>>>> err) { printf(str); printf("unexpected error %d\n",res); return res;}
>>>>>
>>>>> I.e. something like ";}" -> "; }"
>>>>>
>>>>>
>>>>> I don't think changes like
>>>>>
>>>>> -static jvmtiEvent testEvents[] = {JVMTI_EVENT_THREAD_START, 
>>>>> JVMTI_EVENT_THREAD_END};
>>>>> +static jvmtiEvent testEvents[] = { JVMTI_EVENT_THREAD_START, 
>>>>> JVMTI_EVENT_THREAD_END };
>>>>>
>>>>> are required.
>>>>
>>>> It is better to have it - rules are rules.
>>>>
>>>> Thanks,
>>>> Serguei
>>>>
>>>>>
>>>>> --alex
>>>>>
>>>>>
>>>>> On 11/28/2018 11:20, JC Beyler wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> When working on a previous clean-up (JDK-8212771), I was asked to 
>>>>>> clean-up also spaces around {}.
>>>>>>
>>>>>> Here is the first batch out of 2 to fix these cases. Let me know 
>>>>>> what you think.
>>>>>>
>>>>>> Webrev: http://cr.openjdk.java.net/~jcbeyler/8214417/webrev.00/
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8214417
>>>>>>
>>>>>> Thanks for your reviews :-),
>>>>>> Jc
>>>>
>>