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

[Volker Simonis]

If this comes only from adlc, you could fix for adlc only by adding

WARNINGS_ARE_ERRORS += -Wno-undefined-bool-conversion

to make/linux/makefiles/adlc.make instead of make/linux/makefiles/gcc.make

I do however not understand why this could happen during jvmci
development if you didn't touch any adlc file. Maybe you used another
compiler (newer/older than 4.8.2) during jvmci development ?

Regards,
Volker

On Thu, Sep 17, 2015 at 3:57 PM, Gilles Duboscq
<gilles.m.duboscq at oracle.com> wrote:
> 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
>>