RFR (M): 8062808: Turn on the -Wreturn-type warning

[Volker Simonis]

On Mon, Oct 5, 2020 at 7:07 PM Hohensee, Paul <hohensee at amazon.com> wrote:
>
> 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
>

Thanks a lot Paul.

Looks good to me now.

Best regards,
Volker

> 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
>     >
>     >
>     >
>     >
>     >
>