--- Begin Message ---
|
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 <address@hidden>
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
--- End Message ---
--- Begin Message ---
|
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 <address@hidden>
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
--- End Message ---