[FFmpeg-devel] [PATCH] doc/developer: update style guidelines to include for loops with declarations

[James Almer]

On 11/9/2017 2:46 PM, Clément Bœsch wrote:
> On Wed, Nov 08, 2017 at 09:26:13PM +0000, Rostislav Pehlivanov wrote:
>> Signed-off-by: Rostislav Pehlivanov <atomnuker at gmail.com>
>> ---
>>  doc/developer.texi | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/doc/developer.texi b/doc/developer.texi
>> index a7b4f1d737..de7d887451 100644
>> --- a/doc/developer.texi
>> +++ b/doc/developer.texi
>> @@ -132,6 +132,9 @@ designated struct initializers (@samp{struct s x = @{ .i = 17 @};});
>>  @item
>>  compound literals (@samp{x = (struct s) @{ 17, 23 @};}).
>>  
>> + at item
>> +for loops with variable definition (@samp{for (int i = 0; i < 8; i++)});
>> +
> 
> I'm fine with this and would be happy to update all the code I maintain in
> FFmpeg to follow this pattern. But I have a few questions/reservations:
> 
> - what does it imply for mixed statements and declarations?
> 
>   If we still do not allow them, how are you going to make the compiler
>   reject them but not the for (int ... ) form?

No way to do that i guess. If we remove the warning, it will never catch
any kind of mixed declarations and statements anymore.

> 
>   Also, allowing this but not the mixed statements and declarations means
>   this is a style decision and not a technical one anymore.

Expanding on the above, doing this will become hard to enforce without
the warning, unless people reviewing patches start looking very closely
for them.

> 
> - are we going to accept all kind of patches to change the coding style
>   all over the codebase?

I'd rather not allow that. Big cosmetics changes tend to make using git
blame a PITA.

> 
> - this require a Changelog entry as it has a technical impact (which could
>   be considered negligible).

No, Changelog is not for this kind of change.

Maybe we should add some sort of changelog specific for doc/developer or
similar documentation.

> 
> Overall, I'd enjoy this change being accepted (even along mixed statements
> and declarations).

I personally find the current style cleaner looking, but i wouldn't
oppose this if it can be proven we're not breaking compilers currently
supported (and still used) just for it, as Mark requested in another email.
Afaik, MSVC 2012 and older stopped working with ffmpeg some time ago, or
at least are not tested by FATE any more.