r175999 - Allow breaking between a type and name in variable declarations.
Daniel Jasper
djasper at google.com
Sun Feb 24 11:15:19 PST 2013
Right, but it is consistent with the splitting before a function name,
which is exclusively done after & and * in LLVM/Clang.
On Sun, Feb 24, 2013 at 7:58 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Sun, Feb 24, 2013 at 10:54 AM, Daniel Jasper <djasper at google.com>wrote:
>
>> 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) {}
>>
>
> The placement of the '&' seems to go against the spirit of "put it with
> the variable, not the type" style.
>
>
>>
>> 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) {}");
>>
>>
>> _______________________________________________
>> 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/20130224/e8d03a94/attachment.html>
More information about the cfe-commits
mailing list