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

Previous Next

Package: coreutils;

Reported by: Jim Meyering <jim <at> meyering.net>

Date: Thu, 15 Apr 2010 08:24:02 UTC

Severity: normal

Tags: patch

Done: Jim Meyering <jim <at> meyering.net>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: Jim Meyering <jim <at> meyering.net>
Subject: bug#5951 closed by Jim Meyering <jim <at> meyering.net> (Re: bug#5951:
 [PATCH] doc: document our code formatting policy	regarding curly braces)
Date: Fri, 16 Apr 2010 06:31:02 +0000
[Message part 1 (text/plain, inline)]
This is an automatic notification regarding your bug report
which was filed against the coreutils package:

#5951: [PATCH] doc: document our code formatting policy regarding curly braces

It has been closed by Jim Meyering <jim <at> meyering.net>.

Their explanation is attached below along with your original report.
If this explanation is unsatisfactory and you have not received a
better one in a separate message then please contact Jim Meyering <jim <at> meyering.net> by
replying to this email.


-- 
5951: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=5951
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems
[Message part 2 (message/rfc822, inline)]
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

[Message part 3 (message/rfc822, inline)]
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




This bug report was last modified 15 years and 44 days ago.

Previous Next


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