r179287 - Change clang-format's affinity for breaking after return types.
Daniel Jasper
djasper at google.com
Thu Apr 11 07:29:13 PDT 2013
Author: djasper
Date: Thu Apr 11 09:29:13 2013
New Revision: 179287
URL: http://llvm.org/viewvc/llvm-project?rev=179287&view=rev
Log:
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.
Modified:
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
Modified: cfe/trunk/lib/Format/Format.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=179287&r1=179286&r2=179287&view=diff
==============================================================================
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Thu Apr 11 09:29:13 2013
@@ -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())
Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=179287&r1=179286&r2=179287&view=diff
==============================================================================
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Thu Apr 11 09:29:13 2013
@@ -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(co
// 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(co
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;
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=179287&r1=179286&r2=179287&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Thu Apr 11 09:29:13 2013
@@ -1199,8 +1199,8 @@ TEST_F(FormatTest, FormatsSmallMacroDefi
TEST_F(FormatTest, DoesNotBreakPureVirtualFunctionDefinition) {
verifyFormat(
- "virtual void\n"
- "write(ELFWriter *writerrr, OwningPtr<FileOutputBuffer> &buffer) = 0;");
+ "virtual void write(ELFWriter *writerrr,\n"
+ " OwningPtr<FileOutputBuffer> &buffer) = 0;");
}
TEST_F(FormatTest, LayoutUnknownPPDirective) {
@@ -1700,6 +1700,56 @@ TEST_F(FormatTest, BreaksAsHighAsPossibl
" 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, DoesNotBreakTrailingA
" 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, BreaksLongDeclaration
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<TemplateParam1, TemplateParam2>\n"
"ReallyReallyLongFunctionName(\n"
" const std::string &SomeParameter,\n"
More information about the cfe-commits
mailing list