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

[Hohensee, Paul]

Thanks, Volker. Tagged.

On 10/5/20, 10:31 AM, "Volker Simonis" <volker.simonis at gmail.com> wrote:

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