Re: [PATCH v3 1/5] build: make cc-option properly deal with unrecognized sub-options

[Jan Beulich <jbeulich@xxxxxxxx>]

On 11.08.2023 15:48, Anthony PERARD wrote:
> On Wed, Jul 26, 2023 at 12:33:07PM +0200, Jan Beulich wrote:
>> In options like -march=, it may be only the sub-option which is
>> unrecognized by the compiler. In such an event the error message often
>> splits option and argument, typically saying something like "bad value
>> '<argument>' for '<option>'. Extend the grep invocation accordingly,
>> also accounting for Clang to not mention e.g. -march at all when an
>> incorrect argument was given for it.
>>
>> To keep things halfway readable, re-wrap and re-indent the entire
>> construct.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> ---
>> In principle -e "$$pat" could now be omitted from the grep invocation,
>> since if that matches, both $$opt and $$arg will, too. But I thought I'd
>> leave it for completeness.
>> ---
>> v3: Fix build with make 4.3 and newer, where the treatment of \# has
>>     changed.
>> v2: Further relax grep patterns for clang, which doesn't mention -march
>>     when complaining about an invalid argument to it.
>>
>> --- a/Config.mk
>> +++ b/Config.mk
>> @@ -8,6 +8,7 @@ endif
>>  comma   := ,
>>  open    := (
>>  close   := )
>> +sharp   := \#
>>  squote  := '
>>  #' Balancing squote, to help syntax highlighting
>>  empty   :=
>> @@ -90,9 +91,14 @@ PYTHON_PREFIX_ARG ?= --prefix="$(prefix)
>>  # of which would indicate an "unrecognized command-line option" 
>> warning/error.
>>  #
>>  # Usage: cflags-y += $(call cc-option,$(CC),-march=winchip-c6,-march=i586)
>> -cc-option = $(shell if test -z "`echo 'void*p=1;' | \
>> -              $(1) $(2) -c -o /dev/null -x c - 2>&1 | grep -- 
>> $(2:-Wa$(comma)%=%) -`"; \
>> -              then echo "$(2)"; else echo "$(3)"; fi ;)
>> +cc-option = $(shell pat='$(2:-Wa$(comma)%=%)'; \
>> +                    opt="$${pat%%=*}" arg="$${pat$(sharp)*=}"; \
>> +                    if test -z "`echo 'void*p=1;' | \
>> +                                 $(1) $(2) -c -o /dev/null -x c - 2>&1 | \
>> +                                 grep -e "$$pat" -e "$$opt" -e "$$arg" -`"; 
>> \
>> +                    then echo "$(2)"; \
>> +                    else echo "$(3)"; \
>> +                    fi;)
> 
> This patch looks fine. Shouldn't the comment been updated as well? At
> the moment, it only discuss about -Wno-*, which it seems is why `grep`
> was introduced in the first place.
> 
> But isn't it doing doing pattern matching on an error message going to
> lead sometime to false positive? Linux's build system seems to works
> fine by just using the exit value. They've got a few trick to deal with
> -Wno-* and with clang.
> 
> For -Wno-$(warning), they test -W$(warning) instead. For clang, they've
> enable additional warnings:
>     -Werror=unknown-warning-option
>     -Werror=ignored-optimization-argument
>     -Werror=option-ignored
>     -Werror=unused-command-line-argument

I think using just -Werror is going to be enough. The completely changed
patch below appears to be doing fine, but of course requires me to drop
...

> In any case, the patch is fine.
> Reviewed-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>

... this.

Jan

--- a/Config.mk
+++ b/Config.mk
@@ -81,17 +81,17 @@ PYTHON_PREFIX_ARG ?= --prefix="$(prefix)
 
 # cc-option: Check if compiler supports first option, else fall back to second.
 #
-# This is complicated by the fact that unrecognised -Wno-* options:
+# This is complicated by the fact that with most gcc versions unrecognised
+# -Wno-* options:
 #   (a) are ignored unless the compilation emits a warning; and
 #   (b) even then produce a warning rather than an error
-# To handle this we do a test compile, passing the option-under-test, on a code
-# fragment that will always produce a warning (integer assigned to pointer).
-# We then grep for the option-under-test in the compiler's output, the presence
-# of which would indicate an "unrecognized command-line option" warning/error.
+# Further Clang also only warns for unrecognised -W* options.  To handle this
+# we do a test compile, substituting -Wno-* by -W* and adding -Werror.  This
+# way all unrecognised options are diagnosed uniformly, allowing us to merely
+# check exit status.
 #
 # Usage: cflags-y += $(call cc-option,$(CC),-march=winchip-c6,-march=i586)
-cc-option = $(shell if test -z "`echo 'void*p=1;' | \
-              $(1) $(2) -c -o /dev/null -x c - 2>&1 | grep -- 
$(2:-Wa$(comma)%=%) -`"; \
+cc-option = $(shell if $(1) $(2:-Wno-%=-W%) -Werror -c -o /dev/null -x c 
/dev/null >/dev/null 2>&1; \
               then echo "$(2)"; else echo "$(3)"; fi ;)
 
 # cc-option-add: Add an option to compilation flags, but only if supported.