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(); + 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
