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

From: Junio C Hamano <[email-masked]>
Date: Tue, 28 Sep 2021 14:30:02 -0700

Ævar Arnfjörð Bjarmason <[email-masked]> writes:

> 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".

I had to read this three times and still cannot decide what it wants
ot say.

> +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))

It is empty if the compiler isn't X.  It is also empty if the
compiler isn't earlier than X?  That would mean version (X+1) would
qualify, as it is not X, which is not what we want.

I think you meant "... empty string, unless the compiler is X or
later."

Also, I often see "Note that" to imply "despite what I just said",
but as far as I can tell CLANG4 is not all that special and follows
the same general rule.  Perhaps "Note that" -> "For example," is
needed to clarify.

Having said all that, I find that the original 

	ifneq ($(filter x y, $(COMPILER_FEATURES)),)

idiom is readable enough, and

	ifneq ($(CC_HAVE_X)$(CC_HAVE_Y),)

does not necessarily make it easier to spot X and Y that are being
checked with the conditional.

>  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),)