RFR: 8331020: Assign LANG to C++ for Windows code that uses C++

Magnus Ihse Bursie ihse at openjdk.org
Tue Apr 30 14:15:05 UTC 2024


On Tue, 30 Apr 2024 13:50:22 GMT, Julian Waters <jwaters at openjdk.org> wrote:

>> make/modules/java.desktop/lib/AwtLibraries.gmk line 102:
>> 
>>> 100: $(eval $(call SetupJdkLibrary, BUILD_LIBAWT, \
>>> 101:     NAME := awt, \
>>> 102:     LANG := $(if $(filter $(OPENJDK_TARGET_OS), windows), C++, C), \
>> 
>> No, this is not okay. You need to do this as for the LIBJSOUND_LINK_TYPE above. The same goes for the one below, too.
>
> Oh, I'm surprised! I thought that you'd prefer the more lambda-like approach. I guess the other way of LIBAWT_LINK_TYPE works too in that case

There are two reasons:

1) To keep up with the style elsewhere, were we use "ifeq (... platform)" to setup platform-specific arguments. Even if this was not an ideal style, it would still make sense to keep to one way of doing it.

2) I actually think that is better. We have gravitated towards that solution over the years. The make syntax is hard to read and easy to get wrong. We try to make the arguments in the Setup calls trivial, and if we can't do that in place, then we create a "local" variable (by prefixing it with the name of the lib) outside the Setup call, were we can use more space to clearly show what is going on.

In fact, I really dislike the `$(if...)` syntax, and use it only if I must. It is hopeless to see what is the if-clause and the else-clause, and it is way too easy to get a "false positive" since you do not compare the variable with another value, but checks if it evaluates to non-empty.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/18927#discussion_r1584903238


More information about the build-dev mailing list