Re: [PATCH 3/3] config.mak.dev: simplify compiler check for multiple compilers

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Sep 28, 2021, 12:07 UTC
Message-ID: <87k0j0x5mg.fsf@evledraar.gmail.com>
In-Reply-To: <20210928091054.78895-4-carenas@gmail.com>

On Tue, Sep 28 2021, Carlo Marcelo Arenas Belón wrote:

Show 25 quoted lines

> 1da1580e4c (Makefile: detect compiler and enable more warnings in
> DEVELOPER=1, 2018-04-14), includes an $(or) of two different filters
> to check for both gcc and clang versions.
>
> As shown in a previous patch, a simpler syntax is available so apply
> the same logic here also for consistency.
>
> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> ---
>  config.mak.dev | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/config.mak.dev b/config.mak.dev
> index 90c47d2782..b66fae8665 100644
> --- a/config.mak.dev
> +++ b/config.mak.dev
> @@ -31,7 +31,7 @@ ifneq ($(filter clang4,$(COMPILER_FEATURES)),)
>  DEVELOPER_CFLAGS += -Wtautological-constant-out-of-range-compare
>  endif
>  
> -ifneq ($(or $(filter gcc6,$(COMPILER_FEATURES)),$(filter clang4,$(COMPILER_FEATURES))),)
> +ifneq ($(filter clang4 gcc6,$(COMPILER_FEATURES)),)
>  DEVELOPER_CFLAGS += -Wextra
>  # if a function is public, there should be a prototype and the right
>  # header file should be included. If not, it should be static.

This looks like a good cleanup and ends up being much more readable.

I wonder if eventually a larger change to simplify this like perhaps the below wouldn't be nicer. I didn't test it carefully & may have gotten the logic wrong, which I think somewhat makes the point that reading this ifeq/ifneq logic (especially the nested bit at the end) is a bit hard, at least for me:)

Anyway, feel free to ignore the below, and I think it's certainly not needed for this series, just my 0.02 if you're eventually refactoring some of this.

diff --git a/config.mak.dev b/config.mak.dev
index 0a87d8cbe9d..df27340b4b0 100644
--- a/config.mak.dev
+++ b/config.mak.dev
@@ -2,6 +2,14 @@ ifndef COMPILER_FEATURES
 COMPILER_FEATURES := $(shell ./detect-compiler $(CC))
 endif
 
+# These are all the empty string if the compiler *isn't* X or
+# earlier. Note that clang v5, v6 etc. also qualify as "have v4".
+CC_HAVE_CLANG4 = $(filter clang4,$(COMPILER_FEATURES))
+CC_HAVE_GCC4 = $(filter gcc4,$(COMPILER_FEATURES))
+CC_HAVE_GCC5 = $(filter gcc5,$(COMPILER_FEATURES))
+CC_HAVE_GCC6 = $(filter gcc6,$(COMPILER_FEATURES))
+CC_HAVE_GCC10 = $(filter gcc10,$(COMPILER_FEATURES))
+
 ifeq ($(filter no-error,$(DEVOPTS)),)
 DEVELOPER_CFLAGS += -Werror
 SPARSE_FLAGS += -Wsparse-error
@@ -9,9 +17,9 @@ endif
 DEVELOPER_CFLAGS += -Wall
 ifeq ($(filter no-pedantic,$(DEVOPTS)),)
 DEVELOPER_CFLAGS += -pedantic
-ifneq ($(filter clang4 gcc5,$(COMPILER_FEATURES)),)
+ifneq ($(CC_HAVE_CLANG4)$(CC_HAVE_GCC5),)
 DEVELOPER_CFLAGS += -Wpedantic
-ifneq ($(filter gcc10,$(COMPILER_FEATURES)),)
+ifneq ($(CC_HAVE_GCC10),)
 ifeq ($(uname_S),MINGW)
 DEVELOPER_CFLAGS += -Wno-pedantic-ms-format
 DEVELOPER_CFLAGS += -Wno-incompatible-pointer-types
@@ -29,11 +37,11 @@ DEVELOPER_CFLAGS += -Wunused
 DEVELOPER_CFLAGS += -Wvla
 DEVELOPER_CFLAGS += -fno-common
 
-ifneq ($(filter clang4,$(COMPILER_FEATURES)),)
+ifneq ($(CC_HAVE_CLANG4),)
 DEVELOPER_CFLAGS += -Wtautological-constant-out-of-range-compare
 endif
 
-ifneq ($(filter clang4 gcc6,$(COMPILER_FEATURES)),)
+ifneq ($(CC_HAVE_CLANG4)$(CC_HAVE_GCC6),)
 DEVELOPER_CFLAGS += -Wextra
 # if a function is public, there should be a prototype and the right
 # header file should be included. If not, it should be static.
@@ -49,8 +57,8 @@ endif
 
 # uninitialized warnings on gcc 4.9.2 in xdiff/xdiffi.c and config.c
 # not worth fixing since newer compilers correctly stop complaining
-ifneq ($(filter gcc4,$(COMPILER_FEATURES)),)
-ifeq ($(filter gcc5,$(COMPILER_FEATURES)),)
+ifneq ($(CC_HAVE_GCC4),)
+ifeq ($(CC_HAVE_GCC5),)
 DEVELOPER_CFLAGS += -Wno-uninitialized
 endif
 endif