Re: [PATCH 3/3] config.mak.dev: simplify compiler check for multiple compilers
From: Junio C Hamano <hidden>
Date: 2021-09-28 21:30:12
Ævar Arnfjörð Bjarmason [off-list ref] writes:
quoted hunk
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.
quoted hunk
+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.
quoted hunk
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),)