RFR (L/S) manual cleanup of white space issues (8057109)

[Daniel D. Daugherty]

On 9/8/14 6:38 AM, Frederic Parain wrote:
> Dan,
>
> Thank you for this additional cleaning. It looks good to me,
> only a few minor issues (feel free to ignore them):
>
> src/os/bsd/vm/os_bsd.cpp:1937-1943
>
>   Why typedef statements are not aligned with #define statements?

We're in a globals section of the code so the prevailing indent
level == 0 so that's why the typedef line is at indent == 0.
Pre-processor lines are passed through without modification and
we don't interpret them as having an indent effect on regular code
lines so the #ifdef does not cause the typedef... line to be indented.

We have been evolving our pre-processor usage to reflect some
indenting rules so:

#ifdef FOO
#define BAR
#endif

becomes

#ifdef FOO
   #define BAR
#endif

which works fine for blocks that contain all pre-processor lines,
but not for combinations of regular code and pre-processor lines.
This is an unfortunate collision between two different rule sets
that fortunately doesn't happen in too many places.

For now, I've added a blank line after the typedef to make the
difference a little less jarring.

Note: One place we are not doing indents is in the header guards, e.g.:

#ifndef OS_BSD_VM_OS_BSD_HPP
#define OS_BSD_VM_OS_BSD_HPP

because that would cause the entire body of the header to be
indented...



> src/os/windows/vm/os_windows.cpp:
>
>   1606     if (lib_arch == arch_array[i].arch_code)
>   1607       lib_arch_str = arch_array[i].arch_name;
>   1608     if (running_arch == arch_array[i].arch_code)
>   1609       running_arch_str = arch_array[i].arch_name;
>
>   Brackets around the conditioned statements could make the code
>   easier to read.

Fixed. And then I made a pass to fix similar lines.


>
>     if (lib_arch == arch_array[i].arch_code) {
>       lib_arch_str = arch_array[i].arch_name;
>     }
>     if (running_arch == arch_array[i].arch_code) {
>       running_arch_str = arch_array[i].arch_name;
>     }
>
>   lines 4442-4443: do we really need two lines of empty comments?

Fixed. And then I made a pass to fix similar lines.

Dan


>
> Regards,
>
> Fred
>
> On 07/09/2014 21:13, Daniel D. Daugherty wrote:
>> Greetings,
>>
>> I have the fix for the following bug ready for JDK9 RT_Baseline:
>>
>>      JDK-8057109 manual cleanup of white space issues prior to Contended
>>                  Locking reorder and cache line bucket
>>      https://bugs.openjdk.java.net/browse/JDK-8057109
>>
>> This is both a (L)arge and a (S)mall review! Here is the URL for the
>> (L)arge webrev:
>>
>> http://cr.openjdk.java.net/~dcubed/8057109-webrev/0-jdk9-hs-rt/
>>
>> Since these are mostly 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 (S)mall webrev which was generated with a 
>> version
>> of webrev that ignores all whitespace changes:
>>
>> http://cr.openjdk.java.net/~dcubed/8057109-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 minimal non-white space changes. The patch file
>> for this webrev shows all the changes.
>>
>> These manual changes were motivated by various review comments made for
>> earlier fixes in the Contended Locking project. During those reviews, I
>> deferred making some of the requested format changes to this round of
>> manual changes. In most cases, the reviewers posted comments with one
>> or two examples of things they did not like and I've swept all 17 files
>> that we've targeted for cleanup in the Contended Locking project looking
>> for those style problems and fixing them. I've made a total of 12 passes
>> over the 17 files, but I cannot say that I've caught everything. Most
>> stuff that annoys people is fixed, but it's very difficult to catch and
>> fix them all without a real parser based solution.
>>
>> Thanks, in advance, for any comments, questions or suggestions.
>>
>> Dan
>>
>>
>