RFR: JDK-8211071: unpack.cpp fails to compile with statement has no effect [-Werror=unused-value]

[Langer, Christoph]

Hi Roman,

this looks good to me. +1

Best regards
Christoph

> -----Original Message-----
> From: build-dev <build-dev-bounces at openjdk.java.net> On Behalf Of
> Roman Kennke
> Sent: Mittwoch, 26. September 2018 19:24
> To: Magnus Ihse Bursie <magnus.ihse.bursie at oracle.com>; core-libs-
> dev at openjdk.java.net
> Cc: build-dev at 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 <rkennke at 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
> >>
> >