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

[Gilles Duboscq]

This only affects clang, and yes, we probably use a newer version. For
example, this warning already triggers for me in clang 3.5.

If this change is a problem, we can file a separate bug since it's not
really related to the jvmci changes.

 Gilles

On 17/09/15 18:01, Volker Simonis wrote:
> 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
>>>