Re: [XEN PATCH v7 01/51] build: introduce cpp_flags macro

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.09.2021 12:06, Anthony PERARD wrote:
> On Thu, Sep 02, 2021 at 12:08:58PM +0200, Jan Beulich wrote:
>> On 24.08.2021 12:49, Anthony PERARD wrote:
>>> Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
>>
>> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
>> albeit with a remark:
>>
>>> --- a/xen/Rules.mk
>>> +++ b/xen/Rules.mk
>>> @@ -133,6 +133,9 @@ endif
>>>  # Always build obj-bin files as binary even if they come from C source. 
>>>  $(obj-bin-y): XEN_CFLAGS := $(filter-out -flto,$(XEN_CFLAGS))
>>>  
>>> +# To be use with $(a_flags) or $(c_flags) to produce CPP flags
>>> +cpp_flags = $(filter-out -Wa$(comma)% -flto,$(1))
>>
>> Afaics this has nothing to do with Linux'es cpp_flags, so what we do here
>> is entirely up to us. If this is strictly intended to be used the another
>> macro, wouldn't it make sense to have
>>
>> cpp_flags = $(filter-out -Wa$(comma)% -flto,$($(1)))
>>
>> here and then e.g. ...
>>
>>> @@ -222,13 +225,13 @@ $(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): 
>>> %.init.o: %.o FORCE
>>>     $(call if_changed,obj_init_o)
>>>  
>>>  quiet_cmd_cpp_i_c = CPP     $@
>>> -cmd_cpp_i_c = $(CPP) $(filter-out -Wa$(comma)%,$(c_flags)) -MQ $@ -o $@ $<
>>> +cmd_cpp_i_c = $(CPP) $(call cpp_flags,$(c_flags)) -MQ $@ -o $@ $<
>>
>> ... the slightly simpler / easier to read
>>
>> cmd_cpp_i_c = $(CPP) $(call cpp_flags,c_flags) -MQ $@ -o $@ $<
>>
>> here?
> 
> I don't think this is better or simpler. "cpp_flags" don't need to know
> the name of the variable to be useful. I think it is better to know that
> "cpp_flags" act on the value of the variable rather than the variable
> itself, when reading "$(call cpp_flags, $(a_flags))".

Well, yes. This way one could also pass more than just the expansion of
either of these two variables. The thing that made me think of the
alternative is the comment: Would you mind if I inserted "e.g." in there,
to make clear this isn't limited to these two variables?

Jan