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