On Tue, Apr 2, 2013 at 4:33 PM, Daniel Jasper <[email protected]> wrote:
> Author: djasper > Date: Tue Apr 2 09:33:13 2013 > New Revision: 178542 > > URL: http://llvm.org/viewvc/llvm-project?rev=178542&view=rev > Log: > Fix some inconsistent use of indentation. > > Basically we have always special-cased the top-level statement of an > unwrapped line (the one with ParenLevel == 0) and that lead to several > inconsistencies. All added tests were formatted in a strange way, for > example: > > Before: > aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa > .aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa > .aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa > .aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(); > if (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa > .aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa > .aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa > .aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa()) { > } > > After: > aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa > .aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa > .aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa > .aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(); > if (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa > .aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa > .aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa > .aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa()) { > } > > Modified: > cfe/trunk/lib/Format/Format.cpp > 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=178542&r1=178541&r2=178542&view=diff > > ============================================================================== > --- cfe/trunk/lib/Format/Format.cpp (original) > +++ cfe/trunk/lib/Format/Format.cpp Tue Apr 2 09:33:13 2013 > @@ -485,7 +485,7 @@ public: > State.Column = FirstIndent; > State.NextToken = &RootToken; > State.Stack.push_back( > - ParenState(FirstIndent + 4, FirstIndent, !Style.BinPackParameters, > + ParenState(FirstIndent, FirstIndent, !Style.BinPackParameters, > /*HasMultiParameterLine=*/ false)); > State.VariablePos = 0; > State.LineContainsContinuedForLoopSection = false; > @@ -536,7 +536,8 @@ private: > BreakBeforeClosingBrace(false), QuestionColumn(0), > AvoidBinPacking(AvoidBinPacking), BreakBeforeParameter(false), > HasMultiParameterLine(HasMultiParameterLine), ColonPos(0), > - StartOfFunctionCall(0) {} > + StartOfFunctionCall(0), NestedNameSpecifierContinuation(0), > + CallContinuation(0) {} > > /// \brief The position to which a specific parenthesis level needs > to be > /// indented. > @@ -582,6 +583,14 @@ private: > /// \brief The start of the most recent function in a builder-type > call. > unsigned StartOfFunctionCall; > > + /// \brief If a nested name specifier was broken over multiple lines, > this > + /// contains the start column of the second line. Otherwise 0. > + unsigned NestedNameSpecifierContinuation; > + > + /// \brief If a call expression was broken over multiple lines, this > + /// contains the start column of the second line. Otherwise 0. > + unsigned CallContinuation; > + > bool operator<(const ParenState &Other) const { > if (Indent != Other.Indent) > return Indent < Other.Indent; > @@ -603,6 +612,12 @@ private: > return ColonPos < Other.ColonPos; > if (StartOfFunctionCall != Other.StartOfFunctionCall) > return StartOfFunctionCall < Other.StartOfFunctionCall; > + if (NestedNameSpecifierContinuation != > + Other.NestedNameSpecifierContinuation) > + return NestedNameSpecifierContinuation < > + Other.NestedNameSpecifierContinuation; > + if (CallContinuation != Other.CallContinuation) > + return CallContinuation < Other.CallContinuation; > return false; > } > }; > @@ -682,6 +697,8 @@ private: > return 0; > } > > + unsigned IndentedFurther = > I'd call it "ContinuationIndent" > + std::max(State.Stack.back().LastSpace, State.Stack.back().Indent) > + 4; > if (Newline) { > unsigned WhitespaceStartColumn = State.Column; > if (Current.is(tok::r_brace)) { > @@ -693,15 +710,16 @@ private: > } else if (Current.is(tok::lessless) && > State.Stack.back().FirstLessLess != 0) { > State.Column = State.Stack.back().FirstLessLess; > - } else if (State.ParenLevel != 0 && > - (Previous.isOneOf(tok::equal, tok::coloncolon) || > - Current.isOneOf(tok::period, tok::arrow, tok::question) > || > - isComparison(Previous))) { > - // Indent and extra 4 spaces after if we know the current > expression is > - // continued. Don't do that on the top level, as we already > indent 4 > - // there. > - State.Column = std::max(State.Stack.back().LastSpace, > - State.Stack.back().Indent) + 4; > + } else if (Previous.is(tok::coloncolon)) { > I think this is easy to understand for me as I saw you scribbling on the whiteboard, but I'd add a comment like // Continuation of a broken nested name specifier. > + State.Column = IndentedFurther; > + if (State.Stack.back().NestedNameSpecifierContinuation == 0) > + State.Stack.back().NestedNameSpecifierContinuation = > State.Column; > + State.Column = State.Stack.back().NestedNameSpecifierContinuation; > You really wanted to save lines here :) I'd unfold this into two trivially to understand cases. Then you also don't need the comment, as the first line I hit will say "NestedNameSpecifierContinuation". > + } else if (Current.isOneOf(tok::period, tok::arrow)) { > + State.Column = IndentedFurther; > + if (State.Stack.back().CallContinuation == 0) > + State.Stack.back().CallContinuation = State.Column; > + State.Column = State.Stack.back().CallContinuation; } else if (Current.Type == TT_ConditionalExpr) { > State.Column = State.Stack.back().QuestionColumn; > } else if (Previous.is(tok::comma) && State.VariablePos != 0 && > @@ -710,7 +728,7 @@ private: > State.Column = State.VariablePos; > } else if (Previous.ClosesTemplateDeclaration || > (Current.Type == TT_StartOfName && State.ParenLevel == > 0)) { > - State.Column = State.Stack.back().Indent - 4; > + State.Column = State.Stack.back().Indent; > } else if (Current.Type == TT_ObjCSelectorName) { > if (State.Stack.back().ColonPos > Current.FormatTok.TokenLength) { > State.Column = > @@ -720,11 +738,15 @@ private: > State.Stack.back().ColonPos = > State.Column + Current.FormatTok.TokenLength; > } > - } else if (Previous.Type == TT_ObjCMethodExpr || > - Current.Type == TT_StartOfName) { > - State.Column = State.Stack.back().Indent + 4; > + } else if (Current.Type == TT_StartOfName || > Current.is(tok::question) || > + Previous.is(tok::equal) || isComparison(Previous) || > + Previous.Type == TT_ObjCMethodExpr) { > + // Indent and extra 4 spaces if the current expression is > continued. > s/and/an/ > + State.Column = IndentedFurther; > } else { > State.Column = State.Stack.back().Indent; > + if (State.Column == FirstIndent) > Can you please explain this in a comment. > + State.Column += 4; > } > > if (Current.is(tok::question)) > @@ -845,6 +867,8 @@ private: > State.Stack.back().AvoidBinPacking = true; > State.Stack.back().BreakBeforeParameter = false; > } > + if (Current.Type == TT_ObjCMethodSpecifier) > + State.Stack.back().Indent += 4; > ... and this. > > // Insert scopes created by fake parenthesis. > for (unsigned i = 0, e = Current.FakeLParens; i != e; ++i) { > > Modified: cfe/trunk/unittests/Format/FormatTest.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=178542&r1=178541&r2=178542&view=diff > > ============================================================================== > --- cfe/trunk/unittests/Format/FormatTest.cpp (original) > +++ cfe/trunk/unittests/Format/FormatTest.cpp Tue Apr 2 09:33:13 2013 > @@ -1670,7 +1670,7 @@ TEST_F(FormatTest, FormatsBuilderPattern > " .Default(ORDER_TEXT);\n"); > > verifyFormat("return > aaaaaaaaaaaaaaaaa->aaaaa().aaaaaaaaaaaaa().aaaaaa() <\n" > - " > aaaaaaaaaaaaaaaaaaa->aaaaa().aaaaaaaaaaaaa().aaaaaa();"); > + " > aaaaaaaaaaaaaaa->aaaaa().aaaaaaaaaaaaa().aaaaaa();"); > verifyFormat( > "aaaaaaa->aaaaaaa\n" > " > ->aaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa)\n" > @@ -1765,6 +1765,12 @@ TEST_F(FormatTest, AlignsAfterReturn) { > verifyFormat( > "return (aaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaa +\n" > " aaaaaaaaaaaaaaaaaaaaaaaaa);"); > + verifyFormat( > + "return aaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa > >=\n" > + " aaaaaaaaaaaaaaaaaaaaaa();"); > + verifyFormat( > + "return (aaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa > >=\n" > + " aaaaaaaaaaaaaaaaaaaaaa());"); > } > > TEST_F(FormatTest, BreaksConditionalExpressions) { > @@ -1799,6 +1805,10 @@ TEST_F(FormatTest, BreaksConditionalExpr > verifyFormat("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" > " ? aaaaaaaaaaaaaaaaaaaaaaaaaaa\n" > " : aaaaaaaaaaaaaaaaaaaaaaaaaaa;"); > + verifyFormat("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa aaaaaa =\n" > + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" > + " ? aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" > + " : aaaaaaaaaaaaaaaa;"); > verifyFormat( > "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa == aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" > " ? aaaaaaaaaaaaaaa\n" > @@ -1837,7 +1847,7 @@ TEST_F(FormatTest, DeclarationsOfMultipl > verifyFormat("bool aaaaaaaaaaaaaaaaaaaaaaaaa =\n" > " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaa),\n" > " bbbbbbbbbbbbbbbbbbbbbbbbb =\n" > - " bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb(bbbbbbbbbbbbbbbb);"); > + " > bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb(bbbbbbbbbbbbbbbb);"); > verifyFormat( > "bool aaaaaaaaaaaaaaaaaaaaa =\n" > " bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb && > cccccccccccccccccccccccccccc,\n" > @@ -1920,6 +1930,10 @@ TEST_F(FormatTest, AlignsPipes) { > " << \"eeeeeeeeeeeeeeeee = \" << eeeeeeeeeeeeeeeee;"); > verifyFormat("llvm::outs() << aaaaaaaaaaaaaaaaaaaaaaaa << \"=\"\n" > " << bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb;"); > + > + verifyFormat( > + "llvm::errs() << aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" > + " .aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa();"); > } > > TEST_F(FormatTest, UnderstandsEquals) { > @@ -1973,6 +1987,11 @@ TEST_F(FormatTest, WrapsAtFunctionCallsI > " .aaaaaaaaaaaaaaa(\n" > " aa(aaaaaaaaaaaaaaaaaaaaaaaaaaa, > aaaaaaaaaaaaaaaaaaaaaaaaaaa,\n" > " aaaaaaaaaaaaaaaaaaaaaaaaaaa));"); > + verifyFormat("if (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" > + " .aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" > + " .aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" > + " .aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa()) {\n" > + "}"); > > // Here, it is not necessary to wrap at "." or "->". > verifyFormat("if (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaa) > ||\n" > @@ -2068,6 +2087,10 @@ TEST_F(FormatTest, WrapsAtNestedNameSpec > " > aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,\n" > " aaaaaaaaaaaaaaaaaaaaa);", > getLLVMStyleWithColumns(74)); > + > + verifyFormat("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa::\n" > + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" > + " .aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa();"); > } > > TEST_F(FormatTest, UnderstandsTemplateParameters) { > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
