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

[Daniel D. Daugherty]

I would also write code like what David shows... :-)

Dan


On 11/29/18 5:48 AM, David Holmes wrote:
> I vote for the spaces around { and }
>
> I would write:
>
> int foo() { return 1; }
>
> so would also write:
>
> static jvmtiEvent testEvents[] = { JVMTI_EVENT_THREAD_START, 
> JVMTI_EVENT_THREAD_END };
>
> Cheers,
> David
>
> On 29/11/2018 8:29 pm, serguei.spitsyn at oracle.com wrote:
>> 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
>>>>>>
>>>>
>>