[PATCH] [clang-format] AllowSimpleBracedStatements

[Gonzalo BG]

This patch adds a new option "AllowSimpleBracedStatements" that allows
braced statements to be considered as "simple". That is, if
AllowShortIfStatementsOnASingleLine and AllowShortLoopStatementsOnASingle
line are enabled, code like:

if (true) {
  f();
}

can be wrapped into a single line as:

if (true) { f(); }

AllowSimpleBracedStatements defaults to false, meaning that the above
transformation will not happen since braced block are considered "complex"
and thus are not allowed to be put on a single line.

This is a fix for http://llvm.org/bugs/show_bug.cgi?id=19193 . There, the
following remarks were made:

Things to do before this can get accepted:
> - You clang-format in LLVM style to format the changes (or e.g.
> git-clang-format).
> - Add tests, i.e. your test program should be put as unit tests into
> unittests/Format/FormatTest.cpp


These issues have been addressed (hopefully!) in the attached patch
and adding the tests has made the patch better (so thanks for the
feedback!).

Some newbie impressions on the clang-format code for those interested:

I have almost no experience with clang-format but after hacking on it a bit
I feel that in the future there should be a way to format blocks
independently of them being braced or not, belonging to a function, a
control statements, a loop, ... Right now there are multiple functions
dealing with the formatting of blocks and it felt a bit messy.

It also took me a bit to figure out how to add a new test, and how to
launch clang-format tests only (independently of all others llvm tests) but
this could probably fixed with a small paragraph in the docs.

Otherwise the code is self explanatory and well documented, and for someone
like me who has no idea what he is doing (and what clang-format is doing)
it was really easy to get clang-format to do what I wanted.

Bests,
Gonzalo BG
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140512/dd43d0fb/attachment.html>
-------------- next part --------------
--- unittests/Format/FormatTest.cpp	(before formatting)
+++ unittests/Format/FormatTest.cpp	(after formatting)
@@ -8880,7 +8880,6 @@
                    "int i;\n"));
 }
 
-
 // Tests AllowSimpleBracedStatements
 TEST_F(FormatTest, FormatSimpleBracedStatements) {
   FormatStyle AllowSimpleBracedStatements = getLLVMStyle();
--- lib/Format/Format.cpp	(before formatting)
+++ lib/Format/Format.cpp	(after formatting)
@@ -613,9 +613,8 @@
     if (Limit == 0)
       return 0;
     if ((Style.BreakBeforeBraces == FormatStyle::BS_Allman ||
-         Style.BreakBeforeBraces == FormatStyle::BS_GNU)
-        && (I[1]->First->is(tok::l_brace)
-            && !Style.AllowSimpleBracedStatements))
+         Style.BreakBeforeBraces == FormatStyle::BS_GNU) &&
+        (I[1]->First->is(tok::l_brace) && !Style.AllowSimpleBracedStatements))
       return 0;
     if (I[1]->InPPDirective != (*I)->InPPDirective ||
         (I[1]->InPPDirective && I[1]->First->HasUnescapedNewline))
@@ -642,43 +641,42 @@
                       SmallVectorImpl<AnnotatedLine *>::const_iterator E,
                       unsigned Limit) {
     AnnotatedLine &Line = **I;
-    // First, check that the current line allows merging: 
+    // First, check that the current line allows merging:
     // - this gets rid of all ObjC @ keywords and methods.
     if (Line.First->isOneOf(tok::at, tok::minus, tok::plus)) {
       return 0;
     }
-    // - this is the case if we're not inside a control flow statement: 
-    if (Line.First->isOneOf(tok::kw_else,  tok::kw_case)) {
+    // - this is the case if we're not inside a control flow statement:
+    if (Line.First->isOneOf(tok::kw_else, tok::kw_case)) {
       return 0;
-    } 
-    // - if merging of simple braced statements is not allowed, 
+    }
+    // - if merging of simple braced statements is not allowed,
     //   we can't merge:
-    //     - control flow statements (if, while, do, try, catch, for) 
+    //     - control flow statements (if, while, do, try, catch, for)
     //     - inline blocks {\n ... }
-    if (!Style.AllowSimpleBracedStatements) { 
+    if (!Style.AllowSimpleBracedStatements) {
       if (Line.First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_do,
-			      tok::kw_try,tok::kw_catch, tok::kw_for,
-			      tok::r_brace)) {
-	
-	return 0; 
-      }
-    } 
+                              tok::kw_try, tok::kw_catch, tok::kw_for,
+                              tok::r_brace)) {
+
+        return 0;
+      }
+    }
     // - if merging of simple braced statements is allowed, check
     // that we are allowed to merge simple ifs and simple loops
-    if (Style.AllowSimpleBracedStatements) { 
-      if (!Style.AllowShortIfStatementsOnASingleLine
-	  && Line.First->is(tok::kw_if)) {
-	return 0; 
-      }
-      if (!Style.AllowShortLoopsOnASingleLine
-	  && Line.First->isOneOf(tok::kw_while, tok::kw_do,
-				 tok::kw_for)) {
-	return 0; 
+    if (Style.AllowSimpleBracedStatements) {
+      if (!Style.AllowShortIfStatementsOnASingleLine &&
+          Line.First->is(tok::kw_if)) {
+        return 0;
+      }
+      if (!Style.AllowShortLoopsOnASingleLine &&
+          Line.First->isOneOf(tok::kw_while, tok::kw_do, tok::kw_for)) {
+        return 0;
       }
       // \todo There is no option to allow short exception handling
       // clauses on a single line. That could be fixed here.
       if (Line.First->isOneOf(tok::kw_try, tok::kw_catch)) {
-	return 0;
+        return 0;
       }
     }
 
@@ -708,8 +706,8 @@
       if (I[1]->Last->Type == TT_LineComment)
         return 0;
       do {
-        if (Tok->isOneOf(tok::l_brace, tok::r_brace)
-            && !Style.AllowSimpleBracedStatements)
+        if (Tok->isOneOf(tok::l_brace, tok::r_brace) &&
+            !Style.AllowSimpleBracedStatements)
           return 0;
         Tok = Tok->Next;
       } while (Tok);