[PATCH] [clang-format] AllowSimpleBracedStatements
Gonzalo BG
gonzalobg88 at gmail.com
Wed May 14 00:38:05 PDT 2014
>
> It looks like you might have attached the wrong diff. It contains nothing
> but formatting changes as far as I can tell.
Indeed. Sorry about that. Here is the whole patch. It is however
unformatted.
Bests,
Gonzalo
P.S: I tried to find how to format the diff with clang-format. The only way
I found was:
"svn diff | tools/clang-format/clang-format-diff.py > ~/patch.diff".
However, clang-format-diff swallows almost the whole patch..
I tried setting up "git clang-format" but it seems to be non trivial and
there seem to be no docs about it.
If you could point me to the documentation about how to set up git
clang-format and use it to format the patch I can try again.
On Wed, May 14, 2014 at 7:49 AM, Daniel Jasper <djasper at google.com> wrote:
> It looks like you might have attached the wrong diff. It contains
> nothing but formatting changes as far as I can tell.
>
> Cheers,
> Daniel
>
>
> On Mon, May 12, 2014 at 8:19 PM, Gonzalo BG <gonzalobg88 at gmail.com> wrote:
>
>> 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/20140514/81f6868c/attachment.html>
-------------- next part --------------
Index: docs/ClangFormatStyleOptions.rst
===================================================================
--- docs/ClangFormatStyleOptions.rst (revision 208542)
+++ docs/ClangFormatStyleOptions.rst (working copy)
@@ -106,6 +106,13 @@
Allow putting all parameters of a function declaration onto
the next line even if ``BinPackParameters`` is ``false``.
+**AllowSimpleBracedStatements** (``bool``)
+ Allows braced statements to be treated as simple. This
+ enables, e.g., the formatting of control statements
+ into the same line even when braces are used to delimit the
+ control block: ``if (a) { return; }`` ,
+ ``while (true) { continue; }``, ...
+
**AllowShortFunctionsOnASingleLine** (``ShortFunctionStyle``)
Dependent on the value, ``int f() { return 0; }`` can be put
on a single line.
Index: include/clang/Format/Format.h
===================================================================
--- include/clang/Format/Format.h (revision 208542)
+++ include/clang/Format/Format.h (working copy)
@@ -158,6 +158,10 @@
/// the commas with the colon.
bool BreakConstructorInitializersBeforeComma;
+ /// \brief Allows braced statements to be treated as simple enabling, among
+ /// others, <tt>if (a) { return; }</tt> to be put on a single line.
+ bool AllowSimpleBracedStatements;
+
/// \brief If \c true, <tt>if (a) return;</tt> can be put on a single
/// line.
bool AllowShortIfStatementsOnASingleLine;
@@ -338,6 +342,7 @@
R.AllowAllParametersOfDeclarationOnNextLine &&
AllowShortFunctionsOnASingleLine ==
R.AllowShortFunctionsOnASingleLine &&
+ AllowSimpleBracedStatements == R.AllowSimpleBracedStatements &&
AllowShortIfStatementsOnASingleLine ==
R.AllowShortIfStatementsOnASingleLine &&
AllowShortLoopsOnASingleLine == R.AllowShortLoopsOnASingleLine &&
Index: lib/Format/Format.cpp
===================================================================
--- lib/Format/Format.cpp (revision 208542)
+++ lib/Format/Format.cpp (working copy)
@@ -151,6 +151,8 @@
IO.mapOptional("AlignTrailingComments", Style.AlignTrailingComments);
IO.mapOptional("AllowAllParametersOfDeclarationOnNextLine",
Style.AllowAllParametersOfDeclarationOnNextLine);
+ IO.mapOptional("AllowSimpleBracedStatements",
+ Style.AllowSimpleBracedStatements);
IO.mapOptional("AllowShortIfStatementsOnASingleLine",
Style.AllowShortIfStatementsOnASingleLine);
IO.mapOptional("AllowShortLoopsOnASingleLine",
@@ -263,6 +265,7 @@
LLVMStyle.AlignTrailingComments = true;
LLVMStyle.AllowAllParametersOfDeclarationOnNextLine = true;
LLVMStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
+ LLVMStyle.AllowSimpleBracedStatements = false;
LLVMStyle.AllowShortIfStatementsOnASingleLine = false;
LLVMStyle.AllowShortLoopsOnASingleLine = false;
LLVMStyle.AlwaysBreakBeforeMultilineStrings = false;
@@ -320,6 +323,7 @@
GoogleStyle.AccessModifierOffset = -1;
GoogleStyle.AlignEscapedNewlinesLeft = true;
+ GoogleStyle.AllowSimpleBracedStatements = false;
GoogleStyle.AllowShortIfStatementsOnASingleLine = true;
GoogleStyle.AllowShortLoopsOnASingleLine = true;
GoogleStyle.AlwaysBreakBeforeMultilineStrings = true;
@@ -354,6 +358,7 @@
FormatStyle ChromiumStyle = getGoogleStyle(Language);
ChromiumStyle.AllowAllParametersOfDeclarationOnNextLine = false;
ChromiumStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
+ ChromiumStyle.AllowSimpleBracedStatements = false;
ChromiumStyle.AllowShortIfStatementsOnASingleLine = false;
ChromiumStyle.AllowShortLoopsOnASingleLine = false;
ChromiumStyle.BinPackParameters = false;
@@ -608,8 +613,9 @@
if (Limit == 0)
return 0;
if ((Style.BreakBeforeBraces == FormatStyle::BS_Allman ||
- Style.BreakBeforeBraces == FormatStyle::BS_GNU) &&
- I[1]->First->is(tok::l_brace))
+ 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))
@@ -635,16 +641,46 @@
tryMergeSimpleBlock(SmallVectorImpl<AnnotatedLine *>::const_iterator I,
SmallVectorImpl<AnnotatedLine *>::const_iterator E,
unsigned Limit) {
- // First, check that the current line allows merging. This is the case if
- // we're not in a control flow statement and the last token is an opening
- // brace.
AnnotatedLine &Line = **I;
- if (Line.First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_do, tok::r_brace,
- tok::kw_else, tok::kw_try, tok::kw_catch,
- tok::kw_for, tok::kw_case,
- // This gets rid of all ObjC @ keywords and methods.
- tok::at, tok::minus, tok::plus))
+ // 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)) {
+ return 0;
+ }
+ // - if merging of simple braced statements is not allowed,
+ // we can't merge:
+ // - control flow statements (if, while, do, try, catch, for)
+ // - inline blocks {\n ... }
+ 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;
+ }
+ }
+ // - 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;
+ }
+ // \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;
+ }
+ }
FormatToken *Tok = I[1]->First;
if (Tok->is(tok::r_brace) && !Tok->MustBreakBefore &&
@@ -672,7 +708,8 @@
if (I[1]->Last->Type == TT_LineComment)
return 0;
do {
- if (Tok->isOneOf(tok::l_brace, tok::r_brace))
+ if (Tok->isOneOf(tok::l_brace, tok::r_brace)
+ && !Style.AllowSimpleBracedStatements)
return 0;
Tok = Tok->Next;
} while (Tok);
Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp (revision 208542)
+++ unittests/Format/FormatTest.cpp (working copy)
@@ -7922,6 +7922,7 @@
CHECK_PARSE_BOOL(AlignEscapedNewlinesLeft);
CHECK_PARSE_BOOL(AlignTrailingComments);
CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
+ CHECK_PARSE_BOOL(AllowSimpleBracedStatements);
CHECK_PARSE_BOOL(AllowShortIfStatementsOnASingleLine);
CHECK_PARSE_BOOL(AllowShortLoopsOnASingleLine);
CHECK_PARSE_BOOL(AlwaysBreakTemplateDeclarations);
@@ -8879,5 +8880,28 @@
"int i;\n"));
}
+
+// Tests AllowSimpleBracedStatements
+TEST_F(FormatTest, FormatSimpleBracedStatements) {
+ FormatStyle AllowSimpleBracedStatements = getLLVMStyle();
+ AllowSimpleBracedStatements.AllowSimpleBracedStatements = true;
+
+ AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = true;
+ AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine = true;
+
+ verifyFormat("if (true) { f(); }", AllowSimpleBracedStatements);
+ verifyFormat("if (true) { //\n f();\n}", AllowSimpleBracedStatements);
+ verifyFormat("while (true) { f(); }", AllowSimpleBracedStatements);
+ verifyFormat("for (;;) { f(); }", AllowSimpleBracedStatements);
+ verifyFormat("if (true) {\n f();\n f();\n}", AllowSimpleBracedStatements);
+
+ AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = false;
+ verifyFormat("if (true) {\n f();\n}", AllowSimpleBracedStatements);
+
+ AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine = false;
+ verifyFormat("while (true) {\n f();\n}", AllowSimpleBracedStatements);
+ verifyFormat("for (;;) {\n f();\n}", AllowSimpleBracedStatements);
+}
+
} // end namespace tooling
} // end namespace clang
More information about the cfe-commits
mailing list