On Thu, 23 Jan 2025 13:48:06 GMT, Joachim Kern <jk...@openjdk.org> wrote:

>> We (SAP) try to introduce the ‘IBM Open XL C/C++ for AIX 17.1.2’ (based on 
>> clang 17) as a feasible compiler for jdk25, because in combination with the 
>> 17.1.3 runtime this would enable the support for ubsan.
>> Unfortunately, the new compiler comes along with a new set of compiler 
>> warnings turned into errors by -Werror.
>> One of the warnings is -Wtentative-definitions. It is fired when a variable 
>> definition in a header is found:
>> /jdk/src/java.desktop/share/native/libawt/awt/image/imageInitIDs.h:36:20: 
>> error: possible missing 'extern' on global variable definition in header 
>> [-Werror,-Wtentative-definitions]
>>    36 | IMGEXTERN jfieldID g_BImgRasterID;
>>       | ^
>> From now on headers allow only extern declarations of variables. The 
>> definition must take place in a c/cpp file. This means e.g.
>> imageInitIDs.h:36:20
>>    36 extern jfieldID g_BImgRasterID;
>> and the corresponding c-File
>>    jfieldID g_BImgRasterID;
>> 
>> The other possible solution would be to compile everything with
>> -Wno-tentative-definitions
>> which could be set in flags-cflags.m4, if compiling with clang 17.
>
> Joachim Kern has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   remove old solution

> I think `-Wtentative-definition` doesn't really buy all that much. [...] But 
> I don't think I'd want to globally disable it, as it seems like it would 
> usually be a valid complaint

I think we all agree on that. Apparently clang 17 enables it by default, at 
least on AIX. So then we have the choice to either disable it or accept to have 
it on, and deal with what it complains about. 

These were the only problematic places in the entire code base. I think it was 
the right decision to change the code in these places. The code here was, if 
not "smelly" so at least not conforming to the practice used everywhere else in 
the code base. I personally think your workaround with disabling the warning 
seems like a worse solution compared to what was actually integrated.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/23236#issuecomment-2642875242

Reply via email to