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
