Post by s***@oracle.comHi Alex,
Yes that is true, I had initially only done the one-liner {} because
I was unsure about the cases above :-)
Webrev: http://cr.openjdk.java.net/~jcbeyler/8214417/webrev.01/
Bug: https://bugs.openjdk.java.net/browse/JDK-8214417
This was tested on my dev machine for all changed tests.
Thanks!
Jc
On Thu, Nov 29, 2018 at 2:26 PM Alex Menkov
Ok, guys, you win :)
Looks like only 1-line initializers are fixed, so now we have mixed
ClearFieldAccessWatch/clrfldw001/clrfldw001.cpp (lines 52-53 are not
NULL },
51 { "nsk/jvmti/ClearFieldAccessWatch/clrfldw001", "fld1",
"I", 1,
NULL },
52 {"nsk/jvmti/ClearFieldAccessWatch/clrfldw001", "fld2",
53 "Lnsk/jvmti/ClearFieldAccessWatch/clrfldw001a;", 0, NULL},
54 { "nsk/jvmti/ClearFieldAccessWatch/clrfldw001a", "fld3",
"[I", 0,
NULL },
The same in ClearFieldModificationWatch/clrfmodw001/clrfmodw001.cpp
51 { "nsk/jvmti/ClearFieldModificationWatch/clrfmodw001", "fld1",
"I", 1, NULL },
52 {"nsk/jvmti/ClearFieldModificationWatch/clrfmodw001", "fld2",
53 "Lnsk/jvmti/ClearFieldModificationWatch/clrfmodw001a;",
0, NULL},
54 { "nsk/jvmti/ClearFieldModificationWatch/clrfmodw001a",
"fld3",
"[I", 0, NULL },
GetClassSignature/getclsig006/getclsig006.cpp (44-51)
43 { "getclsig006",
"Lnsk/jvmti/GetClassSignature/getclsig006;",
"NULL" },
44 {"getclsig006b",
"Lnsk/jvmti/GetClassSignature/getclsig006b;",
45 "<L:Ljava/lang/String;>Ljava/lang/Object;"},
46 {"getclsig006c",
"Lnsk/jvmti/GetClassSignature/getclsig006c;",
47
"<A:Ljava/lang/Object;B:Ljava/lang/Integer;>Ljava/lang/Object;"},
(and others files)
--alex
> I would also write code like what David shows... :-)
>
> Dan
>
>
>> I vote for the spaces around { and }
>>
>>
>> int foo() { return 1; }
>>
>>
>> static jvmtiEvent testEvents[] = { JVMTI_EVENT_THREAD_START,
>> JVMTI_EVENT_THREAD_END };
>>
>> Cheers,
>> David
>>
>>> Hi Alex,
>>>
>>>
>>>> Hi Serguei,
>>>>
>>>>> 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
>>>> <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
>>>>>
>>>>>
>>>>>> I don't see such rule (I suppose
>>>>>> https://wiki.openjdk.java.net/display/HotSpot/StyleGuide is
>>>>>> correct link?)
>>>>>>
>>>>>> --alex
>>>>>>
>>>>>>>> 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
>>>>>>>>
>>>>>>>>
>>>>>>>>> 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.
>>>>>>>>>
http://cr.openjdk.java.net/~jcbeyler/8214417/webrev.00/
>>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8214417
>>>>>>>>>
>>>>>>>>> Thanks for your reviews :-),
>>>>>>>>> Jc
>>>>>>>
>>>>>
>>>
>
--