[OpenJDK 2D-Dev] RFR JDK-8211300: Convert C-style array declarations in Java code

[Phil Race]

The indentation now looks inconsistent in these two files. Maybe we can 
manually fix that up ?

http://cr.openjdk.java.net/~tvaleev/webrev/8211300/r1/src/java.desktop/share/classes/javax/swing/tree/FixedHeightLayoutCache.java.udiff.html
http://cr.openjdk.java.net/~tvaleev/webrev/8211300/r1/src/java.desktop/share/classes/javax/swing/tree/VariableHeightLayoutCache.java.udiff.html

Sergey, are you going to commit this, or am I ?

The changes in demos + accessibility are best handled under a different 
bug id since
this one is quite far along ..

-phil.

On 10/03/2018 03:06 AM, Tagir Valeev wrote:
> Hello!
>
>> Given what Sergey has done, I don't need to repeat that work.
>> I am now just looking for formatting anomalies, but there is a lot to
>> look at.
>> I should be able to OK this tomorrow and we can then commit it on your
>> behalf.
> Thanks, that would be great!
>
>> Have  you looked at the other client modules ? datatransfer, accessibility,
>> jdk.unsupported.desktop, or the client demos ?
> No, I haven't. I was not aware that there are other client modules. I
> assume that client demos are in src/demo/share directory, right?
> I found no hits in datatransfer and jdk.unsupported.desktop, 4 files
> in accessibility and 71 files in demos. Here's separate webrev for
> these 75 files:
> http://cr.openjdk.java.net/~tvaleev/patches/c_style_arrays_demos_accessibility/
> Should I post a separate JBS issue for this (or two issues) or it
> could be incorporated within existing issue?
> Should I post separate reviews for accessibility and demos or it's
> fine to have them together?
>
>> If you are crazy (just an idea) then maybe even tests !
>> But that latter one is a low priority.
> Let's postpone tests. I'd like to move to some core modules like
> java.base until my volunteering energy runs out :-)
>
>>   > It appeared that compound declarations like `byte r[], g[], b[];`
>>   > are converted to `byte[] r;byte[] g; byte[] b;
>>
>> Seems like someone should fix their tool :-)
> Yeah, this is what I'm thinking about (I'm IDEA developer). See, IDEA
> processes every hit separately, and we have three independent hits
> here. So when it processes r[] it doesn't know that I want to convert
> others as well, thus it has to split the compound declaration into
> `byte[] r;byte g[], b[];` (when formatting is active, they are placed
> on separate lines, but I switched off the formatting to simplify the
> changeset). The same is repeated for `g` and `b` independently and we
> have unpleasant result. Probably we can fix this issuing single
> warning per declaration and suggesting to fix the whole declaration or
> nothing (it's unlikely that somebody would like to fix only some of
> these arrays), but then the question is which source element should be
> highlighted. Now we highlight three pairs of brackets in three
> individual warnings, but if we have a single warning, we should
> highlight something continuous. We may highlight the whole
> declaration, but what if it also contains non-convertible things like
> `byte r[], g, b[]`. Looks like a simple problem, but I don't see the
> trivial solution. But ok, it's an off-topic :-)
>
> With best regards,
> Tagir Valeev
>
>> -phil.
>>
>>
>> On 10/1/18, 6:24 PM, Sergey Bylokhov wrote:
>>> Looks fine to me.
>>> I have checked the patch, have build the change and run some tests on
>>> our supported platforms.
>>>
>>> On 30/09/2018 20:29, Tagir Valeev wrote:
>>>> Hello!
>>>>
>>>> Please review JDK-8211300 [1] this change: [2]. Also it needs a
>>>> sponsor as I have only JDK author status in OpenJDK Census [3].
>>>>
>>>> The discussion in core-libs-dev [4] shows that it's desired to get rid
>>>> of old style array declarations like `int array[]` and massively
>>>> convert them to `int[] array`. I volunteered to work on this. As Alan
>>>> Bateman noted [5], java.desktop module is pushed to separate repo, so
>>>> it would be better to process it separately, thus I'd like to start
>>>> with it.
>>>>
>>>> The changeset was created in the following steps:
>>>> * Import JDK sources to IntelliJ IDEA
>>>> * Mark java.desktop/aix/classes, java.desktop/macosx/classes,
>>>> java.desktop/unix/classes, java.desktop/solaris/classes,
>>>> java.desktop/windows/classes and java.desktop/share/classes as source
>>>> roots
>>>> * Disable automatic formatting on the whole project
>>>> * Run the inspection "C-style array declaration"
>>>> * Apply the quick-fix massively
>>>> * Perform regex replace over changed files only:
>>>> Search: Copyright \(c\) (\d+), (\d+), Oracle and/or its affiliates.
>>>> All rights reserved.
>>>> Replace: Copyright \(c\) $1, 2018, Oracle and/or its affiliates. All
>>>> rights reserved.
>>>> * Perform regex replace over changed files only:
>>>> Search: Copyright \(c\) (\d+), Oracle and/or its affiliates. All
>>>> rights reserved.
>>>> Replace: Copyright \(c\) $1, 2018, Oracle and/or its affiliates. All
>>>> rights reserved.
>>>> * It appeared that compound declarations like `byte r[], g[], b[];`
>>>> are converted to `byte[] r;byte[] g; byte[] b;` which does not look
>>>> nice while compilable. I found three such cases via simple regexp
>>>> search in BMPImageReader#658, BMPImageWriter#325 and
>>>> AreaAveragingScaleFilter#66 and fixed them manually.
>>>>
>>>> In total 339 files were changed with 1335 lines of array declaration
>>>> updates and 310 lines of copyright updates. I reviewed the generated
>>>> patch by eyes, but only partially, because it's too big. Also I
>>>> performed some kind of simple sanity check using regexps:
>>>>
>>>> $ grep '^+[^+]' jdk.patch | grep -v 'Oracle and/or its affiliates. All
>>>> rights reserved'|grep -v '\[\]'|wc
>>>>         0       0       0
>>>> (check that every updated line contains either copyright message or [])
>>>>
>>>> With best regards,
>>>> Tagir Valeev.
>>>>
>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8211300
>>>> [2] http://cr.openjdk.java.net/~tvaleev/webrev/8211300/r1/
>>>> [3] http://openjdk.java.net/census#tvaleev
>>>> [4]
>>>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-September/055390.html
>>>> [5]
>>>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-September/055759.html
>>>>
>>>