r195256 - Added an option to allow short function bodies be placed on a single line.
Daniel Jasper
djasper at google.com
Wed Nov 20 09:12:58 PST 2013
On Wed, Nov 20, 2013 at 8:33 AM, Alexander Kornienko <alexfh at google.com>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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131120/065810ec/attachment.html>
More information about the cfe-commits
mailing list