RFR: 8328157: Move C[XX]FLAGS_JDK[LIB/EXE] to JdkNativeCompilation.gmk [v3]
Magnus Ihse Bursie
ihse at openjdk.org
Fri Mar 15 12:12:40 UTC 2024
On Thu, 14 Mar 2024 18:01:34 GMT, Erik Joelsson <erikj at openjdk.org> wrote:
>> Magnus Ihse Bursie has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Fix syntax error
>
> make/common/JdkNativeCompilation.gmk line 206:
>
>> 204: $$(foreach flag, $$($1_CXXFLAGS_FILTER_OUT), \
>> 205: $$(eval $1_CXXFLAGS := $(filter-out $$(flag), $$($1_CXXFLAGS)) \
>> 206: ))
>
> There shouldn't be a need for a loop here.
> Suggestion:
>
> $$(filter-out $$($1_CXXFLAGS_FILTER_OUT), $$($1_CXXFLAGS))
>
> Avoiding eval calls is usually a good idea as `$` escaping gets really convoluted when nesting them. Also, the single `$` in the filter-out call can't possibly work?
>
> The conditional is also not necessary as filter-out with an empty pattern is a noop, so all of this could be condensed to a single line, including the assignment.
>
> Same thing applies to a couple of more places below.
Ah, I did not realize filter-out took a list of words to remove. So `$(filter-out foo bar, bar foo)` would return the empty string, and I thought it would return `bar foo`. That indeed simplifies things, thank you!
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18301#discussion_r1526196585
More information about the build-dev
mailing list