<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Nov 20, 2013 at 8:33 AM, Alexander Kornienko <span dir="ltr"><<a href="mailto:alexfh@google.com" target="_blank" class="cremed">alexfh@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: alexfh<br>
Date: Wed Nov 20 10:33:05 2013<br>
New Revision: 195256<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=195256&view=rev" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project?rev=195256&view=rev</a><br>
Log:<br>
Added an option to allow short function bodies be placed on a single line.<br>
<br>
Summary:<br>
The AllowShortFunctionsOnASingleLine option now controls short function<br>
body placement on a single line independent of the BreakBeforeBraces option.<br>
Updated tests using BreakBeforeBraces other than BS_Attach.<br>
<br>
Addresses <a href="http://llvm.org/PR17888" target="_blank" class="cremed">http://llvm.org/PR17888</a><br>
<br>
Reviewers: klimek, djasper<br>
<br>
Reviewed By: klimek<br>
<br>
CC: cfe-commits, klimek<br>
<br>
Differential Revision: <a href="http://llvm-reviews.chandlerc.com/D2230" target="_blank" class="cremed">http://llvm-reviews.chandlerc.com/D2230</a><br>
<br>
Modified:<br>
cfe/trunk/include/clang/Format/Format.h<br>
cfe/trunk/lib/Format/Format.cpp<br>
cfe/trunk/lib/Format/FormatToken.h<br>
cfe/trunk/lib/Format/TokenAnnotator.cpp<br>
cfe/trunk/lib/Format/UnwrappedLineParser.cpp<br>
cfe/trunk/unittests/Format/FormatTest.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Format/Format.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=195256&r1=195255&r2=195256&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=195256&r1=195255&r2=195256&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Format/Format.h (original)<br>
+++ cfe/trunk/include/clang/Format/Format.h Wed Nov 20 10:33:05 2013<br>
@@ -141,6 +141,10 @@ struct FormatStyle {<br>
/// single line.<br>
bool AllowShortLoopsOnASingleLine;<br>
<br>
+ /// \brief If \c true, <tt>int f() { return 0; }</tt> can be put on a single<br>
+ /// line.<br>
+ bool AllowShortFunctionsOnASingleLine;<br>
+<br>
/// \brief Add a space in front of an Objective-C protocol list, i.e. use<br>
/// <tt>Foo <Protocol></tt> instead of \c Foo<Protocol>.<br>
bool ObjCSpaceBeforeProtocolList;<br>
@@ -255,6 +259,8 @@ struct FormatStyle {<br>
AlignTrailingComments == R.AlignTrailingComments &&<br>
AllowAllParametersOfDeclarationOnNextLine ==<br>
R.AllowAllParametersOfDeclarationOnNextLine &&<br>
+ AllowShortFunctionsOnASingleLine ==<br>
+ R.AllowShortFunctionsOnASingleLine &&<br>
AllowShortIfStatementsOnASingleLine ==<br>
R.AllowShortIfStatementsOnASingleLine &&<br>
AllowShortLoopsOnASingleLine == R.AllowShortLoopsOnASingleLine &&<br>
<br>
Modified: cfe/trunk/lib/Format/Format.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=195256&r1=195255&r2=195256&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=195256&r1=195255&r2=195256&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Format/Format.cpp (original)<br>
+++ cfe/trunk/lib/Format/Format.cpp Wed Nov 20 10:33:05 2013<br>
@@ -117,6 +117,8 @@ template <> struct MappingTraits<clang::<br>
Style.AllowShortIfStatementsOnASingleLine);<br>
IO.mapOptional("AllowShortLoopsOnASingleLine",<br>
Style.AllowShortLoopsOnASingleLine);<br>
+ IO.mapOptional("AllowShortFunctionsOnASingleLine",<br>
+ Style.AllowShortFunctionsOnASingleLine);<br>
IO.mapOptional("AlwaysBreakTemplateDeclarations",<br>
Style.AlwaysBreakTemplateDeclarations);<br>
IO.mapOptional("AlwaysBreakBeforeMultilineStrings",<br>
@@ -190,6 +192,7 @@ FormatStyle getLLVMStyle() {<br>
LLVMStyle.AlignEscapedNewlinesLeft = false;<br>
LLVMStyle.AlignTrailingComments = true;<br>
LLVMStyle.AllowAllParametersOfDeclarationOnNextLine = true;<br>
+ LLVMStyle.AllowShortFunctionsOnASingleLine = true;<br>
LLVMStyle.AllowShortIfStatementsOnASingleLine = false;<br>
LLVMStyle.AllowShortLoopsOnASingleLine = false;<br>
LLVMStyle.AlwaysBreakBeforeMultilineStrings = false;<br>
@@ -237,6 +240,7 @@ FormatStyle getGoogleStyle() {<br>
GoogleStyle.AlignEscapedNewlinesLeft = true;<br>
GoogleStyle.AlignTrailingComments = true;<br>
GoogleStyle.AllowAllParametersOfDeclarationOnNextLine = true;<br>
+ GoogleStyle.AllowShortFunctionsOnASingleLine = true;<br>
GoogleStyle.AllowShortIfStatementsOnASingleLine = true;<br>
GoogleStyle.AllowShortLoopsOnASingleLine = true;<br>
GoogleStyle.AlwaysBreakBeforeMultilineStrings = true;<br>
@@ -381,7 +385,7 @@ public:<br>
/// \brief Calculates how many lines can be merged into 1 starting at \p I.<br>
unsigned<br>
tryFitMultipleLinesInOne(unsigned Indent,<br>
- SmallVectorImpl<AnnotatedLine *>::const_iterator &I,<br>
+ SmallVectorImpl<AnnotatedLine *>::const_iterator I,<br>
SmallVectorImpl<AnnotatedLine *>::const_iterator E) {<br>
// We can never merge stuff if there are trailing line comments.<br>
AnnotatedLine *TheLine = *I;<br>
@@ -402,16 +406,43 @@ public:<br>
if (I + 1 == E || I[1]->Type == LT_Invalid)<br>
return 0;<br>
<br>
+ if (TheLine->Last->Type == TT_FunctionLBrace) {<br>
+ return Style.AllowShortFunctionsOnASingleLine<br>
+ ? tryMergeSimpleBlock(I, E, Limit)<br>
+ : 0;<br>
+ }<br>
if (TheLine->Last->is(tok::l_brace)) {<br>
- return tryMergeSimpleBlock(I, E, Limit);<br>
- } else if (Style.AllowShortIfStatementsOnASingleLine &&<br>
- TheLine->First->is(tok::kw_if)) {<br>
- return tryMergeSimpleControlStatement(I, E, Limit);<br>
- } else if (Style.AllowShortLoopsOnASingleLine &&<br>
- TheLine->First->isOneOf(tok::kw_for, tok::kw_while)) {<br>
- return tryMergeSimpleControlStatement(I, E, Limit);<br>
- } else if (TheLine->InPPDirective && (TheLine->First->HasUnescapedNewline ||<br>
- TheLine->First->IsFirst)) {<br>
+ return Style.BreakBeforeBraces == clang::format::FormatStyle::BS_Attach<br>
+ ? tryMergeSimpleBlock(I, E, Limit)<br>
+ : 0;<br>
+ }<br>
+ if (I[1]->First->Type == TT_FunctionLBrace &&<br>
+ Style.BreakBeforeBraces != FormatStyle::BS_Attach) {<br>
+ // Reduce the column limit by the number of spaces we need to insert<br>
+ // around braces.<br>
+ Limit = Limit > 3 ? Limit - 3 : 0;<br>
+ unsigned MergedLines = 0;<br>
+ if (Style.AllowShortFunctionsOnASingleLine) {<br>
+ MergedLines = tryMergeSimpleBlock(I + 1, E, Limit);<br>
+ // If we managed to merge the block, count the function header, which is<br>
+ // on a separate line.<br>
+ if (MergedLines > 0)<br>
+ ++MergedLines;<br>
+ }<br>
+ return MergedLines;<br>
+ }<br>
+ if (TheLine->First->is(tok::kw_if)) {<br>
+ return Style.AllowShortIfStatementsOnASingleLine<br>
+ ? tryMergeSimpleControlStatement(I, E, Limit)<br>
+ : 0;<br>
+ }<br>
+ if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while)) {<br>
+ return Style.AllowShortLoopsOnASingleLine<br>
+ ? tryMergeSimpleControlStatement(I, E, Limit)<br>
+ : 0;<br>
+ }<br>
+ if (TheLine->InPPDirective &&<br>
+ (TheLine->First->HasUnescapedNewline || TheLine->First->IsFirst)) {<br>
return tryMergeSimplePPDirective(I, E, Limit);<br>
}<br>
return 0;<br>
@@ -419,7 +450,7 @@ public:<br>
<br>
private:<br>
unsigned<br>
- tryMergeSimplePPDirective(SmallVectorImpl<AnnotatedLine *>::const_iterator &I,<br>
+ tryMergeSimplePPDirective(SmallVectorImpl<AnnotatedLine *>::const_iterator I,<br>
SmallVectorImpl<AnnotatedLine *>::const_iterator E,<br>
unsigned Limit) {<br>
if (Limit == 0)<br>
@@ -434,7 +465,7 @@ private:<br>
}<br>
<br>
unsigned tryMergeSimpleControlStatement(<br>
- SmallVectorImpl<AnnotatedLine *>::const_iterator &I,<br>
+ SmallVectorImpl<AnnotatedLine *>::const_iterator I,<br>
SmallVectorImpl<AnnotatedLine *>::const_iterator E, unsigned Limit) {<br>
if (Limit == 0)<br>
return 0;<br>
@@ -450,7 +481,7 @@ private:<br>
if (1 + I[1]->Last->TotalLength > Limit)<br>
return 0;<br>
if (I[1]->First->isOneOf(tok::semi, tok::kw_if, tok::kw_for,<br>
- tok::kw_while) ||<br>
+ tok::kw_while) ||<br>
I[1]->First->Type == TT_LineComment)<br>
return 0;<br>
// Only inline simple if's (no nested if or else).<br>
@@ -461,13 +492,9 @@ private:<br>
}<br>
<br>
unsigned<br>
- tryMergeSimpleBlock(SmallVectorImpl<AnnotatedLine *>::const_iterator &I,<br>
+ tryMergeSimpleBlock(SmallVectorImpl<AnnotatedLine *>::const_iterator I,<br>
SmallVectorImpl<AnnotatedLine *>::const_iterator E,<br>
unsigned Limit) {<br>
- // No merging if the brace already is on the next line.<br>
- if (Style.BreakBeforeBraces != FormatStyle::BS_Attach)<br>
- return 0;<br>
-<br>
// First, check that the current line allows merging. This is the case if<br>
// we're not in a control flow statement and the last token is an opening<br>
// brace.<br>
@@ -583,8 +610,7 @@ public:<br>
}<br>
} else if (TheLine.Type != LT_Invalid &&<br>
(WasMoved || FormatPPDirective || touchesLine(TheLine))) {<br>
- unsigned LevelIndent =<br>
- getIndent(IndentForLevel, TheLine.Level);<br>
+ unsigned LevelIndent = getIndent(IndentForLevel, TheLine.Level);<br>
if (FirstTok->WhitespaceRange.isValid()) {<br>
if (!DryRun)<br>
formatFirstToken(*TheLine.First, PreviousLine, TheLine.Level,<br>
@@ -732,9 +758,9 @@ private:<br>
if (PreviousLine && PreviousLine->First->isAccessSpecifier())<br>
Newlines = std::min(1u, Newlines);<br>
<br>
- Whitespaces->replaceWhitespace(<br>
- RootToken, Newlines, IndentLevel, Indent, Indent,<br>
- InPPDirective && !RootToken.HasUnescapedNewline);<br>
+ Whitespaces->replaceWhitespace(RootToken, Newlines, IndentLevel, Indent,<br>
+ Indent, InPPDirective &&<br>
+ !RootToken.HasUnescapedNewline);<br>
}<br>
<br>
/// \brief Get the indent of \p Level from \p IndentForLevel.<br>
@@ -961,7 +987,7 @@ private:<br>
<br>
// Cannot merge multiple statements into a single line.<br>
if (Previous.Children.size() > 1)<br>
- return false;<br>
+ return false;<br>
<br>
// We can't put the closing "}" on a line with a trailing comment.<br>
if (Previous.Children[0]->Last->isTrailingComment())<br>
<br>
Modified: cfe/trunk/lib/Format/FormatToken.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/FormatToken.h?rev=195256&r1=195255&r2=195256&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/FormatToken.h?rev=195256&r1=195255&r2=195256&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Format/FormatToken.h (original)<br>
+++ cfe/trunk/lib/Format/FormatToken.h Wed Nov 20 10:33:05 2013<br>
@@ -39,6 +39,7 @@ enum TokenType {<br>
TT_ImplicitStringLiteral,<br>
TT_InlineASMColon,<br>
TT_InheritanceColon,<br>
+ TT_FunctionLBrace,<br>
TT_FunctionTypeLParen,<br>
TT_LambdaLSquare,<br>
TT_LineComment,<br>
<br>
Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=195256&r1=195255&r2=195256&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=195256&r1=195255&r2=195256&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)<br>
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Wed Nov 20 10:33:05 2013<br>
@@ -538,6 +538,7 @@ private:<br>
// Reset token type in case we have already looked at it and then<br>
// recovered from an error (e.g. failure to find the matching >).<br>
if (CurrentToken->Type != TT_LambdaLSquare &&<br>
+ CurrentToken->Type != TT_FunctionLBrace &&<br>
CurrentToken->Type != TT_ImplicitStringLiteral)<br>
CurrentToken->Type = TT_Unknown;<br>
if (CurrentToken->Role)<br>
<br>
Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp?rev=195256&r1=195255&r2=195256&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp?rev=195256&r1=195255&r2=195256&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp (original)<br>
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp Wed Nov 20 10:33:05 2013<br>
@@ -681,6 +681,7 @@ void UnwrappedLineParser::parseStructura<br>
Style.BreakBeforeBraces == FormatStyle::BS_Stroustrup ||<br>
Style.BreakBeforeBraces == FormatStyle::BS_Allman)<br>
addUnwrappedLine();<br></blockquote><div><br></div><div>Does it still make sense to report the "{" as its own unwrapped line? Seems a bit convoluted to first report multiple lines and then merge them afterwards. I think this would make the merging code simpler.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ FormatTok->Type = TT_FunctionLBrace;<br>
parseBlock(/*MustBeDeclaration=*/false);<br>
addUnwrappedLine();<br>
return;<br>
<br>
Modified: cfe/trunk/unittests/Format/FormatTest.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=195256&r1=195255&r2=195256&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=195256&r1=195255&r2=195256&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)<br>
+++ cfe/trunk/unittests/Format/FormatTest.cpp Wed Nov 20 10:33:05 2013<br>
@@ -4776,8 +4776,15 @@ TEST_F(FormatTest, FormatsBracedListsInC<br>
}<br>
<br>
TEST_F(FormatTest, PullTrivialFunctionDefinitionsIntoSingleLine) {<br>
+ FormatStyle DoNotMerge = getLLVMStyle();<br>
+ DoNotMerge.AllowShortFunctionsOnASingleLine = false;<br>
+<br>
verifyFormat("void f() { return 42; }");<br>
verifyFormat("void f() {\n"<br>
+ " return 42;\n"<br>
+ "}",<br>
+ DoNotMerge);<br>
+ verifyFormat("void f() {\n"<br>
" // Comment\n"<br>
"}");<br>
verifyFormat("{\n"<br>
@@ -4792,6 +4799,13 @@ TEST_F(FormatTest, PullTrivialFunctionDe<br>
verifyFormat("void f() { int a; } // comment");<br>
verifyFormat("void f() {\n"<br>
"} // comment",<br>
+ DoNotMerge);<br>
+ verifyFormat("void f() {\n"<br>
+ " int a;\n"<br>
+ "} // comment",<br>
+ DoNotMerge);<br>
+ verifyFormat("void f() {\n"<br>
+ "} // comment",<br>
getLLVMStyleWithColumns(15));<br>
<br>
verifyFormat("void f() { return 42; }", getLLVMStyleWithColumns(23));<br>
@@ -6613,10 +6627,7 @@ TEST_F(FormatTest, LinuxBraceBreaking) {<br>
" b();\n"<br>
" }\n"<br>
" }\n"<br>
- " void g()\n"<br>
- " {\n"<br>
- " return;\n"<br>
- " }\n"<br>
+ " void g() { return; }\n"<br>
"}\n"<br>
"}",<br>
BreakBeforeBrace);<br>
@@ -6634,10 +6645,7 @@ TEST_F(FormatTest, StroustrupBraceBreaki<br>
" b();\n"<br>
" }\n"<br>
" }\n"<br>
- " void g()\n"<br>
- " {\n"<br>
- " return;\n"<br>
- " }\n"<br>
+ " void g() { return; }\n"<br>
"}\n"<br>
"}",<br>
BreakBeforeBrace);<br>
@@ -6658,10 +6666,7 @@ TEST_F(FormatTest, AllmanBraceBreaking)<br>
" b();\n"<br>
" }\n"<br>
" }\n"<br>
- " void g()\n"<br>
- " {\n"<br>
- " return;\n"<br>
- " }\n"<br>
+ " void g() { return; }\n"<br>
"}\n"<br>
"}",<br>
BreakBeforeBrace);<br>
@@ -6822,6 +6827,7 @@ TEST_F(FormatTest, ParsesConfiguration)<br>
CHECK_PARSE_BOOL(AlignEscapedNewlinesLeft);<br>
CHECK_PARSE_BOOL(AlignTrailingComments);<br>
CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);<br>
+ CHECK_PARSE_BOOL(AllowShortFunctionsOnASingleLine);<br>
CHECK_PARSE_BOOL(AllowShortIfStatementsOnASingleLine);<br>
CHECK_PARSE_BOOL(AllowShortLoopsOnASingleLine);<br>
CHECK_PARSE_BOOL(AlwaysBreakTemplateDeclarations);<br>
@@ -7126,7 +7132,7 @@ TEST_F(FormatTest, FormatsWithWebKitStyl<br>
" : aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaa)\n"<br>
" , aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaa, // break\n"<br>
" aaaaaaaaaaaaaa)\n"<br>
- " , aaaaaaaaaaaaaaaaaaaaaaa()\n{\n}",<br>
+ " , aaaaaaaaaaaaaaaaaaaaaaa() {}",<br>
Style);<br>
<br>
// Access specifiers should be aligned left.<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" class="cremed">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank" class="cremed">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div></div>