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

[JC Beyler]

Hi both,

I actually am letting you both decide which you would rather do :-).

For what it's worth, for *cpp files:
   A) there are 83 files in vmTestbase that have on the same line { and }
where } does not have a space before (659 cases of {} in the vmTestbase
(grep "{.*[^ {]}")
        - there are 98 files in all (including the vmTestbase files) in the
whole test/hotspot directory () (with 790 cases)
        - there are 281 files that contain it in the whole jdk tree, with
1776  cases in all (so 10% of the files contain at least one case)

  (Note I'm not counting the number of times grep "{[^ }].*}"  shows up here

  B) there are 10 files in vmTestbase that have on the same line { and ;}
where ;} does not have a space between (19 cases of {} in the vmTestbase
(grep "{.*;}")
        - there are 10 files in all (including the vmTestbase files) in the
whole test/hotspot directory (19 cases in all); so all are in the vmTestbase
        - there are 44 files that contain it in the whole jdk tree, with
111 cases in all

(A) would close 821822 in general and we could determine if you wanted it
across the codebase, just for the vmTestbase or for the hotspot tests in
general
(B) is a much smaller clean-up, we could again do it across the codebase
with 44 files touched; or just in the vmTestbase since the hotspot tests do
not have the case except there.
(C) Not do it :)

What would you prefer I do ? I have an awk script for (A), a simple sed
command for (B), a few clicks to close the bugs in JBS for (C);  so it's
easy either way :).
Jc




On Wed, Nov 28, 2018 at 6:18 PM Alex Menkov <alexey.menkov at oracle.com>
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. And to me for
> 1-line array initializers spaces do not improve readability.
>
> 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>
>
> --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
> >>>
> >
>


-- 

Thanks,
Jc
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20181128/1081bf6f/attachment.html>