CODING_STYLE: don't allow non-indented statements after if/else blocks

Submitted by Aurelien Jarno on Oct. 26, 2009, 8:03 p.m.

Details

Message ID 20091026200326.GA9669@volta.aurel32.net
State New
Headers show

Commit Message

Aurelien Jarno Oct. 26, 2009, 8:03 p.m.
On Mon, Oct 26, 2009 at 06:02:52PM +0200, Blue Swirl wrote:
> On Mon, Oct 26, 2009 at 8:26 AM, Aurelien Jarno <aurelien@aurel32.net> wrote:
> > Rationale: The following code is difficult to read, but allowed by the
> > current coding style.
> 
> Fully agree.
> 
> > +Every control flow statement is followed by a new indented and braced
> > +block; even if the block contains just one statement.  The opening brace
> > +is on the line that contains the control flow statement that introduces
> > +the new block; the closing brace is on the same line as the else keyword,
> > +or on a line by itself if there is no else keyword.  Example:
> 
> I think an exception should be granted for "else if" case, otherwise
> the style would require braces around "if", like:
>     if (a == 5) {
>         printf("a was 5.\n");
>     } else {
>         if (a == 6) {
>             printf("a was 6.\n");
>         }
>     } else {
>         printf("a was something else entirely.\n");
>     }
> 
> Picking nits: "while" is a control flow statement, even in "do {}
> while" statement and then it would illegal to require a braced block
> after the "while" statement.

Good point. Please find another try below:

From: Aurelien Jarno <aurelien@aurel32.net>

Rationale: The following code is difficult to read:

    if (a == 5) printf("a was 5.\n");
    else if (a == 6) printf("a was 6.\n");
    else printf("a was something else entirely.\n");

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 CODING_STYLE |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

Comments

Blue Swirl Oct. 26, 2009, 8:20 p.m.
On Mon, Oct 26, 2009 at 10:03 PM, Aurelien Jarno <aurelien@aurel32.net> wrote:
> On Mon, Oct 26, 2009 at 06:02:52PM +0200, Blue Swirl wrote:
>> On Mon, Oct 26, 2009 at 8:26 AM, Aurelien Jarno <aurelien@aurel32.net> wrote:
>> > Rationale: The following code is difficult to read, but allowed by the
>> > current coding style.
>>
>> Fully agree.
>>
>> > +Every control flow statement is followed by a new indented and braced
>> > +block; even if the block contains just one statement.  The opening brace
>> > +is on the line that contains the control flow statement that introduces
>> > +the new block; the closing brace is on the same line as the else keyword,
>> > +or on a line by itself if there is no else keyword.  Example:
>>
>> I think an exception should be granted for "else if" case, otherwise
>> the style would require braces around "if", like:
>>     if (a == 5) {
>>         printf("a was 5.\n");
>>     } else {
>>         if (a == 6) {
>>             printf("a was 6.\n");
>>         }
>>     } else {
>>         printf("a was something else entirely.\n");
>>     }
>>
>> Picking nits: "while" is a control flow statement, even in "do {}
>> while" statement and then it would illegal to require a braced block
>> after the "while" statement.
>
> Good point. Please find another try below:
>
> From: Aurelien Jarno <aurelien@aurel32.net>
>
> Rationale: The following code is difficult to read:
>
>    if (a == 5) printf("a was 5.\n");
>    else if (a == 6) printf("a was 6.\n");
>    else printf("a was something else entirely.\n");
>
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> ---
>  CODING_STYLE |   12 +++++++-----
>  1 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/CODING_STYLE b/CODING_STYLE
> index a579cb1..c17c3f3 100644
> --- a/CODING_STYLE
> +++ b/CODING_STYLE
> @@ -51,11 +51,13 @@ QEMU coding style.
>
>  4. Block structure
>
> -Every indented statement is braced; even if the block contains just one
> -statement.  The opening brace is on the line that contains the control
> -flow statement that introduces the new block; the closing brace is on the
> -same line as the else keyword, or on a line by itself if there is no else
> -keyword.  Example:
> +Every control flow statement is followed by a new indented and braced
> +block, except if it is followed by another control flow statement (else
> +if) or by a condition (do {} while ()); even if the block contains just
> +one statement.  The opening brace is on the line that contains the
> +control flow statement that introduces the new block; the closing
> +brace is on the same line as the else keyword, or on a line by itself
> +if there is no else keyword.  Example:

Nice try, but does it prevent this:
    if (x) for (;;) do {
    } while (0);
?

Maybe also "break" and "goto" can be considered control flow statements.

How about something like "wherever C syntax allows potentially
ambiguous sequence of statements, braces must be used, with the
exception of 'else' followed by 'if'"? Now the problem becomes
defining ambiguous sequences of statements.
Aurelien Jarno Oct. 26, 2009, 8:27 p.m.
On Mon, Oct 26, 2009 at 10:20:34PM +0200, Blue Swirl wrote:
> On Mon, Oct 26, 2009 at 10:03 PM, Aurelien Jarno <aurelien@aurel32.net> wrote:
> > On Mon, Oct 26, 2009 at 06:02:52PM +0200, Blue Swirl wrote:
> >> On Mon, Oct 26, 2009 at 8:26 AM, Aurelien Jarno <aurelien@aurel32.net> wrote:
> >> > Rationale: The following code is difficult to read, but allowed by the
> >> > current coding style.
> >>
> >> Fully agree.
> >>
> >> > +Every control flow statement is followed by a new indented and braced
> >> > +block; even if the block contains just one statement.  The opening brace
> >> > +is on the line that contains the control flow statement that introduces
> >> > +the new block; the closing brace is on the same line as the else keyword,
> >> > +or on a line by itself if there is no else keyword.  Example:
> >>
> >> I think an exception should be granted for "else if" case, otherwise
> >> the style would require braces around "if", like:
> >>     if (a == 5) {
> >>         printf("a was 5.\n");
> >>     } else {
> >>         if (a == 6) {
> >>             printf("a was 6.\n");
> >>         }
> >>     } else {
> >>         printf("a was something else entirely.\n");
> >>     }
> >>
> >> Picking nits: "while" is a control flow statement, even in "do {}
> >> while" statement and then it would illegal to require a braced block
> >> after the "while" statement.
> >
> > Good point. Please find another try below:
> >
> > From: Aurelien Jarno <aurelien@aurel32.net>
> >
> > Rationale: The following code is difficult to read:
> >
> >    if (a == 5) printf("a was 5.\n");
> >    else if (a == 6) printf("a was 6.\n");
> >    else printf("a was something else entirely.\n");
> >
> > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> > ---
> >  CODING_STYLE |   12 +++++++-----
> >  1 files changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/CODING_STYLE b/CODING_STYLE
> > index a579cb1..c17c3f3 100644
> > --- a/CODING_STYLE
> > +++ b/CODING_STYLE
> > @@ -51,11 +51,13 @@ QEMU coding style.
> >
> >  4. Block structure
> >
> > -Every indented statement is braced; even if the block contains just one
> > -statement.  The opening brace is on the line that contains the control
> > -flow statement that introduces the new block; the closing brace is on the
> > -same line as the else keyword, or on a line by itself if there is no else
> > -keyword.  Example:
> > +Every control flow statement is followed by a new indented and braced
> > +block, except if it is followed by another control flow statement (else
> > +if) or by a condition (do {} while ()); even if the block contains just
> > +one statement.  The opening brace is on the line that contains the
> > +control flow statement that introduces the new block; the closing
> > +brace is on the same line as the else keyword, or on a line by itself
> > +if there is no else keyword.  Example:
> 
> Nice try, but does it prevent this:
>     if (x) for (;;) do {
>     } while (0);
> ?

We should find a way to define "for (;;)" as a single statement.

> Maybe also "break" and "goto" can be considered control flow statements.

Correct, they should be excluded from there as they don't need to be
followed by braces.

> How about something like "wherever C syntax allows potentially
> ambiguous sequence of statements, braces must be used, with the
> exception of 'else' followed by 'if'"? Now the problem becomes
> defining ambiguous sequences of statements.

That's the problem. We have seen that people already take advantage of
the ambiguities, as I would have never have imagined someone writing the
code in the rationale of this patch to avoid putting braces.
Anthony Liguori Oct. 26, 2009, 9:16 p.m.
Aurelien Jarno wrote:
> That's the problem. We have seen that people already take advantage of
> the ambiguities, as I would have never have imagined someone writing the
> code in the rationale of this patch to avoid putting braces.
>   

I appreciate the desire to be precise, but we aren't writing a compiler 
or an automated syntax checker.  We're writing a set of guidelines for 
reasonable people to follow.  If someone sets out to "defeat" 
CODING_STYLE by introducing some crazy syntax, the solution is 
simple--just don't take the patch.  Good sense must prevail regardless 
of what CODING_STYLE says.

Regards,

Anthony Liguori

Patch hide | download patch | download mbox

diff --git a/CODING_STYLE b/CODING_STYLE
index a579cb1..c17c3f3 100644
--- a/CODING_STYLE
+++ b/CODING_STYLE
@@ -51,11 +51,13 @@  QEMU coding style.
 
 4. Block structure
 
-Every indented statement is braced; even if the block contains just one
-statement.  The opening brace is on the line that contains the control
-flow statement that introduces the new block; the closing brace is on the
-same line as the else keyword, or on a line by itself if there is no else
-keyword.  Example:
+Every control flow statement is followed by a new indented and braced
+block, except if it is followed by another control flow statement (else
+if) or by a condition (do {} while ()); even if the block contains just
+one statement.  The opening brace is on the line that contains the
+control flow statement that introduces the new block; the closing
+brace is on the same line as the else keyword, or on a line by itself
+if there is no else keyword.  Example:
 
     if (a == 5) {
         printf("a was 5.\n");