Re: [PATCH] revision: use C99 declaration of variable in for() loop

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 18, 2021, 07:09 UTC
Message-ID: <xmqq1r3eym7f.fsf@gitster.g>
In-Reply-To: <61518213-9ce8-00d2-efd9-7f2091c574c4@gmail.com>

Phillip Wood <phillip.wood123@gmail.com> writes:

> I like the idea of using a specific test balloon for the features that
> we want to use but wont this one break the build for anyone doing
> 'make DEVELOPER=1' because -Wdeclaration-after-statement will error
> out.

I think you are missing '?' at the end of the sentence, but the answer is "no, at least not for me".

    # pardon my "make" wrapper; it is to pass DEVELOPER=1 etc. to
    # the underlying "make" command.
    $ Meta/Make V=1 revision.o
    cc -o revision.o -c -MF ./.depend/revision.o.d -MQ revision.o -MMD -MP  -Werror -Wall -pedantic -Wpedantic -Wdeclaration-after-statement -Wformat-security -Wold-style-definition -Woverflow -Wpointer-arith -Wstrict-prototypes -Wunused -Wvla -fno-common -Wextra -Wmissing-prototypes -Wno-empty-body -Wno-missing-field-initializers -Wno-sign-compare -Wno-unused-parameter  -g -O2 -Wall -I. -DHAVE_SYSINFO -DGIT_HOST_CPU="\"x86_64\"" -DUSE_LIBPCRE2 -DHAVE_ALLOCA_H  -DUSE_CURL_FOR_IMAP_SEND -DSUPPORTS_SIMPLE_IPC -DSHA1_DC -DSHA1DC_NO_STANDARD_INCLUDES -DSHA1DC_INIT_SAFE_HASH_DEFAULT=0 -DSHA1DC_CUSTOM_INCLUDE_SHA1_C="\"cache.h\"" -DSHA1DC_CUSTOM_INCLUDE_UBC_CHECK_C="\"git-compat-util.h\"" -DSHA256_BLK  -DHAVE_PATHS_H -DHAVE_DEV_TTY -DHAVE_CLOCK_GETTIME -DHAVE_CLOCK_MONOTONIC -DHAVE_SYNC_FILE_RANGE -DHAVE_GETDELIM '-DPROCFS_EXECUTABLE_PATH="/proc/self/exe"' -DFREAD_READS_DIRECTORIES -DNO_STRLCPY -DSHELL_PATH='"/bin/sh"' -DPAGER_ENV='"LESS=FRX LV=-c"'  revision.c
    $ cc --version
    cc (Debian 10.3.0-11) 10.3.0
    Copyright (C) 2020 Free Software Foundation, Inc.
    This is free software; see the source for copying conditions.  There is NO
    warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

It would be quite sad if we had to allow decl-after-stmt, only to allow

	stmt;
	for (type var = init; ...; ...) {
		...;
	}

because it should merely be a short-hand for

	stmt;
	{
	    type var;
	    for (var = init; ...; ...) {
		...;
	    }
	}

that does not need to allow decl-after-stmt.

Different compilers may behave differently, so it might be an issue for somebody else, but I am hoping any reasonable compiler would behave sensibly.

Thanks for raising a potential issue, as others can try it out in their environment and see if their compilers behave well.