RFR (M/XXS) cleanup more non-indent white space issues (8047156)

[Lois Foltan]

On 6/18/2014 9:54 AM, Daniel D. Daugherty wrote:
> Lois,
>
> Thanks for the review. More replies embedded below.

Hi Dan,

Thanks for the explanation.  Any chance you would clean up these two 
instances manually since you are in those files anyways?  Either way, I 
like the clean up and am good with your changes.

Lois

>
>
> On 6/18/14 5:40 AM, Lois Foltan wrote:
>> Hi Dan,
>>
>> Looks good.  One comment,  why wasn't the space removed after the 
>> function name but before the "(" for the start of the parameters?
>>
>> src/share/vm/runtime/mutex.cpp
>> -#define TRACE(m) { static volatile int ctr = 0 ; int x = ++ctr ; if ((x & (x-1))==0) { ::printf ("%d:%s\n", x, #m); ::fflush(stdout); }}
>> +#define TRACE(m) { static volatile int ctr = 0; int x = ++ctr; if ((x & (x-1))==0) { ::printf ("%d:%s\n", x, #m); ::fflush(stdout); }}
>
> Single line code blocks are difficult to clean. The '<space>;' instances
> were easy for the script to clean, but the '::printf<space>(' is much
> harder because it is a function call in the middle of other things that
> look like function calls ('TRACE(m)' and '::fflush(stdout)')
>
>
>> src/share/vm/runtime/synchronizer.hpp:
>> -  void waitUninterruptibly (TRAPS) { ObjectSynchronizer::waitUninterruptibly (_obj, 0, CHECK);}
>> +  void waitUninterruptibly (TRAPS) { ObjectSynchronizer::waitUninterruptibly (_obj, 0, CHECK); }
>
> Single line function definitions are difficult, but I would have
> expected 'waitUninterruptibly<space>(TRAPS)' to work... Hmmm...
> wait... the 'TRAPS' parameter doesn't match the usual format for
> parameters in a function definition, i.e., <type> <param>. It's a
> single word and that's probably why the filter skipped it.
>
> The interior 'ObjectSynchronizer::waitUninterruptibly<space>(' is
> harder for the filter to recognize so it was skipped.
>
>
>>
>> I'm just curious because there are other places where removing the 
>> space does occur?  For example:
>>
>> src/share/vm/runtime/mutex.cpp
>> -    const intptr_t u = CASPTR (&_LockWord, v, v|_LBIT) ;
>> -    if (v == u) return 1 ;
>> -    v = u ;
>> +    if ((v & _LBIT) != 0) return 0;
>> +    const intptr_t u = CASPTR(&_LockWord, v, v|_LBIT);
>
> Right. The 'CASPTR(...)' matches the pattern for a function call so
> it got cleaned up.
>
> The big flaw is that my script isn't a real parser. It's a collection
> of heuristic pattern matchers and filter/edit lines. The real solution
> is a parser that not only tokenizes C++, but keeps track of the white
> space associated with each token. Once you have such a tree, you can
> apply a set of rules that changes/adds whitespace as needed on a per
> token basis and then outputs the resulting tree as a modified source
> file.
>
> Dan
>
>
>>
>> Thanks,
>> Lois
>>
>>
>> On 6/17/2014 9:12 PM, Daniel D. Daugherty wrote:
>>> Greetings,
>>>
>>> I forgot to update my list of files to clean for the latest version
>>> of the cleanup bucket so I missed these seven files when I sent out
>>> the review for:
>>>
>>>     JDK-8046758 cleanup non-indent white space issues prior to 
>>> Contended
>>>                 Locking cleanup bucket
>>> https://bugs.openjdk.java.net/browse/JDK-8046758
>>>
>>> I have the fix for the following bug ready for JDK9 RT_Baseline:
>>>
>>>     JDK-8047156 cleanup more non-indent white space issues prior to
>>>                 Contended Locking cleanup bucket
>>> https://bugs.openjdk.java.net/browse/JDK-8047156
>>>
>>> This is both a (M)edium and e(X)tra e(X)tra (S)mall review! Here is the
>>> URL for the (M)edium webrev:
>>>
>>> http://cr.openjdk.java.net/~dcubed/8047156-webrev/0-jdk9-hs-rt/
>>>
>>> Since these are white space fixes, I recommend the udiff links instead
>>> of the frames links. Of course, some folks prefer the patch file itself
>>> for white space fixes.
>>>
>>> Here is the URL for the e(X)tra e(X)tra (S)mall webrev which was 
>>> generated with a version of webrev that ignores all whitespace changes:
>>>
>>> http://cr.openjdk.java.net/~dcubed/8047156-webrev/0-jdk9-hs-rt-no_ws/
>>>
>>> Again, I recommend the udiff links since that's the fastest way to see
>>> that webrev_no_ws found no non-white space changes. The patch file for
>>> this webrev shows all the white space changes.
>>>
>>> Thanks, in advance, for any comments, questions or suggestions.
>>>
>>> At the end of the review cycle for this fix, I will attach the 
>>> version of the do_space_filter.ksh script used to do the work to 
>>> JDK-8047156.
>>>
>>> Dan
>>>
>>>
>>
>