RFR (XXL): JEP 243: Java-Level JVM Compiler Interface

[Gilles Duboscq]

This warning (undefined-bool-conversion) comes from existing code, not
from new code contained in this patch.
The warning appears is in adlc:
/src/share/vm/adlc/filebuff.cpp:109:8: error: 'this' pointer cannot be
null in well-defined C++ code; pointer may be assumed to always convert
to true [-Werror,-Wundefined-bool-conversion]
We added -Wundefined-bool-conversion during jvmci development because it
was making our builds fail but it's not related to jvmci at all.

Gilles

On 17/09/15 09:24, Magnus Ihse Bursie wrote:
> On 2015-09-16 22:25, Christian Thalinger wrote:
>>
>>> On Sep 16, 2015, at 10:11 AM, Magnus Ihse Bursie
>>> <magnus.ihse.bursie at oracle.com
>>> <mailto:magnus.ihse.bursie at oracle.com>> wrote:
>>>
>>> On 2015-09-16 18:52, Christian Thalinger wrote:
>>>>> On Sep 16, 2015, at 2:57 AM, Magnus Ihse Bursie
>>>>> <magnus.ihse.bursie at oracle.com
>>>>> <mailto:magnus.ihse.bursie at oracle.com>> wrote:
>>>>>
>>>>> On 2015-09-14 09:24, Christian Thalinger wrote:
>>>>>> The JEP itself can be found here:
>>>>>>
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8062493
>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8062493>
>>>>>>
>>>>>> Here are the webrevs:
>>>>>>
>>>>>> http://cr.openjdk.java.net/~kvn/JVMCI/webrev.top/
>>>>>> <http://cr.openjdk.java.net/%7Ekvn/JVMCI/webrev.top/>
>>>>>> <http://cr.openjdk.java.net/~kvn/JVMCI/webrev.top/
>>>>>> <http://cr.openjdk.java.net/%7Ekvn/JVMCI/webrev.top/>>
>>>>>> http://cr.openjdk.java.net/~kvn/JVMCI/webrev.hotspot/
>>>>>> <http://cr.openjdk.java.net/%7Ekvn/JVMCI/webrev.hotspot/>
>>>>>> <http://cr.openjdk.java.net/~kvn/JVMCI/webrev.hotspot/
>>>>>> <http://cr.openjdk.java.net/%7Ekvn/JVMCI/webrev.hotspot/>>
>>>>>>
>>>>>> The change has already undergone a few iterations of internal
>>>>>> review by different people of different teams.  Most comments and
>>>>>> suggestions were handled accordingly. Although there is one open
>>>>>> item we agreed we will address after the integration of JEP 243
>>>>>> and that work is captured in RFE:
>>>>>>
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8134994
>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8134994>
>>>>>>
>>>>>> SQE is still working on the tests and all test tasks can be seen
>>>>>> as sub-tasks of the JEP.
>>>>>>
>>>>>> The integration will happen under the bug number:
>>>>>>
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8136421
>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8136421>
>>>>>>
>>>>> Hi Christian,
>>>>>
>>>>> (Adding build-dev since this review includes some major build
>>>>> changes.)
>>>>>
>>>>> The makefile changes looks good in general to me. I have not
>>>>> reviewed the source code changes.
>>>>>
>>>>> Some comments:
>>>>>
>>>>> * modules.xml:
>>>>> Make sure you get sign-off from a Jigsaw developer for modifying
>>>>> this file.
>>>> I’ve asked Alan to take a look.
>>>>
>>>>> * hotspot/make/linux/makefiles/gcc.make:
>>>>> Seems unfortunate to have to disable a new warning
>>>>> (undefined-bool-conversion) for newly incorporated code. Is it not
>>>>> possible to fix the code emitting this warning instead?
>>>> We had this question before.  It’s not obvious because you can’t see
>>>> it in the regular diff views but this is under:
>>>>
>>>> ifeq ($(USE_CLANG), true)
>>>
>>> I'm not sure I understand why that's relevant..? Isn't it just as
>>> important to try to submit warning-free code when compiling with
>>> clang as with any other compiler? Or is clang just being anal about
>>> the code in question?
>>
>> I don’t have a Clang compiler at hand but Clang is anal about
>> everything.  Do you want that suppression to be removed?
> 
> It's more a hotspot code quality issue, not a build system issue, so I
> won't insist. I just wanted to point out that this change will start
> hiding a new kind of warning for all files in hotspot. Unless there was
> a compelling reason, I would personally rather see an effort to fix the
> code in question. But if no-one from Hotspot agrees on this, I'll drop it.
> 
> /Magnus
>