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

On 18/11/2021 07:09, Junio C Hamano wrote:

Phillip Wood [off-list ref] writes:

quoted

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,

sorry yes I am

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.

Oh it seems I misunderstood exactly what decl-after-stmt does, thinking 
about it your explanation makes sense. I guess that means we have not 
had any warning flags set (because no such flags exist?)  to protect 
against the accidental introduction of the construct that this patch is 
testing compiler support for.

Best Wishes

Phillip