On Sat, 30 Nov 2024 10:54:19 GMT, Kim Barrett <kbarr...@openjdk.org> wrote:
> I only noticed this had been changed back to Draft after I was mostly done > looking at it. But I don't think this should be done this way, esp. since it > didn't seem to work (as in suppressing warnings from LTO) for me. Yeah, I had noticed that it didn't work too, which is why I returned it to draft. It also causes VC to explode when compiling the Windows HotSpot, so that isn't ideal. I guess returning g1ParScanThreadState.cpp to LTO status will have to wait until FORBID_C_FUNCTION is properly fixed up to be LTO proof > src/hotspot/share/utilities/compilerWarnings_gcc.hpp line 89: > >> 87: // forbidden warnings in such builds. >> 88: #define FORBID_C_FUNCTION(signature, alternative) \ >> 89: [[gnu::warning(alternative)]] signature noexcept; > > Why are you making this change at all, let alone under this JBS issue? > > Among other problems, `noexcept` is mostly irrelevant in HotSpot, since we > build with > exceptions disabled. (There are a few places where `noexcept` affects > semantics, like for > `operator new`, but otherwise there is no point.) I was thinking about the extern "C" and I think it might not be needed. A method that was already declared extern "C" in a C library header will keep that linkage when it is declared again, even without extern "C". There's also the issue that this forbidding macro could declare methods that don't actually exist on the current platform, which I think(?) removing extern "C" helps prevent. There's also the strange case that not all platforms have C library methods that are extern "C" (Windows is a notable example), so this helps declaring things with conflicting linkages and causing an error. The noexcept was just to match the declarations in standard library headers, since they are supposed to be noexcept according to the Standard ------------- PR Comment: https://git.openjdk.org/jdk/pull/22464#issuecomment-2509126800 PR Review Comment: https://git.openjdk.org/jdk/pull/22464#discussion_r1874614943
