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 Coming late to the party, since this PR has already been integrated. First, tentative definitions are only applicable to C, not to C++. I think `-Wtentative-definition` doesn't really buy all that much. It looks like it turns a *potential* link-time failure into a definite compile-time warning. But I don't think I'd want to globally disable it, as it seems like it would usually be a valid complaint (unless the code is jumping through some hoops as done by imageInitIDs.[ch]), and probably provides a better error message too. gcc provides `-fcommon`, which controls the placement of tentative definitions. We're not using that, instead implicitly using the default `-fno-common`. (There is one place where we unnecessarily use `-fno-common` explicitly: https://bugs.openjdk.org/browse/JDK-8349565.) With `-fcommon`, tentative definitions in different translation units are merged, with potential access penalties. With `-fno-common`, instead of merging one gets multiple definition errors at link time. Note that code that relies on `-fcommon` has never been standards compliant. The changes related to fp_pipewire.h seem fine. I'm not keen on the large copy-paste-modify for imageInitIDs.[ch]. I kind of prefer a different approach, of locally suppressing the warning, i.e. something like (untested, may need version conditionalization): In imageInitIDs.c + PRAGMA_DIAG_PUSH + #pragma clang diagnostic ignored "-Wtentative-definition" #define IMGEXTERN #include "imageInitIDs.h" + PRAGMA_DIAG_POP I'll leave it up to the client team to decide whether they want to make such a change, or leave things as-is. ------------- PR Comment: https://git.openjdk.org/jdk/pull/23236#issuecomment-2640539884
