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