Re: [RFC/PATCH] CodingGuidelines: spell out post-C89 rules

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jul 17, 2019, 00:55 UTC
Message-ID: <20190717005537.GB93801@google.com>
In-Reply-To: <xmqq4l3l520f.fsf_-_@gitster-ct.c.googlers.com>

Junio C Hamano wrote:

Show 5 quoted lines

> Even though we have been sticking to C89, there are a few handy
> features we borrow from more recent C language in our codebase after
> trying them in weather balloons and saw that nobody screamed.
>
> Spell them out.

Thanks for this. It gives a place to advertise future weather balloons, too.

[...]

Show 14 quoted lines

> --- a/Documentation/CodingGuidelines
> +++ b/Documentation/CodingGuidelines
> @@ -195,10 +195,24 @@ For C programs:
>     by e.g. "echo DEVELOPER=1 >>config.mak".
>  
>   - We try to support a wide range of C compilers to compile Git with,
> -   including old ones. That means that you should not use C99
> -   initializers, even if a lot of compilers grok it.
> +   including old ones. That means that you should not use certain C99
> +   features, even if your compiler groks it.  There are a few
> +   exceptions:
> +
> +   . since early 2012 with e1327023ea, we have been using an enum
> +     definition whose last element is followed by a comma.

This is an interesting one: it's super convenient, but we have received patches every 10 years or so to remove the trailing comma --- e.g. https://public-inbox.org/git/20100311163235.GC7877@thor.il.thewrittenword.com/

I *think* these were motivated by wanting to be able to build Git with old compilers with pedantic warnings on, and certainly the last seven years of silence on the subject suggests it's okay. Should we be even more prescriptive and say that the last element should always be followed by a comma, for ease of later patching?

> +
> +   . since mid 2017 with cbc0f81d and 512f41cf, we have been using
> +     designated initializers for struct and array.

Can this include an example for the benefit of readers that don't know what a designated initializer is? E.g.

      . since mid 2017 with cb0f81d and 512f41cf, we have been using
        designated initializers for struct members ("{ .alloc = 1 }")
	and array members ("[5] = 0").

> +
> +   These used to be forbidden, but we have not heard breakage report,
> +   so they are assumed to be safe.

nit: missing article "any" before "breakage reports".

Show 7 quoted lines

>  
> - - Variables have to be declared at the beginning of the block.
> + - Variables have to be declared at the beginning of the block, before
> +   the first statement (i.e. -Wdeclaration-after-statement).
> +
> + - Declaring a variable in the for loop "for (int i = 0; i < 10; i++)"
> +   is still not allowed in this codebase.

Nice.

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Thanks.