GNU bug report logs - #5951
[PATCH] doc: document our code formatting policy regarding curly braces

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 5951 in the body.
You can then email your comments to 5951 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: bug-coreutils <at> gnu.org
Subject: [PATCH] doc: document our code formatting policy regarding curly
	braces
Date: Thu, 15 Apr 2010 10:22:49 +0200

Hello,

I was burned by a multi-line single-stmt (no braces) loop body
in libvirt yesterday:

    http://thread.gmane.org/gmane.comp.emulators.libvirt/23715

and that has prompted me to write the following,
which codifies my personal policy/practice.  It may
be derived from the GCS, but I haven't checked yet.

Any suggestions or comments before I push?

From a7d51ecb8ea2788081a23f1dce4eb0d503c02ce4 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering <at> redhat.com>
Date: Thu, 15 Apr 2010 10:17:47 +0200
Subject: [PATCH] doc: document our code formatting policy regarding curly braces

* HACKING (Curly braces: use judiciously): New section.
---
 HACKING |  104 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 104 insertions(+), 0 deletions(-)

diff --git a/HACKING b/HACKING
index 124c666..7ccd2be 100644
--- a/HACKING
+++ b/HACKING
@@ -233,6 +233,110 @@ Try to make the summary line fit one of the following forms:
   maint: change-description


+Curly braces: use judiciously
+=============================
+Omit the curly braces around an "if", "while", "for" etc. body only when
+that body occupies a single line.  In every other case we require the braces.
+This ensures that it is trivially easy to identify a single-*statement* loop:
+each has only one *line* in its body.
+
+For example, do not omit the curly braces even when the body is just a
+single-line statement but with a preceding comment.
+
+Omitting braces with a single-line body is fine:
+
+     while (expr)
+       single_line_stmt ();
+
+However, the moment your loop/if/else body extends onto a second line,
+for whatever reason (even if it's just an added comment), then you should
+add braces.  Otherwise, it would be too easy to insert a statement just
+before that comment (without adding braces), thinking it is already a
+multi-statement loop:
+
+     while (true)
+       /* comment... */      // BAD: multi-line body without braces
+       single_line_stmt ();
+
+Do this instead:
+
+     while (true)
+       {  /* Always put braces around a multi-line body.  */
+         /* explanation... */
+         single_line_stmt ();
+       }
+
+There is one exception: when the second body line is not
+at the same indentation level as the first body line.
+
+     if (expr)
+       error (0, 0, _("a diagnostic that would make this line"
+                      " extend past the 80-column limit"));
+
+It seems safe not to require curly braces in this case,
+since the further-indented second body line makes it obvious
+that this is still a single-statement body.
+
+To reiterate, don't do this:
+
+     if (expr)
+       while (expr_2)        // BAD: multi-line body without braces
+         {
+           ...
+         }
+
+Do this, instead:
+
+     if (expr)
+       {
+         while (expr_2)
+           {
+             ...
+           }
+       }
+
+However, there is one exception in the other direction, when
+even a one-line block should have braces.
+That occurs when that one-line, brace-less block
+is an "else" block, and the corresponding "then" block *does* use braces.
+In that case, either put braces around the "else" block, or negate the
+"if"-condition and swap the bodies, putting the one-line block first
+and making the longer, multi-line block be the "else" block.
+
+    if (expr)
+      {
+        ...
+        ...
+      }
+    else
+      x = y;    // BAD: braceless "else" with braced "then"
+
+This is preferred, especially when the multi-line body is more
+than a few lines long, because it is easier to read and grasp
+the semantics of an if-then-else block when the simpler block
+occurs first, rather than after the more involved block:
+
+    if (!expr)
+      x = y;                  /* more readable */
+    else
+      {
+        ...
+        ...
+      }
+
+If you'd rather not negate the condition, then add braces:
+
+    if (expr)
+      {
+        ...
+        ...
+      }
+    else
+      {
+        x = y;
+      }
+
+
 Use SPACE-only indentation in all[*] files
 ==========================================
 We use space-only indentation in nearly all files.
--
1.7.1.rc1.248.gcefbb

Message #8 received at 5951 <at> debbugs.gnu.org (full text, mbox):

From: Eric Blake <eblake <at> redhat.com>
To: Jim Meyering <jim <at> meyering.net>
Cc: 5951 <at> debbugs.gnu.org
Subject: Re: bug#5951: [PATCH] doc: document our code formatting
	policy	regarding curly braces
Date: Thu, 15 Apr 2010 08:03:54 -0600

[Message part 1 (text/plain, inline)]

On 04/15/2010 02:22 AM, Jim Meyering wrote:
> Hello,
> 
> I was burned by a multi-line single-stmt (no braces) loop body
> in libvirt yesterday:
> 
>     http://thread.gmane.org/gmane.comp.emulators.libvirt/23715
> 
> and that has prompted me to write the following,
> which codifies my personal policy/practice.  It may
> be derived from the GCS, but I haven't checked yet.
> 
> Any suggestions or comments before I push?

Looks good, except:

> +Curly braces: use judiciously
> +=============================
> +Omit the curly braces around an "if", "while", "for" etc. body only when
> +that body occupies a single line.  In every other case we require the braces.
> +This ensures that it is trivially easy to identify a single-*statement* loop:
> +each has only one *line* in its body.
> +
> +For example, do not omit the curly braces even when the body is just a
> +single-line statement but with a preceding comment.

the paragraph above...

> +
> +Omitting braces with a single-line body is fine:
> +
> +     while (expr)
> +       single_line_stmt ();
> +
> +However, the moment your loop/if/else body extends onto a second line,
> +for whatever reason (even if it's just an added comment), then you should
> +add braces.  Otherwise, it would be too easy to insert a statement just
> +before that comment (without adding braces), thinking it is already a
> +multi-statement loop:

seems redundant with this paragraph.  Besides, it makes for an awkward flow:

when omitting is good
when it is bad
example of good
when it is bad
example of bad

Deleting the paragraph in question makes for a nicer flow, with no loss
of information:

when it is good
example of good
when it is bad
example of bad

-- 
Eric Blake   eblake <at> redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

[signature.asc (application/pgp-signature, attachment)]

Message #11 received at 5951 <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: Eric Blake <eblake <at> redhat.com>
Cc: 5951 <at> debbugs.gnu.org
Subject: Re: bug#5951: [PATCH] doc: document our code formatting
	policy	regarding curly braces
Date: Thu, 15 Apr 2010 19:00:34 +0200

Eric Blake wrote:

> On 04/15/2010 02:22 AM, Jim Meyering wrote:
>> Hello,
>>
>> I was burned by a multi-line single-stmt (no braces) loop body
>> in libvirt yesterday:
>>
>>     http://thread.gmane.org/gmane.comp.emulators.libvirt/23715
>>
>> and that has prompted me to write the following,
>> which codifies my personal policy/practice.  It may
>> be derived from the GCS, but I haven't checked yet.
>>
>> Any suggestions or comments before I push?
>
> Looks good, except:
>
>> +Curly braces: use judiciously
>> +=============================
>> +Omit the curly braces around an "if", "while", "for" etc. body only when
>> +that body occupies a single line.  In every other case we require the braces.
>> +This ensures that it is trivially easy to identify a single-*statement* loop:
>> +each has only one *line* in its body.
>> +
>> +For example, do not omit the curly braces even when the body is just a
>> +single-line statement but with a preceding comment.
>
> the paragraph above...

Thanks.  I removed those two lines and made this change below:

  -It seems safe not to require curly braces in this case,
  +It is safe not to require curly braces in code like this,
   since the further-indented second body line makes it obvious
   that this is still a single-statement body.

From f8291d0ec489c6363769c3c767b161ffbdb7f082 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering <at> redhat.com>
Date: Thu, 15 Apr 2010 10:17:47 +0200
Subject: [PATCH] doc: document our code formatting policy regarding curly braces

* HACKING (Curly braces: use judiciously): New section.
---
 HACKING |  101 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 101 insertions(+), 0 deletions(-)

diff --git a/HACKING b/HACKING
index 124c666..18e9c54 100644
--- a/HACKING
+++ b/HACKING
@@ -233,6 +233,107 @@ Try to make the summary line fit one of the following forms:
   maint: change-description


+Curly braces: use judiciously
+=============================
+Omit the curly braces around an "if", "while", "for" etc. body only when
+that body occupies a single line.  In every other case we require the braces.
+This ensures that it is trivially easy to identify a single-*statement* loop:
+each has only one *line* in its body.
+
+Omitting braces with a single-line body is fine:
+
+     while (expr)
+       single_line_stmt ();
+
+However, the moment your loop/if/else body extends onto a second line,
+for whatever reason (even if it's just an added comment), then you should
+add braces.  Otherwise, it would be too easy to insert a statement just
+before that comment (without adding braces), thinking it is already a
+multi-statement loop:
+
+     while (true)
+       /* comment... */      // BAD: multi-line body without braces
+       single_line_stmt ();
+
+Do this instead:
+
+     while (true)
+       {  /* Always put braces around a multi-line body.  */
+         /* explanation... */
+         single_line_stmt ();
+       }
+
+There is one exception: when the second body line is not
+at the same indentation level as the first body line.
+
+     if (expr)
+       error (0, 0, _("a diagnostic that would make this line"
+                      " extend past the 80-column limit"));
+
+It is safe not to require curly braces in code like this,
+since the further-indented second body line makes it obvious
+that this is still a single-statement body.
+
+To reiterate, don't do this:
+
+     if (expr)
+       while (expr_2)        // BAD: multi-line body without braces
+         {
+           ...
+         }
+
+Do this, instead:
+
+     if (expr)
+       {
+         while (expr_2)
+           {
+             ...
+           }
+       }
+
+However, there is one exception in the other direction, when
+even a one-line block should have braces.
+That occurs when that one-line, brace-less block
+is an "else" block, and the corresponding "then" block *does* use braces.
+In that case, either put braces around the "else" block, or negate the
+"if"-condition and swap the bodies, putting the one-line block first
+and making the longer, multi-line block be the "else" block.
+
+    if (expr)
+      {
+        ...
+        ...
+      }
+    else
+      x = y;    // BAD: braceless "else" with braced "then"
+
+This is preferred, especially when the multi-line body is more
+than a few lines long, because it is easier to read and grasp
+the semantics of an if-then-else block when the simpler block
+occurs first, rather than after the more involved block:
+
+    if (!expr)
+      x = y;                  /* more readable */
+    else
+      {
+        ...
+        ...
+      }
+
+If you'd rather not negate the condition, then add braces:
+
+    if (expr)
+      {
+        ...
+        ...
+      }
+    else
+      {
+        x = y;
+      }
+
+
 Use SPACE-only indentation in all[*] files
 ==========================================
 We use space-only indentation in nearly all files.
--
1.7.1.rc1.248.gcefbb

Message #16 received at 5951-done <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: Eric Blake <eblake <at> redhat.com>
Cc: 5951-done <at> debbugs.gnu.org
Subject: Re: bug#5951: [PATCH] doc: document our code formatting
	policy	regarding curly braces
Date: Fri, 16 Apr 2010 08:30:12 +0200

...
> Thanks.  I removed those two lines and made this change below:
>
>   -It seems safe not to require curly braces in this case,
>   +It is safe not to require curly braces in code like this,
>    since the further-indented second body line makes it obvious
>    that this is still a single-statement body.

One more tweak:
(only the first hunk has a wording change.
The others are just re-flowed. )

From 36cc6ac787d3c8f98c88cfe14c42fe27027b785b Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering <at> redhat.com>
Date: Fri, 16 Apr 2010 08:21:32 +0200
Subject: [PATCH] doc: tweak HACKING

* HACKING (Curly braces): Tweak a sentence.  Filter a few
paragraphs through "fmt".
---
 HACKING |   32 ++++++++++++++++----------------
 1 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/HACKING b/HACKING
index 18e9c54..a6589d3 100644
--- a/HACKING
+++ b/HACKING
@@ -263,16 +263,16 @@ Do this instead:
          single_line_stmt ();
        }

-There is one exception: when the second body line is not
-at the same indentation level as the first body line.
+There is one exception: when the second body line is not at the same
+indentation level as the first body line.

      if (expr)
        error (0, 0, _("a diagnostic that would make this line"
                       " extend past the 80-column limit"));

-It is safe not to require curly braces in code like this,
-since the further-indented second body line makes it obvious
-that this is still a single-statement body.
+It is safe to omit the braces in the code above, since the
+further-indented second body line makes it obvious that this is still
+a single-statement body.

 To reiterate, don't do this:

@@ -292,13 +292,13 @@ Do this, instead:
            }
        }

-However, there is one exception in the other direction, when
-even a one-line block should have braces.
-That occurs when that one-line, brace-less block
-is an "else" block, and the corresponding "then" block *does* use braces.
-In that case, either put braces around the "else" block, or negate the
-"if"-condition and swap the bodies, putting the one-line block first
-and making the longer, multi-line block be the "else" block.
+However, there is one exception in the other direction, when even a
+one-line block should have braces.  That occurs when that one-line,
+brace-less block is an "else" block, and the corresponding "then" block
+*does* use braces.  In that case, either put braces around the "else"
+block, or negate the "if"-condition and swap the bodies, putting the
+one-line block first and making the longer, multi-line block be the
+"else" block.

     if (expr)
       {
@@ -308,10 +308,10 @@ and making the longer, multi-line block be the "else" block.
     else
       x = y;    // BAD: braceless "else" with braced "then"

-This is preferred, especially when the multi-line body is more
-than a few lines long, because it is easier to read and grasp
-the semantics of an if-then-else block when the simpler block
-occurs first, rather than after the more involved block:
+This is preferred, especially when the multi-line body is more than a
+few lines long, because it is easier to read and grasp the semantics of
+an if-then-else block when the simpler block occurs first, rather than
+after the more involved block:

     if (!expr)
       x = y;                  /* more readable */
--
1.7.1.rc1.269.ga27c7

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.