From 26d1b1db23babd59003f7c79219894b07fac32bb Mon Sep 17 00:00:00 2001 From: Daniel Jasper Date: Sun, 24 Feb 2013 18:54:32 +0000 Subject: [PATCH] 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 ReallyReallyLongFunctionName( const std::string &SomeParameter, const SomeType &ReallyReallyLongParameterName, const SomeType &AnotherLongParameterName) {} After: ReallyLongReturnType ReallyReallyLongFunctionName( const std::string &SomeParameter, const SomeType & ReallyReallyLongParameterName, const SomeType & AnotherLongParameterName) {} llvm-svn: 175999 --- clang/lib/Format/Format.cpp | 16 ++++------- clang/lib/Format/TokenAnnotator.cpp | 52 ++++++++++++++--------------------- clang/lib/Format/TokenAnnotator.h | 7 +++-- clang/unittests/Format/FormatTest.cpp | 13 ++++++++- 4 files changed, 42 insertions(+), 46 deletions(-) diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index d8784c1..60fe957 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -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; diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp index b7f43d5..86daa8d 100644 --- a/clang/lib/Format/TokenAnnotator.cpp +++ b/clang/lib/Format/TokenAnnotator.cpp @@ -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(const AnnotatedLine &Line, 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(const AnnotatedLine &Line, if (Level != prec::Unknown) return Level; - + return 3; } @@ -1099,12 +1093,6 @@ bool TokenAnnotator::canBreakBefore(const AnnotatedLine &Line, // 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 && diff --git a/clang/lib/Format/TokenAnnotator.h b/clang/lib/Format/TokenAnnotator.h index 850b6dd..aa78779 100644 --- a/clang/lib/Format/TokenAnnotator.h +++ b/clang/lib/Format/TokenAnnotator.h @@ -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::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) { diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 2f8097c..94ac4cf 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -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\n" + "ReallyReallyLongFunctionName(\n" + " const std::string &SomeParameter,\n" + " const SomeType &\n" + " ReallyReallyLongParameterName,\n" + " const SomeType &\n" + " AnotherLongParameterName) {}"); + verifyFormat( + "aaaaaaaaaaaaaaaa::aaaaaaaaaaaaaaaa\n" + "aaaaaaaaaaaaaaaaaaaaaaa;"); + verifyGoogleFormat( "TypeSpecDecl* TypeSpecDecl::Create(\n" " ASTContext& C, DeclContext* DC, SourceLocation L) {}"); -- 2.7.4