Re: RFR: JDK-8211071: unpack.cpp fails to compile with statement has no effect [-Werror=unused-value]
25 sep. 2018 kl. 10:21 skrev Roman Kennke <rkennke@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
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@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
Hi Roman, this looks good to me. +1 Best regards Christoph
-----Original Message----- From: build-dev <build-dev-bounces@openjdk.java.net> On Behalf Of Roman Kennke Sent: Mittwoch, 26. September 2018 19:24 To: Magnus Ihse Bursie <magnus.ihse.bursie@oracle.com>; core-libs- dev@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 <rkennke@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
Hi Christoph & Magnus, thanks for reviewing! Am 27.09.18 um 08:22 schrieb Langer, Christoph:
Hi Roman,
this looks good to me. +1
Best regards Christoph
-----Original Message----- From: build-dev <build-dev-bounces@openjdk.java.net> On Behalf Of Roman Kennke Sent: Mittwoch, 26. September 2018 19:24 To: Magnus Ihse Bursie <magnus.ihse.bursie@oracle.com>; core-libs- dev@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 <rkennke@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
participants (3)
-
Langer, Christoph
-
Magnus Ihse Bursie
-
Roman Kennke