RFR: 8348286: [AIX] clang 17 introduces new warning Wtentative-Definitions which produces Build errors [v3]

Fri Feb 7 13:17:17 UTC 2025

On Thu, 23 Jan 2025 13:48:06 GMT, Joachim Kern <jkern at 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