RFR (M): 8062808: Turn on the -Wreturn-type warning
Hohensee, Paul
hohensee at amazon.com
Mon Oct 5 16:40:22 UTC 2020
Thanks for your review, Volker.
Fixed src/bsd/makefiles/gcc.make. New webrevs
http://cr.openjdk.java.net/~phh/8030350/webrev.8u.hotspot.02/
http://cr.openjdk.java.net/~phh/8036122/webrev.8u.hotspot.02/
http://cr.openjdk.java.net/~phh/8030350.8036122/webrev.8u.hotspot.02/
http://cr.openjdk.java.net/~phh/8030350.8036122.8062808/webrev.8u.hotspot.03/
The 8062808 webrev
http://cr.openjdk.java.net/~phh/8062808/webrev.8u.hotspot.02/
is the same because it doesn't include make/bsd/makefiles/gcc.make.
Paul
On 10/5/20, 5:50 AM, "Volker Simonis" <volker.simonis at gmail.com> wrote:
On Thu, Oct 1, 2020 at 10:21 PM Hohensee, Paul <hohensee at amazon.com> wrote:
>
> Reviving from https://mail.openjdk.java.net/pipermail/jdk8u-dev/2020-February/011088.html.
>
> Xin’s last comment on https://bugs.openjdk.java.net/browse/JDK-8062808 notes that there are two remaining predecessor backports, https://bugs.openjdk.java.net/browse/JDK-8030350 and https://bugs.openjdk.java.net/browse/JDK-8036122, so this review request is for all three of them. They will be pushed together (but as separate changesets).
>
> In order of application:
>
> JBS issue: https://bugs.openjdk.java.net/browse/JDK-8030350
> Original root patch: http://hg.openjdk.java.net/jdk9/jdk9/rev/3a5d5a168722
> Original hotspot patch: http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/b6ab2c9abfc5
> Webrev: http://cr.openjdk.java.net/~phh/8030350/webrev.8u.hotspot.01/
>
> The root patch was already done (in flags.m4 rather than toolchain.m4), so is unneeded. The hotspot patch was clean except that make/bsd/makefiles/gcc.make included -Wunused-value, which I retained.
>
Looks good except that I don't understand why you've retrained
"-Wunused-value" in "make/bsd/makefiles/gcc.make". When looking at the
original patch in jdk9 [1] I can see that it was there as well and
then removed by 8030350:
-WARNING_FLAGS = -Wpointer-arith -Wsign-compare -Wundef
-Wunused-function -Wunused-value
+WARNING_FLAGS = -Wpointer-arith -Wsign-compare -Wundef
-Wunused-function -Wformat=2 -Wno-error=format-nonliteral
[1] http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/b6ab2c9abfc5#l1.7
> JBS issue: https://bugs.openjdk.java.net/browse/JDK-8036122
> Original hotspot patch: http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/d72cee0607a3
> Incremental webrev relative to 8030350: http://cr.openjdk.java.net/~phh/8036122/webrev.8u.hotspot.01/
> With 8030350 webrev: http://cr.openjdk.java.net/~phh/8030350.8036122/webrev.8u.hotspot.01/
>
> This patch was clean except for the above-mentioned make/bsd/makefiles/gcc.make issue, and a context diff for the first hunk for os_linux.cpp.
>
If you would have removed "-Wunused-value" from
"make/bsd/makefiles/gcc.make" in the previous change, this one would
apply cleanly as well.
Otherwise looks good.
> JBS issue: https://bugs.openjdk.java.net/browse/JDK-8062808
> Original hotspot patch: http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/ef7449e07592
> Incremental webrev relative to 8030350+8036122: http://cr.openjdk.java.net/~phh/8062808/webrev.8u.hotspot.02/<http://cr.openjdk.java.net/~phh/8062808/webrev.8u.hotspot.01/>
> With 8030350+8036122 webrev: http://cr.openjdk.java.net/~phh/8030350.8036122.8062808/webrev.8u.hotspot.02/
>
> This patch was clean except for the above-mentioned make/bsd/makefiles/gcc.make issue, the addition of perfData.hpp as mentioned by Xin in https://mail.openjdk.java.net/pipermail/jdk8u-dev/2020-February/011254.html, and the addition of the three missing hunks plus a copyright hunk to jvm.cpp (also in Xin’s patch).
>
This one looks good.
Just for the record:
- I reviewed version
http://cr.openjdk.java.net/~phh/8062808/webrev.8u.hotspot.02.
- there's also a missing hunk in constantPool.hpp but that's OK
because 8u doesn't have "klass_at_ignore_error()" and this has also
been mentioned in the cited mail.
Best regards,
Volker
> Compiles with both fastdebug and slowdebug on linux. I’d greatly appreciate if someone could try osx and solaris.
>
> Thanks,
> Paul
>
>
>
>
>
More information about the jdk8u-dev
mailing list