From: Daniel Jasper Date: Thu, 11 Apr 2013 14:29:13 +0000 (+0000) Subject: Change clang-format's affinity for breaking after return types. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=1407bee187d7b964d5293ac8bf4f7a490c78cec6;p=clang Change clang-format's affinity for breaking after return types. Function declarations are now broken with the following preferences: 1) break amongst arguments. 2) break after return type. 3) break after (. 4) break before after nested name specifiers. Options #2 or #3 are preferred over #1 only if a substantial number of lines can be saved by that. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@179287 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Format/Format.cpp b/lib/Format/Format.cpp index 98ae0bdbf9..e956f69875 100644 --- a/lib/Format/Format.cpp +++ b/lib/Format/Format.cpp @@ -48,7 +48,7 @@ FormatStyle getLLVMStyle() { LLVMStyle.AllowShortIfStatementsOnASingleLine = false; LLVMStyle.ObjCSpaceBeforeProtocolList = true; LLVMStyle.PenaltyExcessCharacter = 1000000; - LLVMStyle.PenaltyReturnTypeOnItsOwnLine = 5; + LLVMStyle.PenaltyReturnTypeOnItsOwnLine = 75; return LLVMStyle; } @@ -68,7 +68,7 @@ FormatStyle getGoogleStyle() { GoogleStyle.AllowShortIfStatementsOnASingleLine = false; GoogleStyle.ObjCSpaceBeforeProtocolList = false; GoogleStyle.PenaltyExcessCharacter = 1000000; - GoogleStyle.PenaltyReturnTypeOnItsOwnLine = 100; + GoogleStyle.PenaltyReturnTypeOnItsOwnLine = 200; return GoogleStyle; } @@ -1395,7 +1395,7 @@ public: // Adapt level to the next line if this is a comment. // FIXME: Can/should this be done in the UnwrappedLineParser? - const AnnotatedLine* NextNoneCommentLine = NULL; + const AnnotatedLine *NextNoneCommentLine = NULL; for (unsigned i = AnnotatedLines.size() - 1; i > 0; --i) { if (NextNoneCommentLine && AnnotatedLines[i].First.is(tok::comment) && AnnotatedLines[i].First.Children.empty()) diff --git a/lib/Format/TokenAnnotator.cpp b/lib/Format/TokenAnnotator.cpp index 08d9b10694..3a432c57e0 100644 --- a/lib/Format/TokenAnnotator.cpp +++ b/lib/Format/TokenAnnotator.cpp @@ -81,7 +81,7 @@ public: AnnotatingParser(SourceManager &SourceMgr, Lexer &Lex, AnnotatedLine &Line, IdentifierInfo &Ident_in) : SourceMgr(SourceMgr), Lex(Lex), Line(Line), CurrentToken(&Line.First), - KeywordVirtualFound(false), Ident_in(Ident_in) { + KeywordVirtualFound(false), NameFound(false), Ident_in(Ident_in) { Contexts.push_back(Context(tok::unknown, 1, /*IsExpression=*/ false)); } @@ -347,7 +347,7 @@ private: case tok::l_paren: if (!parseParens()) return false; - if (Line.MustBeDeclaration) + if (Line.MustBeDeclaration && NameFound && !Contexts.back().IsExpression) Line.MightBeFunctionDecl = true; break; case tok::l_square: @@ -602,6 +602,7 @@ private: Current.Parent->Type == TT_TemplateCloser)) { Contexts.back().FirstStartOfName = &Current; Current.Type = TT_StartOfName; + NameFound = true; } else if (Current.isOneOf(tok::star, tok::amp, tok::ampamp)) { Current.Type = determineStarAmpUsage(Current, Contexts.back().IsExpression); @@ -759,6 +760,7 @@ private: AnnotatedLine &Line; AnnotatedToken *CurrentToken; bool KeywordVirtualFound; + bool NameFound; IdentifierInfo &Ident_in; }; @@ -916,7 +918,7 @@ unsigned TokenAnnotator::splitPenalty(const AnnotatedLine &Line, // FIXME: Clean up hack of using BindingStrength to find top-level names. return Style.PenaltyReturnTypeOnItsOwnLine; else - return 100; + return 200; } if (Left.is(tok::equal) && Right.is(tok::l_brace)) return 150; @@ -954,6 +956,8 @@ unsigned TokenAnnotator::splitPenalty(const AnnotatedLine &Line, if (Left.is(tok::colon) && Left.Type == TT_ObjCMethodExpr) return 20; + if (Left.is(tok::l_paren) && Line.MightBeFunctionDecl) + return 100; if (Left.opensScope()) return Left.ParameterCount > 1 ? prec::Comma : 20; diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp index 09fbce830e..91b8f81080 100644 --- a/unittests/Format/FormatTest.cpp +++ b/unittests/Format/FormatTest.cpp @@ -1199,8 +1199,8 @@ TEST_F(FormatTest, FormatsSmallMacroDefinitionsInSingleLine) { TEST_F(FormatTest, DoesNotBreakPureVirtualFunctionDefinition) { verifyFormat( - "virtual void\n" - "write(ELFWriter *writerrr, OwningPtr &buffer) = 0;"); + "virtual void write(ELFWriter *writerrr,\n" + " OwningPtr &buffer) = 0;"); } TEST_F(FormatTest, LayoutUnknownPPDirective) { @@ -1700,6 +1700,56 @@ TEST_F(FormatTest, BreaksAsHighAsPossible) { " Intervals[i - 1].getRange().getLast()) {\n}"); } +TEST_F(FormatTest, BreaksFunctionDeclarations) { + // Principially, we break function declarations in a certain order: + // 1) break amongst arguments. + verifyFormat("Aaaaaaaaaaaaaa bbbbbbbbbbbbbb(Cccccccccccccc cccccccccccccc,\n" + " Cccccccccccccc cccccccccccccc);"); + + // 2) break after return type. + verifyFormat("Aaaaaaaaaaaaaaaaaaaaaaaa\n" + "bbbbbbbbbbbbbb(Cccccccccccccc cccccccccccccccccccccccccc);"); + + // 3) break after (. + verifyFormat( + "Aaaaaaaaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbb(\n" + " Cccccccccccccccccccccccccccccc cccccccccccccccccccccccccccccccc);"); + + // 4) break before after nested name specifiers. + verifyFormat( + "Aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" + "SomeClasssssssssssssssssssssssssssssssssssssss::\n" + " bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb(Cccccccccccccc cccccccccc);"); + + // However, there are exceptions, if a sufficient amount of lines can be + // saved. + // FIXME: The precise cut-offs wrt. the number of saved lines might need some + // more adjusting. + verifyFormat("Aaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbb(Cccccccccccccc cccccccccc,\n" + " Cccccccccccccc cccccccccc,\n" + " Cccccccccccccc cccccccccc,\n" + " Cccccccccccccc cccccccccc,\n" + " Cccccccccccccc cccccccccc);"); + verifyFormat( + "Aaaaaaaaaaaaaaaaaa\n" + "bbbbbbbbbbbbbb(Cccccccccccccc cccccccccc, Cccccccccccccc cccccccccc,\n" + " Cccccccccccccc cccccccccc, Cccccccccccccc cccccccccc,\n" + " Cccccccccccccc cccccccccc, Cccccccccccccc cccccccccc);"); + verifyFormat( + "Aaaaaaaaaa bbbbbbbbbbbbbbbbbbbbbbbbbbbbbb(Cccccccccccccc cccccccccc,\n" + " Cccccccccccccc cccccccccc,\n" + " Cccccccccccccc cccccccccc,\n" + " Cccccccccccccc cccccccccc,\n" + " Cccccccccccccc cccccccccc,\n" + " Cccccccccccccc cccccccccc,\n" + " Cccccccccccccc cccccccccc);"); + verifyFormat("Aaaaaaaaaa bbbbbbbbbbbbbbbbbbbbbbbbbbbbbb(\n" + " Cccccccccccccc cccccccccc, Cccccccccccccc cccccccccc,\n" + " Cccccccccccccc cccccccccc, Cccccccccccccc cccccccccc,\n" + " Cccccccccccccc cccccccccc, Cccccccccccccc cccccccccc,\n" + " Cccccccccccccc cccccccccc, Cccccccccccccc cccccccccc);"); +} + TEST_F(FormatTest, BreaksDesireably) { verifyFormat("if (aaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaa) ||\n" " aaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaa) ||\n" @@ -1850,7 +1900,7 @@ TEST_F(FormatTest, DoesNotBreakTrailingAnnotation) { " aaaaaaaaaaaaaaaaaaaaaaaaa));"); verifyFormat("bool aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" " __attribute__((unused));"); - + // FIXME: This is bad indentation, but generally hard to distinguish from a // function declaration. verifyFormat( @@ -2600,9 +2650,9 @@ TEST_F(FormatTest, BreaksLongDeclarations) { verifyFormat("int *someFunction(int LoooooooooooooooooooongParam1,\n" " int LoooooooooooooooooooongParam2) {}"); verifyFormat( - "TypeSpecDecl *\n" - "TypeSpecDecl::Create(ASTContext &C, DeclContext *DC, SourceLocation L,\n" - " IdentifierIn *II, Type *T) {}"); + "TypeSpecDecl *TypeSpecDecl::Create(ASTContext &C, DeclContext *DC,\n" + " SourceLocation L, IdentifierIn *II,\n" + " Type *T) {}"); verifyFormat("ReallyLongReturnType\n" "ReallyReallyLongFunctionName(\n" " const std::string &SomeParameter,\n"