From 6bee682fae8f7d95a76eb47ea617a3e53673747d Mon Sep 17 00:00:00 2001 From: Daniel Jasper Date: Mon, 8 Apr 2013 20:33:42 +0000 Subject: [PATCH] Revamp indentation behavior for complex binary expressions. The idea is to indent according to operator precedence and pretty much identical to how stuff would be indented with parenthesis. Before: bool value = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa == aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa * bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb && aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa * aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa > ccccccccccccccccccccccccccccccccccccccccc; After: bool value = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa == aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa * bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb && aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa * aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa > ccccccccccccccccccccccccccccccccccccccccc; llvm-svn: 179049 --- clang/lib/Format/Format.cpp | 59 +++++++++++++++++++++-------------- clang/lib/Format/TokenAnnotator.cpp | 43 +++++++++++++++---------- clang/lib/Format/TokenAnnotator.h | 17 ++++++++-- clang/unittests/Format/FormatTest.cpp | 43 ++++++++++++++++++++----- 4 files changed, 112 insertions(+), 50 deletions(-) diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index 101b16f..c78b5b6 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -86,12 +86,6 @@ static bool isTrailingComment(const AnnotatedToken &Tok) { (Tok.Children.empty() || Tok.Children[0].MustBreakBefore); } -static bool isComparison(const AnnotatedToken &Tok) { - prec::Level Precedence = getPrecedence(Tok); - return Tok.Type == TT_BinaryOperator && - (Precedence == prec::Equality || Precedence == prec::Relational); -} - // Returns the length of everything up to the first possible line break after // the ), ], } or > matching \c Tok. static unsigned getLengthToMatchingParen(const AnnotatedToken &Tok) { @@ -492,10 +486,6 @@ public: State.StartOfStringLiteral = 0; State.StartOfLineLevel = State.ParenLevel; - DEBUG({ - DebugTokenState(*State.NextToken); - }); - // The first token has already been indented and thus consumed. moveStateToNextToken(State, /*DryRun=*/ false); @@ -741,8 +731,7 @@ private: State.Stack.back().ColonPos = State.Column + Current.FormatTok.TokenLength; } - } else if (Current.Type == TT_StartOfName || Current.is(tok::question) || - Previous.is(tok::equal) || isComparison(Previous) || + } else if (Current.Type == TT_StartOfName || Previous.is(tok::equal) || Previous.Type == TT_ObjCMethodExpr) { State.Column = ContinuationIndent; } else { @@ -830,9 +819,8 @@ private: State.Column + Spaces + Current.FormatTok.TokenLength; } - if (Current.Type != TT_LineComment && - (Previous.isOneOf(tok::l_paren, tok::l_brace) || - State.NextToken->Parent->Type == TT_TemplateOpener)) + if (opensScope(Previous) && Previous.Type != TT_ObjCMethodExpr && + Current.Type != TT_LineComment) State.Stack.back().Indent = State.Column + Spaces; if (Previous.is(tok::comma) && !isTrailingComment(Current)) State.Stack.back().HasMultiParameterLine = true; @@ -851,9 +839,7 @@ private: State.Stack.back().LastSpace = State.Column; else if (Previous.Type == TT_InheritanceColon) State.Stack.back().Indent = State.Column; - else if (Previous.ParameterCount > 1 && - (Previous.isOneOf(tok::l_paren, tok::l_square, tok::l_brace) || - Previous.Type == TT_TemplateOpener)) + else if (opensScope(Previous) && Previous.ParameterCount > 1) // If this function has multiple parameters, indent nested calls from // the start of the first parameter. State.Stack.back().LastSpace = State.Column; @@ -879,28 +865,55 @@ private: State.Stack.back().StartOfFunctionCall = Current.LastInChainOfCalls ? 0 : State.Column; if (Current.Type == TT_CtorInitializerColon) { + State.Stack.back().Indent = State.Column + 2; if (Style.ConstructorInitializerAllOnOneLineOrOnePerLine) State.Stack.back().AvoidBinPacking = true; State.Stack.back().BreakBeforeParameter = false; } + // If return returns a binary expression, align after it. + if (Current.is(tok::kw_return) && !Current.FakeLParens.empty()) + State.Stack.back().LastSpace = State.Column + 7; + // In ObjC method declaration we align on the ":" of parameters, but we need // to ensure that we indent parameters on subsequent lines by at least 4. if (Current.Type == TT_ObjCMethodSpecifier) State.Stack.back().Indent += 4; // Insert scopes created by fake parenthesis. - for (unsigned i = 0, e = Current.FakeLParens; i != e; ++i) { + const AnnotatedToken *Previous = Current.getPreviousNoneComment(); + // Don't add extra indentation for the first fake parenthesis after + // 'return', assignements or opening <({[. The indentation for these cases + // is special cased. + bool SkipFirstExtraIndent = + Current.is(tok::kw_return) || + (Previous && (opensScope(*Previous) || + getPrecedence(*Previous) == prec::Assignment)); + for (SmallVector::const_reverse_iterator + I = Current.FakeLParens.rbegin(), + E = Current.FakeLParens.rend(); + I != E; ++I) { ParenState NewParenState = State.Stack.back(); - NewParenState.Indent = std::max(State.Column, State.Stack.back().Indent); - NewParenState.BreakBeforeParameter = false; + NewParenState.Indent = + std::max(std::max(State.Column, NewParenState.Indent), + State.Stack.back().LastSpace); + + // Always indent conditional expressions. Never indent expression where + // the 'operator' is ',', ';' or an assignment (i.e. *I <= + // prec::Assignment) as those have different indentation rules. Indent + // other expression, unless the indentation needs to be skipped. + if (*I == prec::Conditional || + (!SkipFirstExtraIndent && *I > prec::Assignment)) + NewParenState.Indent += 4; + if (Previous && !opensScope(*Previous)) + NewParenState.BreakBeforeParameter = false; State.Stack.push_back(NewParenState); + SkipFirstExtraIndent = false; } // If we encounter an opening (, [, { or <, we add a level to our stacks to // prepare for the following tokens. - if (Current.isOneOf(tok::l_paren, tok::l_square, tok::l_brace) || - State.NextToken->Type == TT_TemplateOpener) { + if (opensScope(Current)) { unsigned NewIndent; bool AvoidBinPacking; if (Current.is(tok::l_brace)) { diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp index 44421c4..392a208 100644 --- a/clang/lib/Format/TokenAnnotator.cpp +++ b/clang/lib/Format/TokenAnnotator.cpp @@ -16,6 +16,7 @@ #include "TokenAnnotator.h" #include "clang/Basic/SourceManager.h" #include "clang/Lex/Lexer.h" +#include "llvm/Support/Debug.h" namespace clang { namespace format { @@ -71,12 +72,12 @@ static const AnnotatedToken *getNextToken(const AnnotatedToken &Tok) { return NextToken; } -static bool closesScope(const AnnotatedToken &Tok) { +bool closesScope(const AnnotatedToken &Tok) { return Tok.isOneOf(tok::r_paren, tok::r_brace, tok::r_square) || Tok.Type == TT_TemplateCloser; } -static bool opensScope(const AnnotatedToken &Tok) { +bool opensScope(const AnnotatedToken &Tok) { return Tok.isOneOf(tok::l_paren, tok::l_brace, tok::l_square) || Tok.Type == TT_TemplateOpener; } @@ -782,10 +783,6 @@ public: if (Precedence > prec::PointerToMember || Current == NULL) return; - // Skip over "return" until we can properly parse it. - if (Current->is(tok::kw_return)) - next(); - // Eagerly consume trailing comments. while (isTrailingComment(Current)) { next(); @@ -796,14 +793,13 @@ public: while (Current) { // Consume operators with higher precedence. - parse(prec::Level(Precedence + 1)); + parse(Precedence + 1); int CurrentPrecedence = 0; if (Current) { if (Current->Type == TT_ConditionalExpr) CurrentPrecedence = 1 + (int) prec::Conditional; - else if (Current->is(tok::semi) || Current->Type == TT_InlineASMColon || - Current->Type == TT_CtorInitializerColon) + else if (Current->is(tok::semi) || Current->Type == TT_InlineASMColon) CurrentPrecedence = 1; else if (Current->Type == TT_BinaryOperator || Current->is(tok::comma)) CurrentPrecedence = 1 + (int) getPrecedence(*Current); @@ -814,7 +810,7 @@ public: if (Current == NULL || closesScope(*Current) || (CurrentPrecedence != 0 && CurrentPrecedence < Precedence)) { if (OperatorFound) { - ++Start->FakeLParens; + Start->FakeLParens.push_back(prec::Level(Precedence - 1)); if (Current) ++Current->Parent->FakeRParens; } @@ -823,17 +819,10 @@ public: // Consume scopes: (), [], <> and {} if (opensScope(*Current)) { - AnnotatedToken *Left = Current; while (Current && !closesScope(*Current)) { next(); parse(); } - // Remove fake parens that just duplicate the real parens. - if (Current && Left->Children[0].FakeLParens > 0 && - Current->Parent->FakeRParens > 0) { - --Left->Children[0].FakeLParens; - --Current->Parent->FakeRParens; - } next(); } else { // Operator found. @@ -919,6 +908,10 @@ void TokenAnnotator::calculateFormattingInformation(AnnotatedLine &Line) { Current = Current->Children.empty() ? NULL : &Current->Children[0]; } + + DEBUG({ + printDebugInfo(Line); + }); } unsigned TokenAnnotator::splitPenalty(const AnnotatedLine &Line, @@ -1187,5 +1180,21 @@ bool TokenAnnotator::canBreakBefore(const AnnotatedLine &Line, (Left.is(tok::l_square) && !Right.is(tok::r_square)); } +void TokenAnnotator::printDebugInfo(const AnnotatedLine &Line) { + llvm::errs() << "AnnotatedTokens:\n"; + const AnnotatedToken *Tok = &Line.First; + while (Tok) { + llvm::errs() << " M=" << Tok->MustBreakBefore + << " C=" << Tok->CanBreakBefore << " T=" << Tok->Type + << " S=" << Tok->SpacesRequiredBefore + << " Name=" << Tok->FormatTok.Tok.getName() << " FakeLParens="; + for (unsigned i = 0, e = Tok->FakeLParens.size(); i != e; ++i) + llvm::errs() << Tok->FakeLParens[i] << "/"; + llvm::errs() << " FakeRParens=" << Tok->FakeRParens << "\n"; + Tok = Tok->Children.empty() ? NULL : &Tok->Children[0]; + } + llvm::errs() << "----\n"; +} + } // namespace format } // namespace clang diff --git a/clang/lib/Format/TokenAnnotator.h b/clang/lib/Format/TokenAnnotator.h index c41ee33..f5a8a94 100644 --- a/clang/lib/Format/TokenAnnotator.h +++ b/clang/lib/Format/TokenAnnotator.h @@ -75,7 +75,7 @@ public: CanBreakBefore(false), MustBreakBefore(false), ClosesTemplateDeclaration(false), MatchingParen(NULL), ParameterCount(0), BindingStrength(0), SplitPenalty(0), - LongestObjCSelectorName(0), Parent(NULL), FakeLParens(0), + LongestObjCSelectorName(0), Parent(NULL), FakeRParens(0), LastInChainOfCalls(false), PartOfMultiVariableDeclStmt(false) {} @@ -158,8 +158,12 @@ public: std::vector Children; AnnotatedToken *Parent; - /// \brief Insert this many fake ( before this token for correct indentation. - unsigned FakeLParens; + /// \brief Stores the number of required fake parentheses and the + /// corresponding operator precedence. + /// + /// If multiple fake parentheses start at a token, this vector stores them in + /// reverse order, i.e. inner fake parenthesis first. + SmallVector FakeLParens; /// \brief Insert this many fake ) after this token for correct indentation. unsigned FakeRParens; @@ -223,6 +227,11 @@ inline prec::Level getPrecedence(const AnnotatedToken &Tok) { return getBinOpPrecedence(Tok.FormatTok.Tok.getKind(), true, true); } +/// \brief Returns whether \p Tok is ([{ or a template opening <. +bool opensScope(const AnnotatedToken &Tok); +/// \brief Returns whether \p Tok is )]} or a template opening >. +bool closesScope(const AnnotatedToken &Tok); + /// \brief Determines extra information about the tokens comprising an /// \c UnwrappedLine. class TokenAnnotator { @@ -248,6 +257,8 @@ private: bool canBreakBefore(const AnnotatedLine &Line, const AnnotatedToken &Right); + void printDebugInfo(const AnnotatedLine &Line); + const FormatStyle &Style; SourceManager &SourceMgr; Lexer &Lex; diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 0ef241e..d5995b2 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -1473,6 +1473,30 @@ TEST_F(FormatTest, PreventConfusingIndents) { " ddd);"); } +TEST_F(FormatTest, ExpressionIndentation) { + verifyFormat("bool value = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa ==\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa *\n" + " bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb +\n" + " bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb &&\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa *\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa >\n" + " ccccccccccccccccccccccccccccccccccccccccc;"); + verifyFormat("if (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa *\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa ==\n" + " bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb) {\n}"); + verifyFormat("if (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa *\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa ==\n" + " bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb) {\n}"); + verifyFormat("if (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa ==\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa *\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +\n" + " bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb) {\n}"); +} + TEST_F(FormatTest, ConstructorInitializers) { verifyFormat("Constructor() : Initializer(FitsOnTheLine) {}"); verifyFormat("Constructor() : Inttializer(FitsOnTheLine) {}", @@ -1562,7 +1586,7 @@ TEST_F(FormatTest, BreaksAsHighAsPossible) { " f();\n" "}"); verifyFormat("if (Intervals[i].getRange().getFirst() <\n" - " Intervals[i - 1].getRange().getLast()) {\n}"); + " Intervals[i - 1].getRange().getLast()) {\n}"); } TEST_F(FormatTest, BreaksDesireably) { @@ -1685,9 +1709,9 @@ TEST_F(FormatTest, FormatsBuilderPattern) { " .StartsWith(\".eh_frame\", ORDER_EH_FRAME).StartsWith(\".init\", ORDER_INIT)\n" " .StartsWith(\".fini\", ORDER_FINI).StartsWith(\".hash\", ORDER_HASH)\n" " .Default(ORDER_TEXT);\n"); - + verifyFormat("return aaaaaaaaaaaaaaaaa->aaaaa().aaaaaaaaaaaaa().aaaaaa() <\n" - " aaaaaaaaaaaaaaa->aaaaa().aaaaaaaaaaaaa().aaaaaa();"); + " aaaaaaaaaaaaaaa->aaaaa().aaaaaaaaaaaaa().aaaaaa();"); verifyFormat( "aaaaaaa->aaaaaaa\n" " ->aaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa)\n" @@ -1784,10 +1808,15 @@ TEST_F(FormatTest, AlignsAfterReturn) { " aaaaaaaaaaaaaaaaaaaaaaaaa);"); verifyFormat( "return aaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa >=\n" - " aaaaaaaaaaaaaaaaaaaaaa();"); + " aaaaaaaaaaaaaaaaaaaaaa();"); verifyFormat( "return (aaaaaaaaaaaaaaaaaaaaaa + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa >=\n" - " aaaaaaaaaaaaaaaaaaaaaa());"); + " aaaaaaaaaaaaaaaaaaaaaa());"); + verifyFormat("return aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);"); + verifyFormat("return aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) &&\n" + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa;"); } TEST_F(FormatTest, BreaksConditionalExpressions) { @@ -1831,7 +1860,7 @@ TEST_F(FormatTest, BreaksConditionalExpressions) { " ? aaaaaaaaaaaaaaa\n" " : aaaaaaaaaaaaaaa;"); verifyFormat("f(aaaaaaaaaaaaaaaa == // force break\n" - " aaaaaaaaa\n" + " aaaaaaaaa\n" " ? b\n" " : c);"); verifyFormat( @@ -2398,7 +2427,7 @@ TEST_F(FormatTest, UnderstandsRvalueReferences) { TEST_F(FormatTest, FormatsBinaryOperatorsPrecedingEquals) { verifyFormat("void f() {\n" " x[aaaaaaaaa -\n" - " b] = 23;\n" + " b] = 23;\n" "}", getLLVMStyleWithColumns(15)); } -- 2.7.4