On Thu, 14 Mar 2024 18:01:34 GMT, Erik Joelsson <er...@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
