On Thu, 28 Mar 2024 18:27:39 GMT, Erik Joelsson <er...@openjdk.org> wrote:
>> Currently a lot of code is duplicated between SetupJdkExecutable and >> SetupJdkLibrary. Furthermore, some functionality is still missing from >> SetupJdkExecutable that is present in SetupJdkLibrary. These functions also >> have not had their documentation properly updated as they have evolved. This >> PR will fix all of this. > > make/common/JdkNativeCompilation.gmk line 233: > >> 231: # Set the default flags first to be able to override >> 232: $1_CXXFLAGS := $$(filter-out $$($1_CXXFLAGS_FILTER_OUT), >> $$(CXXFLAGS_JDKLIB)) $$($1_CXXFLAGS) >> 233: endif > > I think it makes sense to share all that is actually common between the two > existing macros, but for these conditional adding default flags, it's just a > big if EXECUTABLE do this, otherwise do that. I think in such cases it makes > more sense to keep that logic in the respective specialized macros. The only > drawback would be that the new `SetupJdkNativeCompilation` won't be usable on > its own, but it's not intended to be anyway. Actually, I'm intending on calling SetupJdkNativeCompilation directly in one place, from SetupTestFilesCompilation, which already mixes executable and libraries at it's entry point. So I'd rather keep SetupJdkNativeCompilation working. Also, I plan to follow this up with a PR that splits up the C[XX]FLAGS_JDK[EXE|LIB] into constituent parts and then just select the few that differs between C and C++, LIB and EXE depending on which is chosen. I should perhaps have been clearer about this. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/18537#discussion_r1543696187
