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