From: Junio C Hamano <gitster@pobox.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: "Carlo Marcelo Arenas Belón" <carenas@gmail.com>,
git@vger.kernel.org, t.gummerer@gmail.com, pclouds@gmail.com,
jonathantanmy@google.com
Subject: Re: [PATCH 3/3] config.mak.dev: simplify compiler check for multiple compilers
Date: Tue, 28 Sep 2021 14:30:02 -0700 [thread overview]
Message-ID: <xmqq4ka4jsmd.fsf@gitster.g> (raw)
In-Reply-To: <87k0j0x5mg.fsf@evledraar.gmail.com> ("Ævar Arnfjörð Bjarmason"'s message of "Tue, 28 Sep 2021 14:07:09 +0200")
Ævar Arnfjörð Bjarmason <avarab@gmail.com> 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),)
prev parent reply other threads:[~2021-09-28 21:30 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-28 9:10 [PATCH 0/3] Makefile: tighten pedantic flag Carlo Marcelo Arenas Belón
2021-09-28 9:10 ` [PATCH 1/3] Makefile: restrict -Wpedantic and -Wno-pedantic-ms-format better Carlo Marcelo Arenas Belón
2021-09-28 11:46 ` Ævar Arnfjörð Bjarmason
2021-09-28 23:39 ` Junio C Hamano
2021-09-28 9:10 ` [PATCH 2/3] Makefile: avoid multiple -Wall in CFLAGS Carlo Marcelo Arenas Belón
2021-09-28 9:19 ` Ævar Arnfjörð Bjarmason
2021-09-28 11:03 ` Carlo Arenas
2021-09-28 21:19 ` Junio C Hamano
2021-09-28 23:22 ` Carlo Arenas
2021-09-29 4:08 ` Junio C Hamano
2021-09-28 9:10 ` [PATCH 3/3] config.mak.dev: simplify compiler check for multiple compilers Carlo Marcelo Arenas Belón
2021-09-28 12:07 ` Ævar Arnfjörð Bjarmason
2021-09-28 21:30 ` Junio C Hamano [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqq4ka4jsmd.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=avarab@gmail.com \
--cc=carenas@gmail.com \
--cc=git@vger.kernel.org \
--cc=jonathantanmy@google.com \
--cc=pclouds@gmail.com \
--cc=t.gummerer@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.