r175999 - Allow breaking between a type and name in variable declarations.
Daniel Jasper
djasper at google.com
Sun Feb 24 10:54:33 PST 2013
Author: djasper
Date: Sun Feb 24 12:54:32 2013
New Revision: 175999
URL: http://llvm.org/viewvc/llvm-project?rev=175999&view=rev
Log:
Allow breaking between a type and name in variable declarations.
This fixes llvm.org/PR14967 and is generall necessary to avoid
situations where the column limit is exceeded. The challenge is
restricting such lines splits, otherwise clang-format suddenly starts
breaking at bad places.
Before:
ReallyLongReturnType<TemplateParam1, TemplateParam2>
ReallyReallyLongFunctionName(
const std::string &SomeParameter,
const SomeType<string,
SomeOtherTemplateParameter> &ReallyReallyLongParameterName,
const SomeType<string,
SomeOtherTemplateParameter> &AnotherLongParameterName) {}
After:
ReallyLongReturnType<TemplateParam1, TemplateParam2>
ReallyReallyLongFunctionName(
const std::string &SomeParameter,
const SomeType<string, SomeOtherTemplateParameter> &
ReallyReallyLongParameterName,
const SomeType<string, SomeOtherTemplateParameter> &
AnotherLongParameterName) {}
Modified:
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/lib/Format/TokenAnnotator.h
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=175999&r1=175998&r2=175999&view=diff
==============================================================================
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Sun Feb 24 12:54:32 2013
@@ -258,12 +258,6 @@ private:
tooling::Replacements Replaces;
};
-static bool isVarDeclName(const AnnotatedToken &Tok) {
- return Tok.Parent != NULL && Tok.is(tok::identifier) &&
- (Tok.Parent->Type == TT_PointerOrReference ||
- Tok.Parent->is(tok::identifier));
-}
-
class UnwrappedLineFormatter {
public:
UnwrappedLineFormatter(const FormatStyle &Style, SourceManager &SourceMgr,
@@ -498,8 +492,8 @@ private:
((RootToken.is(tok::kw_for) && State.ParenLevel == 1) ||
State.ParenLevel == 0)) {
State.Column = State.VariablePos;
- } else if (State.NextToken->Parent->ClosesTemplateDeclaration ||
- Current.Type == TT_StartOfName) {
+ } else if (Previous.ClosesTemplateDeclaration ||
+ (Current.Type == TT_StartOfName && State.ParenLevel == 0)) {
State.Column = State.Stack.back().Indent - 4;
} else if (Current.Type == TT_ObjCSelectorName) {
if (State.Stack.back().ColonPos > Current.FormatTok.TokenLength) {
@@ -510,7 +504,8 @@ private:
State.Stack.back().ColonPos =
State.Column + Current.FormatTok.TokenLength;
}
- } else if (Previous.Type == TT_ObjCMethodExpr || isVarDeclName(Current)) {
+ } else if (Previous.Type == TT_ObjCMethodExpr ||
+ Current.Type == TT_StartOfName) {
State.Column = State.Stack.back().Indent + 4;
} else {
State.Column = State.Stack.back().Indent;
@@ -551,8 +546,7 @@ private:
if (State.Stack.back().AvoidBinPacking) {
// If we are breaking after '(', '{', '<', this is not bin packing
// unless AllowAllParametersOfDeclarationOnNextLine is false.
- if ((Previous.isNot(tok::l_paren) && Previous.isNot(tok::l_brace) &&
- Previous.Type != TT_TemplateOpener) ||
+ if ((Previous.isNot(tok::l_paren) && Previous.isNot(tok::l_brace)) ||
(!Style.AllowAllParametersOfDeclarationOnNextLine &&
Line.MustBeDeclaration))
State.Stack.back().BreakBeforeParameter = true;
Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=175999&r1=175998&r2=175999&view=diff
==============================================================================
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Sun Feb 24 12:54:32 2013
@@ -83,7 +83,6 @@ public:
: SourceMgr(SourceMgr), Lex(Lex), Line(Line), CurrentToken(&Line.First),
KeywordVirtualFound(false), Ident_in(Ident_in) {
Contexts.push_back(Context(1, /*IsExpression=*/ false));
- Contexts.back().LookForFunctionName = Line.MustBeDeclaration;
}
private:
@@ -348,6 +347,8 @@ private:
case tok::l_paren:
if (!parseParens())
return false;
+ if (Line.MustBeDeclaration)
+ Line.MightBeFunctionDecl = true;
break;
case tok::l_square:
if (!parseSquare())
@@ -520,8 +521,7 @@ private:
Context(unsigned BindingStrength, bool IsExpression)
: BindingStrength(BindingStrength), LongestObjCSelectorName(0),
ColonIsForRangeExpr(false), ColonIsObjCMethodExpr(false),
- FirstObjCSelectorName(NULL), IsExpression(IsExpression),
- LookForFunctionName(false) {}
+ FirstObjCSelectorName(NULL), IsExpression(IsExpression) {}
unsigned BindingStrength;
unsigned LongestObjCSelectorName;
@@ -529,7 +529,6 @@ private:
bool ColonIsObjCMethodExpr;
AnnotatedToken *FirstObjCSelectorName;
bool IsExpression;
- bool LookForFunctionName;
};
/// \brief Puts a new \c Context onto the stack \c Contexts for the lifetime
@@ -572,9 +571,13 @@ private:
}
if (Current.Type == TT_Unknown) {
- if (Contexts.back().LookForFunctionName && Current.is(tok::l_paren)) {
- findFunctionName(&Current);
- Contexts.back().LookForFunctionName = false;
+ if (Current.Parent && Current.is(tok::identifier) &&
+ ((Current.Parent->is(tok::identifier) &&
+ Current.Parent->FormatTok.Tok.getIdentifierInfo()
+ ->getPPKeywordID() == tok::pp_not_keyword) ||
+ Current.Parent->Type == TT_PointerOrReference ||
+ Current.Parent->Type == TT_TemplateCloser)) {
+ Current.Type = TT_StartOfName;
} else if (Current.is(tok::star) || Current.is(tok::amp)) {
Current.Type =
determineStarAmpUsage(Current, Contexts.back().IsExpression);
@@ -623,22 +626,6 @@ private:
}
}
- /// \brief Starting from \p Current, this searches backwards for an
- /// identifier which could be the start of a function name and marks it.
- void findFunctionName(AnnotatedToken *Current) {
- AnnotatedToken *Parent = Current->Parent;
- while (Parent != NULL && Parent->Parent != NULL) {
- if (Parent->is(tok::identifier) &&
- (Parent->Parent->is(tok::identifier) ||
- Parent->Parent->Type == TT_PointerOrReference ||
- Parent->Parent->Type == TT_TemplateCloser)) {
- Parent->Type = TT_StartOfName;
- break;
- }
- Parent = Parent->Parent;
- }
- }
-
/// \brief Return the type of the given token assuming it is * or &.
TokenType
determineStarAmpUsage(const AnnotatedToken &Tok, bool IsExpression) {
@@ -883,8 +870,15 @@ unsigned TokenAnnotator::splitPenalty(co
const AnnotatedToken &Left = *Tok.Parent;
const AnnotatedToken &Right = Tok;
- if (Right.Type == TT_StartOfName)
- return Style.PenaltyReturnTypeOnItsOwnLine;
+ if (Right.Type == TT_StartOfName) {
+ if (Line.First.is(tok::kw_for))
+ return 3;
+ else if (Line.MightBeFunctionDecl && Right.BindingStrength == 1)
+ // FIXME: Clean up hack of using BindingStrength to find top-level names.
+ return Style.PenaltyReturnTypeOnItsOwnLine;
+ else
+ return 100;
+ }
if (Left.is(tok::l_brace) && Right.isNot(tok::l_brace))
return 50;
if (Left.is(tok::equal) && Right.is(tok::l_brace))
@@ -941,7 +935,7 @@ unsigned TokenAnnotator::splitPenalty(co
if (Level != prec::Unknown)
return Level;
-
+
return 3;
}
@@ -1099,12 +1093,6 @@ bool TokenAnnotator::canBreakBefore(cons
// change the "binding" behavior of a comment.
return false;
- // FIXME: We can probably remove this special case once we have implemented
- // breaking after types in general.
- if (Line.First.is(tok::kw_for) && !Right.Children.empty() &&
- Right.Children[0].is(tok::equal))
- return true;
-
// Allow breaking after a trailing 'const', e.g. after a method declaration,
// unless it is follow by ';', '{' or '='.
if (Left.is(tok::kw_const) && Left.Parent != NULL &&
Modified: cfe/trunk/lib/Format/TokenAnnotator.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.h?rev=175999&r1=175998&r2=175999&view=diff
==============================================================================
--- cfe/trunk/lib/Format/TokenAnnotator.h (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.h Sun Feb 24 12:54:32 2013
@@ -140,7 +140,8 @@ public:
AnnotatedLine(const UnwrappedLine &Line)
: First(Line.Tokens.front()), Level(Line.Level),
InPPDirective(Line.InPPDirective),
- MustBeDeclaration(Line.MustBeDeclaration) {
+ MustBeDeclaration(Line.MustBeDeclaration),
+ MightBeFunctionDecl(false) {
assert(!Line.Tokens.empty());
AnnotatedToken *Current = &First;
for (std::list<FormatToken>::const_iterator I = ++Line.Tokens.begin(),
@@ -155,7 +156,8 @@ public:
AnnotatedLine(const AnnotatedLine &Other)
: First(Other.First), Type(Other.Type), Level(Other.Level),
InPPDirective(Other.InPPDirective),
- MustBeDeclaration(Other.MustBeDeclaration) {
+ MustBeDeclaration(Other.MustBeDeclaration),
+ MightBeFunctionDecl(Other.MightBeFunctionDecl) {
Last = &First;
while (!Last->Children.empty()) {
Last->Children[0].Parent = Last;
@@ -170,6 +172,7 @@ public:
unsigned Level;
bool InPPDirective;
bool MustBeDeclaration;
+ bool MightBeFunctionDecl;
};
inline prec::Level getPrecedence(const AnnotatedToken &Tok) {
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=175999&r1=175998&r2=175999&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Sun Feb 24 12:54:32 2013
@@ -1931,13 +1931,24 @@ TEST_F(FormatTest, FormatsFunctionTypes)
verifyFormat("int(*func)(void *);");
}
-TEST_F(FormatTest, BreaksFunctionDeclarations) {
+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) {}");
+ verifyFormat("ReallyLongReturnType<TemplateParam1, TemplateParam2>\n"
+ "ReallyReallyLongFunctionName(\n"
+ " const std::string &SomeParameter,\n"
+ " const SomeType<string, SomeOtherTemplateParameter> &\n"
+ " ReallyReallyLongParameterName,\n"
+ " const SomeType<string, SomeOtherTemplateParameter> &\n"
+ " AnotherLongParameterName) {}");
+ verifyFormat(
+ "aaaaaaaaaaaaaaaa::aaaaaaaaaaaaaaaa<aaaaaaaaaaaaa, aaaaaaaaaaaa>\n"
+ "aaaaaaaaaaaaaaaaaaaaaaa;");
+
verifyGoogleFormat(
"TypeSpecDecl* TypeSpecDecl::Create(\n"
" ASTContext& C, DeclContext* DC, SourceLocation L) {}");
More information about the cfe-commits
mailing list