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

[James Almer]

On 11/8/2017 7:41 PM, Rostislav Pehlivanov wrote:
> On 8 November 2017 at 22:20, Mark Thompson <sw at jkqxz.net> wrote:
> 
>> On 08/11/17 22:03, Rostislav Pehlivanov wrote:
>>> On 8 November 2017 at 21:49, Mark Thompson <sw at jkqxz.net> wrote:
>>>
>>>> On 08/11/17 21:26, 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++)});
>>>>> +
>>>>>  @item
>>>>>  Implementation defined behavior for signed integers is assumed to
>> match
>>>> the
>>>>>  expected behavior for two's complement. Non representable values in
>>>> integer
>>>>>
>>>>
>>>> IMO if you want this it would be better to just allow mixed statements
>> and
>>>> declarations, with this as a consequence.
>>>>
>>>> Can you comment on what the consequences would be for platform support?
>>>> It would remove support for at least one platform I know of (the TI ARM
>>>> compiler).  I've no idea whether it or any other platform which would be
>>>> broken has any users, though.
>>>>
>>>
>>> No, I'm kind of against mixed statements and declarations, as are many
>>> people here. It mostly does make the code look worse and encourages
>> overuse
>>> of variables.
>>
>> I think the opposite.  I find declaration inside the loop header ugly and
>> weird, while mixed declarations and code do make sense in some cases: e.g.
>> pointer chasing through structures with different types - declaring all the
>> variables in advance is just annoying.  (Maybe that's not strong enough to
>> allow it everywhere if you believe that people will use it inappropriately
>> though.)
>>
>>
> I'm pretty sure its because you're not used to them yet. I'm not taking
> this as a nak.
> If you want mixed declaration submit a patch later on and let people
> comment on it.

It's the other way around. If you want to introduce some change, you're
the one that needs to convince other devs it's a good change, and so
far, two dislike it.
You can't commit this when people are against it saying "send a patch to
undo it later".

Besides, this patch alone is incomplete. Warnings about mixed code and
declaration are currently force enabled in configure.