From: Daniel Jasper Date: Wed, 3 Apr 2013 13:36:17 +0000 (+0000) Subject: Improve formatting of for loops and multi-variable DeclStmts. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=8ed9f2b25f082a1643ab5310f9eec33cf31a7577;p=clang Improve formatting of for loops and multi-variable DeclStmts. This combines several related changes: a) Don't break before after the variable types in for loops with a single variable. b) Better indent DeclStmts defining multiple variables. Before: bool aaaaaaaaaaaaaaaaaaaaaaaaa = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaa), bbbbbbbbbbbbbbbbbbbbbbbbb = bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb(bbbbbbbbbbbbbbbb); for (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa aaaaaaaaaaa = aaaaaaaaaaaaaaaa.aaaaaaaaaaaaaaa; aaaaaaaaaaa != aaaaaaaaaaaaaaaaaaa; ++aaaaaaaaaaa) { } After: bool aaaaaaaaaaaaaaaaaaaaaaaaa = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaa), bbbbbbbbbbbbbbbbbbbbbbbbb = bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb(bbbbbbbbbbbbbbbb); for (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa aaaaaaaaaaa = aaaaaaaaaaaaaaaa.aaaaaaaaaaaaaaa; aaaaaaaaaaa != aaaaaaaaaaaaaaaaaaa; ++aaaaaaaaaaa) { } git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@178641 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Format/Format.cpp b/lib/Format/Format.cpp index 4d9a228f8c..1e0178e14e 100644 --- a/lib/Format/Format.cpp +++ b/lib/Format/Format.cpp @@ -487,7 +487,6 @@ public: State.Stack.push_back( ParenState(FirstIndent, FirstIndent, !Style.BinPackParameters, /*HasMultiParameterLine=*/ false)); - State.VariablePos = 0; State.LineContainsContinuedForLoopSection = false; State.ParenLevel = 0; State.StartOfStringLiteral = 0; @@ -537,7 +536,7 @@ private: AvoidBinPacking(AvoidBinPacking), BreakBeforeParameter(false), HasMultiParameterLine(HasMultiParameterLine), ColonPos(0), StartOfFunctionCall(0), NestedNameSpecifierContinuation(0), - CallContinuation(0) {} + CallContinuation(0), VariablePos(0) {} /// \brief The position to which a specific parenthesis level needs to be /// indented. @@ -591,6 +590,11 @@ private: /// contains the start column of the second line. Otherwise 0. unsigned CallContinuation; + /// \brief The column of the first variable name in a variable declaration. + /// + /// Used to align further variables if necessary. + unsigned VariablePos; + bool operator<(const ParenState &Other) const { if (Indent != Other.Indent) return Indent < Other.Indent; @@ -618,6 +622,8 @@ private: Other.NestedNameSpecifierContinuation; if (CallContinuation != Other.CallContinuation) return CallContinuation < Other.CallContinuation; + if (VariablePos != Other.VariablePos) + return VariablePos < Other.VariablePos; return false; } }; @@ -632,11 +638,6 @@ private: /// \brief The token that needs to be next formatted. const AnnotatedToken *NextToken; - /// \brief The column of the first variable name in a variable declaration. - /// - /// Used to align further variables if necessary. - unsigned VariablePos; - /// \brief \c true if this line contains a continued for-loop section. bool LineContainsContinuedForLoopSection; @@ -660,8 +661,6 @@ private: return NextToken < Other.NextToken; if (Column != Other.Column) return Column < Other.Column; - if (VariablePos != Other.VariablePos) - return VariablePos < Other.VariablePos; if (LineContainsContinuedForLoopSection != Other.LineContainsContinuedForLoopSection) return LineContainsContinuedForLoopSection; @@ -727,10 +726,9 @@ private: } } else if (Current.Type == TT_ConditionalExpr) { State.Column = State.Stack.back().QuestionColumn; - } else if (Previous.is(tok::comma) && State.VariablePos != 0 && - ((RootToken.is(tok::kw_for) && State.ParenLevel == 1) || - State.ParenLevel == 0)) { - State.Column = State.VariablePos; + } else if (Previous.is(tok::comma) && + State.Stack.back().VariablePos != 0) { + State.Column = State.Stack.back().VariablePos; } else if (Previous.ClosesTemplateDeclaration || (Current.Type == TT_StartOfName && State.ParenLevel == 0)) { State.Column = State.Stack.back().Indent; @@ -799,10 +797,13 @@ private: State.Stack.back().BreakBeforeParameter = true; } } else { - // FIXME: Put VariablePos into ParenState and remove second part of if(). if (Current.is(tok::equal) && - (RootToken.is(tok::kw_for) || State.ParenLevel == 0)) - State.VariablePos = State.Column - Previous.FormatTok.TokenLength; + (RootToken.is(tok::kw_for) || State.ParenLevel == 0)) { + State.Stack.back().VariablePos = + State.Column - Previous.FormatTok.TokenLength; + if (Previous.PartOfMultiVariableDeclStmt) + State.Stack.back().LastSpace = State.Stack.back().VariablePos; + } unsigned Spaces = State.NextToken->SpacesRequiredBefore; @@ -828,7 +829,7 @@ private: State.Stack.back().HasMultiParameterLine = true; State.Column += Spaces; - if (Current.is(tok::l_paren) && Previous.is(tok::kw_if)) + if (Current.is(tok::l_paren) && Previous.isOneOf(tok::kw_if, tok::kw_for)) // Treat the condition inside an if as if it was a second function // parameter, i.e. let nested calls have an indent of 4. State.Stack.back().LastSpace = State.Column + 1; // 1 is length of "(". @@ -926,9 +927,11 @@ private: } // Remove scopes created by fake parenthesis. + unsigned VariablePos = State.Stack.back().VariablePos; for (unsigned i = 0, e = Current.FakeRParens; i != e; ++i) { State.Stack.pop_back(); } + State.Stack.back().VariablePos = VariablePos; if (Current.is(tok::string_literal)) { State.StartOfStringLiteral = State.Column; diff --git a/lib/Format/TokenAnnotator.cpp b/lib/Format/TokenAnnotator.cpp index 23c5a46355..427157e332 100644 --- a/lib/Format/TokenAnnotator.cpp +++ b/lib/Format/TokenAnnotator.cpp @@ -408,6 +408,10 @@ private: Tok->FormatTok.Tok.getIdentifierInfo() == &Ident_in) Tok->Type = TT_ObjCForIn; break; + case tok::comma: + if (Contexts.back().FirstStartOfName) + Contexts.back().FirstStartOfName->PartOfMultiVariableDeclStmt = true; + break; default: break; } @@ -538,7 +542,8 @@ private: : ContextKind(ContextKind), BindingStrength(BindingStrength), LongestObjCSelectorName(0), ColonIsForRangeExpr(false), ColonIsObjCMethodExpr(false), FirstObjCSelectorName(NULL), - IsExpression(IsExpression), CanBeExpression(true) {} + FirstStartOfName(NULL), IsExpression(IsExpression), + CanBeExpression(true) {} tok::TokenKind ContextKind; unsigned BindingStrength; @@ -546,6 +551,7 @@ private: bool ColonIsForRangeExpr; bool ColonIsObjCMethodExpr; AnnotatedToken *FirstObjCSelectorName; + AnnotatedToken *FirstStartOfName; bool IsExpression; bool CanBeExpression; }; @@ -600,8 +606,10 @@ private: ((Current.Parent->is(tok::identifier) && Current.Parent->FormatTok.Tok.getIdentifierInfo() ->getPPKeywordID() == tok::pp_not_keyword) || + isSimpleTypeSpecifier(*Current.Parent) || Current.Parent->Type == TT_PointerOrReference || Current.Parent->Type == TT_TemplateCloser)) { + Contexts.back().FirstStartOfName = &Current; Current.Type = TT_StartOfName; } else if (Current.isOneOf(tok::star, tok::amp, tok::ampamp)) { Current.Type = @@ -720,6 +728,39 @@ private: return TT_UnaryOperator; } + // FIXME: This is copy&pasted from Sema. Put it in a common place and remove + // duplication. + /// \brief Determine whether the token kind starts a simple-type-specifier. + bool isSimpleTypeSpecifier(const AnnotatedToken &Tok) const { + switch (Tok.FormatTok.Tok.getKind()) { + case tok::kw_short: + case tok::kw_long: + case tok::kw___int64: + case tok::kw___int128: + case tok::kw_signed: + case tok::kw_unsigned: + case tok::kw_void: + case tok::kw_char: + case tok::kw_int: + case tok::kw_half: + case tok::kw_float: + case tok::kw_double: + case tok::kw_wchar_t: + case tok::kw_bool: + case tok::kw___underlying_type: + return true; + case tok::annot_typename: + case tok::kw_char16_t: + case tok::kw_char32_t: + case tok::kw_typeof: + case tok::kw_decltype: + return Lex.getLangOpts().CPlusPlus; + default: + break; + } + return false; + } + SmallVector Contexts; SourceManager &SourceMgr; @@ -886,7 +927,7 @@ unsigned TokenAnnotator::splitPenalty(const AnnotatedLine &Line, const AnnotatedToken &Right = Tok; if (Right.Type == TT_StartOfName) { - if (Line.First.is(tok::kw_for)) + if (Line.First.is(tok::kw_for) && Right.PartOfMultiVariableDeclStmt) return 3; else if (Line.MightBeFunctionDecl && Right.BindingStrength == 1) // FIXME: Clean up hack of using BindingStrength to find top-level names. diff --git a/lib/Format/TokenAnnotator.h b/lib/Format/TokenAnnotator.h index 013dd2d27c..c41ee33c43 100644 --- a/lib/Format/TokenAnnotator.h +++ b/lib/Format/TokenAnnotator.h @@ -76,8 +76,8 @@ public: ClosesTemplateDeclaration(false), MatchingParen(NULL), ParameterCount(0), BindingStrength(0), SplitPenalty(0), LongestObjCSelectorName(0), Parent(NULL), FakeLParens(0), - FakeRParens(0), LastInChainOfCalls(false) { - } + FakeRParens(0), LastInChainOfCalls(false), + PartOfMultiVariableDeclStmt(false) {} bool is(tok::TokenKind Kind) const { return FormatTok.Tok.is(Kind); } @@ -166,6 +166,11 @@ public: /// \brief Is this the last "." or "->" in a builder-type call? bool LastInChainOfCalls; + /// \brief Is this token part of a \c DeclStmt defining multiple variables? + /// + /// Only set if \c Type == \c TT_StartOfName. + bool PartOfMultiVariableDeclStmt; + const AnnotatedToken *getPreviousNoneComment() const { AnnotatedToken *Tok = Parent; while (Tok != NULL && Tok->is(tok::comment)) diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp index 92a2363bbd..2038ee167f 100644 --- a/unittests/Format/FormatTest.cpp +++ b/unittests/Format/FormatTest.cpp @@ -311,8 +311,8 @@ TEST_F(FormatTest, FormatsForLoop) { verifyFormat( "for (MachineFun::iterator IIII = PrevIt, EEEE = F.end(); IIII != EEEE;\n" " ++IIIII) {\n}"); - verifyFormat("for (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" - " aaaaaaaaaaa = aaaaaaaaaaaaaaaa.aaaaaaaaaaaaaaa;\n" + verifyFormat("for (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa aaaaaaaaaaa =\n" + " aaaaaaaaaaaaaaaa.aaaaaaaaaaaaaaa;\n" " aaaaaaaaaaa != aaaaaaaaaaaaaaaaaaa; ++aaaaaaaaaaa) {\n}"); verifyFormat("for (llvm::ArrayRef::iterator\n" " I = FD->getDeclsInPrototypeScope().begin(),\n" @@ -329,6 +329,10 @@ TEST_F(FormatTest, FormatsForLoop) { verifyFormat("for (int aaaaaaaaaaa = 1; aaaaaaaaaaa <= bbbbbbbbbbbbbbb;\n" " aaaaaaaaaaa++, bbbbbbbbbbbbbbbbb++) {\n" "}"); + verifyFormat("for (some_namespace::SomeIterator iter( // force break\n" + " aaaaaaaaaa);\n" + " iter; ++iter) {\n" + "}"); FormatStyle NoBinPacking = getLLVMStyle(); NoBinPacking.BinPackParameters = false; @@ -1188,8 +1192,9 @@ TEST_F(FormatTest, FormatsSmallMacroDefinitionsInSingleLine) { } TEST_F(FormatTest, DoesNotBreakPureVirtualFunctionDefinition) { - verifyFormat("virtual void write(ELFWriter *writerrr,\n" - " OwningPtr &buffer) = 0;"); + verifyFormat( + "virtual void\n" + "write(ELFWriter *writerrr, OwningPtr &buffer) = 0;"); } TEST_F(FormatTest, LayoutUnknownPPDirective) { @@ -1378,7 +1383,7 @@ TEST_F(FormatTest, MixingPreprocessorDirectivesAndNormalCode) { TEST_F(FormatTest, LayoutStatementsAroundPreprocessorDirectives) { EXPECT_EQ("int\n" "#define A\n" - " a;", + "a;", format("int\n#define A\na;")); verifyFormat("functionCallTo(\n" " someOtherFunction(\n" @@ -1613,7 +1618,7 @@ TEST_F(FormatTest, BreaksDesireably) { } TEST_F(FormatTest, FormatsOneParameterPerLineIfNecessary) { - FormatStyle NoBinPacking = getLLVMStyle(); + FormatStyle NoBinPacking = getGoogleStyle(); NoBinPacking.BinPackParameters = false; verifyFormat("f(aaaaaaaaaaaaaaaaaaaa,\n" " aaaaaaaaaaaaaaaaaaaa,\n" @@ -1851,14 +1856,13 @@ TEST_F(FormatTest, DeclarationsOfMultipleVariables) { " aaaaaaaaaaa = aaaaaa->aaaaaaaaaaa();"); verifyFormat("bool a = true, b = false;"); - // FIXME: Indentation looks weird. verifyFormat("bool aaaaaaaaaaaaaaaaaaaaaaaaa =\n" - " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaa),\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaa),\n" " bbbbbbbbbbbbbbbbbbbbbbbbb =\n" " bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb(bbbbbbbbbbbbbbbb);"); verifyFormat( "bool aaaaaaaaaaaaaaaaaaaaa =\n" - " bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb && cccccccccccccccccccccccccccc,\n" + " bbbbbbbbbbbbbbbbbbbbbbbbbbbb && cccccccccccccccccccccccccccc,\n" " d = e && f;"); } @@ -2051,7 +2055,7 @@ TEST_F(FormatTest, WrapsTemplateDeclarations) { " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);"); verifyFormat("template \n" "void aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n" - " int aaaaaaaaaaaaaaaaa);"); + " int aaaaaaaaaaaaaaaaaaaaaa);"); verifyFormat( "template \n"