r175999 - Allow breaking between a type and name in variable declarations.
David Blaikie
dblaikie at gmail.com
Sun Feb 24 11:19:59 PST 2013
On Sun, Feb 24, 2013 at 11:15 AM, Daniel Jasper <djasper at google.com> wrote:
> Right, but it is consistent with the splitting before a function name,
> which is exclusively done after & and * in LLVM/Clang.
>
Curious/fair point.
>
>
> 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/23f31264/attachment.html>
More information about the cfe-commits
mailing list