Hi Roman, this looks good to me. +1
Best regards Christoph > -----Original Message----- > From: build-dev <build-dev-boun...@openjdk.java.net> On Behalf Of > Roman Kennke > Sent: Mittwoch, 26. September 2018 19:24 > To: Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com>; core-libs- > d...@openjdk.java.net > Cc: build-dev@openjdk.java.net > Subject: Re: RFR: JDK-8211071: unpack.cpp fails to compile with statement > has no effect [-Werror=unused-value] > > Ping core-libs? > > Roman > > Am 25.09.18 um 11:06 schrieb Magnus Ihse Bursie: > > > >> 25 sep. 2018 kl. 10:21 skrev Roman Kennke <rken...@redhat.com>: > >> > >> Not sure this is the correct list. Please redirect as appropriate. > > > > I believe core-libs is the appropriate place. Cc:d. > > > >> > >> Please review the following proposed change: > >> > >> > >> There are 3 asserts in unpack.cpp which check only constants, and which > >> the compiler rejects as 'has no effect' (in fastdebug builds). This > >> seems to be caused by: > >> > >> https://bugs.openjdk.java.net/browse/JDK-8211029 > >> > >> I propose to fix this by defining a STATIC_ASSERT which can evaluate and > >> reject this at compile time: > >> > >> Webrev: > >> http://cr.openjdk.java.net/~rkennke/JDK-8211071/webrev.00/ > > > > From a build perspective, this looks good. But please let someone from > core-libs review also. > > > >> Bug: > >> https://bugs.openjdk.java.net/browse/JDK-8211071 > >> > >> It might be useful to define the STATIC_ASSERT in a more central place > >> to be used elsewhere too. For example, there is a similar #define in > >> Hotspot's debug.hpp > > > > Unfortunately, we do not have a good place to share definitions like this > across JDK libraries. :-( The need for that has struck me more than once. For > some stuff, we mis-use jni.h. But it would definitely be out of line to add a > STATIC_ASSERT there. > > > > /Magnus > > > >> > >> Thanks, Roman > >> > >
