Re: RFR (M) 212939: Add space after if/while/for/switch and parenthesis

Wed, 07 Nov 2018 14:11:48 -0800

Hi Jc,

It is Okay if it is covered by different bugs.

Thanks,
Serguei


On 11/7/18 13:59, JC Beyler wrote:
Hi Serguei,

I have https://bugs.openjdk.java.net/browse/JDK-8212960 open for that. I like doing things in small contained steps for reviewing and sanity checking. But if you want, I can run a script to fix all these cases now and resent a RFR.

Let me know what you prefer, I can see the pros/cons of both ways :-),
Jc

On Wed, Nov 7, 2018 at 1:01 PM [email protected] <[email protected]> wrote:
Hi Jc,

+1 LGTM.

One question:
  Would it be convenient at the same time to fix missing spaces around commas and comparisons in loops and conditions.
  One more case is function call arguments but I did not see any instances of it.

  Some examples:

  http://cr.openjdk.java.net/%7Ejcbeyler/8212939/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/Breakpoint/breakpoint001/breakpoint001.cpp.udiff.html
-    for(i=0; i<METH_NUM; i++)
+    for (i=0; i<METH_NUM; i++)

  http://cr.openjdk.java.net/%7Ejcbeyler/8212939/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jvmti/ClassLoad/classload001/classload001.cpp.udiff.html
-    for(i=0; i<EXP_SIG_NUM; i++)
+    for (i=0; i<EXP_SIG_NUM; i++)
         clsEvents[i] = 0;
 
-    for(i=0; i<UNEXP_SIG_NUM; i++)
+    for (i=0; i<UNEXP_SIG_NUM; i++)


  http://cr.openjdk.java.net/%7Ejcbeyler/8212939/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/stress/jni/libjnistress004.cpp.udiff.html

  There are many cases like below:
-    for(i=0;i<DIGESTLENGTH;i++) {
+    for (i=0;i<DIGESTLENGTH;i++) {
. . .
-    for(i=0;i<len;i++)
-    if(ch[i]!=tmp[i]) {
+    for (i=0;i<len;i++)
+    if (ch[i]!=tmp[i]) {

Thanks,
Serguei


On 11/7/18 12:23, Chris Plummer wrote:
Hi JC.

In Callbacks.cpp you missed the needed extra indent on the lines following the "if":

 406       if (!NSK_JVMTI_VERIFY(jvmti->GetFieldName(targetClass,
 407 targetFields[field],
 408 &objects_info[object].fields[field].name,
 409 &objects_info[object].fields[field].signature,
 410 NULL)))

 481       if ((objects_info[object].fields[field].primitive && !objects_info[object].collected)
 482          || !objects_info[object].fields[field].collected) {

Otherwise looks good. I don't need to see another webrev.

thanks,

Chris

On 11/7/18 11:20 AM, JC Beyler wrote:
Hi all,

Could I get a review for a relatively straight-forward space transformation webrev? This adds spaces between keywords and '(' and fixes the case "){" across C++ files in vmTestbase.

Webrev: http://cr.openjdk.java.net/~jcbeyler/8212939/webrev.00/ <http://cr.openjdk.java.net/%7Ejcbeyler/8212939/webrev.00/>
Bug: https://bugs.openjdk.java.net/browse/JDK-8212939

Thanks!
Jc






--

Thanks,
Jc

Reply via email to