On Mon, 14 Nov 2022 01:17:38 GMT, Julian Waters <jwat...@openjdk.org> wrote:
>> src/hotspot/os/windows/os_windows.hpp line 35: >> >>> 33: class Thread; >>> 34: >>> 35: static unsigned __stdcall thread_native_entry(Thread*); >> >> Why was this removed? This is needed to correctly specify the call sequence >> for the thread entry routine when used with `_beginThreadex`: >> https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/beginthread-beginthreadex?view=msvc-170 > > I'm not sure I follow, I didn't remove anything here? Sorry my eyes must be playing tricks on me. ?? Why did you need to add this here? >> src/hotspot/share/utilities/compilerWarnings.hpp line 47: >> >>> 45: #endif >>> 46: >>> 47: #ifndef PRAGMA_DISABLE_VISCPP_WARNING >> >> Why rename this from `MSVC` to `VISCPP`? IIRC the full name is Microsft >> Visual Studio C++, so you new name is not obviously better and the change >> just adds noise to the PR. Further `MSVC` matches what MS themselves use and >> even the attribute namespace in C++11 is `MSVC`. >> Update: I see the inconsistency with `compilerWarnings_visCPP.hpp` > > Yep, it was renamed since the file is also named VISCPP, and I felt that > matching the names was a good style change I think it is the file that has the "bad" name in this case. :( But okay. >> src/hotspot/share/utilities/compilerWarnings_gcc.hpp line 37: >> >>> 35: #endif >>> 36: >>> 37: #if defined(__clang_major__) && \ >> >> Not clear why this was moved ?? > > I'm not sure which one you're referring to, but the PRAGMA_DIAG_PUSH/POP was > moved up to the top of the header to match compilerWarnings_visCPP.hpp, and > PRAGMA_DISABLE_GCC_WARNING_AUX was moved to macros.hpp as the more general > PRAGMA macro, since it's useful for all compilers and not just gcc Okay ------------- PR: https://git.openjdk.org/jdk/pull/11081
