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