From 4355e7f0effc788f1af2b48a531f650e1e40db39 Mon Sep 17 00:00:00 2001 From: Daniel Jasper Date: Wed, 9 Jul 2014 07:50:33 +0000 Subject: [PATCH] clang-format: Revamp function declaration/definition indentation. Key changes: - Correctly (well ...) distinguish function declarations and variable declarations with ()-initialization. - Don't indent when breaking function declarations/definitions after the return type. - Indent variable declarations and typedefs when breaking after the type. This fixes llvm.org/PR17999. llvm-svn: 212591 --- clang/include/clang/Format/Format.h | 6 --- clang/lib/Format/ContinuationIndenter.cpp | 15 ++++---- clang/lib/Format/Format.cpp | 4 -- clang/lib/Format/FormatToken.h | 1 + clang/lib/Format/TokenAnnotator.cpp | 47 ++++++++++++++++++++++-- clang/lib/Format/TokenAnnotator.h | 5 +-- clang/unittests/Format/FormatTest.cpp | 61 +++++++++++++++++-------------- 7 files changed, 86 insertions(+), 53 deletions(-) diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h index 0dfd7f0..3cf2a082 100644 --- a/clang/include/clang/Format/Format.h +++ b/clang/include/clang/Format/Format.h @@ -293,10 +293,6 @@ struct FormatStyle { /// a zero-length name is assumed. bool Cpp11BracedListStyle; - /// \brief If \c true, indent when breaking function declarations which - /// are not also definitions after the type. - bool IndentFunctionDeclarationAfterType; - /// \brief If \c true, spaces will be inserted after '(' and before ')'. bool SpacesInParentheses; @@ -387,8 +383,6 @@ struct FormatStyle { ExperimentalAutoDetectBinPacking == R.ExperimentalAutoDetectBinPacking && IndentCaseLabels == R.IndentCaseLabels && - IndentFunctionDeclarationAfterType == - R.IndentFunctionDeclarationAfterType && IndentWidth == R.IndentWidth && Language == R.Language && MaxEmptyLinesToKeep == R.MaxEmptyLinesToKeep && KeepEmptyLinesAtTheStartOfBlocks == diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp index 39baef6..683bb69 100644 --- a/clang/lib/Format/ContinuationIndenter.cpp +++ b/clang/lib/Format/ContinuationIndenter.cpp @@ -203,10 +203,12 @@ bool ContinuationIndenter::mustBreak(const LineState &State) { !Current.isTrailingComment()) return true; - if ((Current.Type == TT_StartOfName || Current.is(tok::kw_operator)) && - State.Line->MightBeFunctionDecl && - State.Stack.back().BreakBeforeParameter && Current.NestingLevel == 0) + // If the return type spans multiple lines, wrap before the function name. + if ((Current.Type == TT_FunctionDeclarationName || + Current.is(tok::kw_operator)) && + State.Stack.back().BreakBeforeParameter) return true; + if (startsSegmentOfBuilderTypeCall(Current) && (State.Stack.back().CallContinuation != 0 || (State.Stack.back().BreakBeforeParameter && @@ -518,11 +520,8 @@ unsigned ContinuationIndenter::getNewLineColumn(const LineState &State) { return State.Stack.back().VariablePos; if ((PreviousNonComment && (PreviousNonComment->ClosesTemplateDeclaration || PreviousNonComment->Type == TT_AttributeParen)) || - ((NextNonComment->Type == TT_StartOfName || - NextNonComment->is(tok::kw_operator)) && - Current.NestingLevel == 0 && - (!Style.IndentFunctionDeclarationAfterType || - State.Line->StartsDefinition))) + NextNonComment->is(tok::kw_operator) || + NextNonComment->Type == TT_FunctionDeclarationName) return std::max(State.Stack.back().LastSpace, State.Stack.back().Indent); if (NextNonComment->Type == TT_SelectorName) { if (!State.Stack.back().ObjCSelectorNameFound) { diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index 2ac5c9d..a65f2e2 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -216,8 +216,6 @@ template <> struct MappingTraits { IO.mapOptional("TabWidth", Style.TabWidth); IO.mapOptional("UseTab", Style.UseTab); IO.mapOptional("BreakBeforeBraces", Style.BreakBeforeBraces); - IO.mapOptional("IndentFunctionDeclarationAfterType", - Style.IndentFunctionDeclarationAfterType); IO.mapOptional("SpacesInParentheses", Style.SpacesInParentheses); IO.mapOptional("SpacesInAngles", Style.SpacesInAngles); IO.mapOptional("SpaceInEmptyParentheses", Style.SpaceInEmptyParentheses); @@ -328,7 +326,6 @@ FormatStyle getLLVMStyle() { LLVMStyle.ForEachMacros.push_back("Q_FOREACH"); LLVMStyle.ForEachMacros.push_back("BOOST_FOREACH"); LLVMStyle.IndentCaseLabels = false; - LLVMStyle.IndentFunctionDeclarationAfterType = false; LLVMStyle.IndentWidth = 2; LLVMStyle.TabWidth = 8; LLVMStyle.MaxEmptyLinesToKeep = 1; @@ -373,7 +370,6 @@ FormatStyle getGoogleStyle(FormatStyle::LanguageKind Language) { GoogleStyle.ConstructorInitializerAllOnOneLineOrOnePerLine = true; GoogleStyle.DerivePointerAlignment = true; GoogleStyle.IndentCaseLabels = true; - GoogleStyle.IndentFunctionDeclarationAfterType = true; GoogleStyle.KeepEmptyLinesAtTheStartOfBlocks = false; GoogleStyle.ObjCSpaceAfterProperty = false; GoogleStyle.ObjCSpaceBeforeProtocolList = false; diff --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h index 26fa061..d83804e 100644 --- a/clang/lib/Format/FormatToken.h +++ b/clang/lib/Format/FormatToken.h @@ -40,6 +40,7 @@ enum TokenType { TT_CtorInitializerComma, TT_DesignatedInitializerPeriod, TT_DictLiteral, + TT_FunctionDeclarationName, TT_FunctionLBrace, TT_FunctionTypeLParen, TT_ImplicitStringLiteral, diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp index f68883d..56aa384 100644 --- a/clang/lib/Format/TokenAnnotator.cpp +++ b/clang/lib/Format/TokenAnnotator.cpp @@ -325,8 +325,6 @@ private: return false; } } - // No closing "}" found, this probably starts a definition. - Line.StartsDefinition = true; return true; } @@ -1201,6 +1199,43 @@ void TokenAnnotator::annotate(AnnotatedLine &Line) { Line.First->CanBreakBefore = Line.First->MustBreakBefore; } +// This function heuristically determines whether 'Current' starts the name of a +// function declaration. +static bool isFunctionDeclarationName(const FormatToken &Current) { + if (Current.Type != TT_StartOfName || + Current.NestingLevel != 0 || + Current.Previous->Type == TT_StartOfName) + return false; + const FormatToken *Next = Current.Next; + for (; Next; Next = Next->Next) { + if (Next->Type == TT_TemplateOpener) { + Next = Next->MatchingParen; + } else if (Next->is(tok::coloncolon)) { + Next = Next->Next; + if (!Next || !Next->is(tok::identifier)) + return false; + } else if (Next->is(tok::l_paren)) { + break; + } else { + return false; + } + } + if (!Next) + return false; + assert(Next->is(tok::l_paren)); + if (Next->Next == Next->MatchingParen) + return true; + for (const FormatToken *Tok = Next->Next; Tok != Next->MatchingParen; + Tok = Tok->Next) { + if (Tok->is(tok::kw_const) || Tok->isSimpleTypeSpecifier() || + Tok->Type == TT_PointerOrReference || Tok->Type == TT_StartOfName) + return true; + if (Tok->isOneOf(tok::l_brace, tok::string_literal) || Tok->Tok.isLiteral()) + return false; + } + return false; +} + void TokenAnnotator::calculateFormattingInformation(AnnotatedLine &Line) { for (SmallVectorImpl::iterator I = Line.Children.begin(), E = Line.Children.end(); @@ -1215,6 +1250,8 @@ void TokenAnnotator::calculateFormattingInformation(AnnotatedLine &Line) { FormatToken *Current = Line.First->Next; bool InFunctionDecl = Line.MightBeFunctionDecl; while (Current) { + if (isFunctionDeclarationName(*Current)) + Current->Type = TT_FunctionDeclarationName; if (Current->Type == TT_LineComment) { if (Current->Previous->BlockKind == BK_BracedInit && Current->Previous->opensScope()) @@ -1320,7 +1357,8 @@ unsigned TokenAnnotator::splitPenalty(const AnnotatedLine &Line, if (Right.Type != TT_ObjCMethodExpr && Right.Type != TT_LambdaLSquare) return 500; } - if (Right.Type == TT_StartOfName || Right.is(tok::kw_operator)) { + if (Right.Type == TT_StartOfName || + Right.Type == TT_FunctionDeclarationName || Right.is(tok::kw_operator)) { if (Line.First->is(tok::kw_for) && Right.PartOfMultiVariableDeclStmt) return 3; if (Left.Type == TT_StartOfName) @@ -1674,7 +1712,8 @@ bool TokenAnnotator::canBreakBefore(const AnnotatedLine &Line, return false; if (Left.Tok.getObjCKeywordID() == tok::objc_interface) return false; - if (Right.Type == TT_StartOfName || Right.is(tok::kw_operator)) + if (Right.Type == TT_StartOfName || + Right.Type == TT_FunctionDeclarationName || Right.is(tok::kw_operator)) return true; if (Right.isTrailingComment()) // We rely on MustBreakBefore being set correctly here as we should not diff --git a/clang/lib/Format/TokenAnnotator.h b/clang/lib/Format/TokenAnnotator.h index 0df70a0..36de010 100644 --- a/clang/lib/Format/TokenAnnotator.h +++ b/clang/lib/Format/TokenAnnotator.h @@ -41,8 +41,8 @@ public: : First(Line.Tokens.front().Tok), Level(Line.Level), InPPDirective(Line.InPPDirective), MustBeDeclaration(Line.MustBeDeclaration), MightBeFunctionDecl(false), - StartsDefinition(false), Affected(false), - LeadingEmptyLinesAffected(false), ChildrenAffected(false) { + Affected(false), LeadingEmptyLinesAffected(false), + ChildrenAffected(false) { assert(!Line.Tokens.empty()); // Calculate Next and Previous for all tokens. Note that we must overwrite @@ -86,7 +86,6 @@ public: bool InPPDirective; bool MustBeDeclaration; bool MightBeFunctionDecl; - bool StartsDefinition; /// \c True if this line should be formatted, i.e. intersects directly or /// indirectly with one of the input ranges. diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 96274fc..7029b43 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -1264,13 +1264,13 @@ TEST_F(FormatTest, DontSplitLineCommentsWithEscapedNewlines) { getLLVMStyleWithColumns(50))); // FIXME: One day we might want to implement adjustment of leading whitespace // of the consecutive lines in this kind of comment: - EXPECT_EQ("int\n" - "a; // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\\\n" - " // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\\\n" - " // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA", - format("int a; // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\\\n" - " // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\\\n" - " // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA", + EXPECT_EQ("double\n" + " a; // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\\\n" + " // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\\\n" + " // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA", + format("double a; // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\\\n" + " // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\\\n" + " // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA", getLLVMStyleWithColumns(49))); } @@ -2687,7 +2687,7 @@ TEST_F(FormatTest, LayoutStatementsAroundPreprocessorDirectives) { EXPECT_EQ("int\n" "#define A\n" " a;", - format("int\n#define A\na;", getGoogleStyle())); + format("int\n#define A\na;")); verifyFormat("functionCallTo(\n" " someOtherFunction(\n" " withSomeParameters, whichInSequence,\n" @@ -3359,7 +3359,7 @@ TEST_F(FormatTest, BreaksFunctionDeclarations) { // 2) break after return type. verifyFormat( "Aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" - " bbbbbbbbbbbbbb(Cccccccccccccc cccccccccccccccccccccccccc);", + "bbbbbbbbbbbbbb(Cccccccccccccc cccccccccccccccccccccccccc);", getGoogleStyle()); // 3) break after (. @@ -3371,8 +3371,8 @@ TEST_F(FormatTest, BreaksFunctionDeclarations) { // 4) break before after nested name specifiers. verifyFormat( "Aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" - " SomeClasssssssssssssssssssssssssssssssssssssss::\n" - " bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb(Cccccccccccccc cccccccccc);", + "SomeClasssssssssssssssssssssssssssssssssssssss::\n" + " bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb(Cccccccccccccc cccccccccc);", getGoogleStyle()); // However, there are exceptions, if a sufficient amount of lines can be @@ -3386,9 +3386,9 @@ TEST_F(FormatTest, BreaksFunctionDeclarations) { " Cccccccccccccc cccccccccc);"); verifyFormat( "Aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" - " bbbbbbbbbbb(Cccccccccccccc cccccccccc, Cccccccccccccc cccccccccc,\n" - " Cccccccccccccc cccccccccc, Cccccccccccccc cccccccccc,\n" - " Cccccccccccccc cccccccccc, Cccccccccccccc cccccccccc);", + "bbbbbbbbbbb(Cccccccccccccc cccccccccc, Cccccccccccccc cccccccccc,\n" + " Cccccccccccccc cccccccccc, Cccccccccccccc cccccccccc,\n" + " Cccccccccccccc cccccccccc, Cccccccccccccc cccccccccc);", getGoogleStyle()); verifyFormat( "Aaaaaaaaaa bbbbbbbbbbbbbbbbbbbbbbbbbbbbbb(Cccccccccccccc cccccccccc,\n" @@ -5054,22 +5054,28 @@ TEST_F(FormatTest, FormatsFunctionTypes) { verifyFormat("void f() { function(*some_pointer_var)[0] = 10; }"); } +TEST_F(FormatTest, BreaksLongVariableDeclarations) { + verifyFormat("LoooooooooooooooooooooooooooooooooooooooongType\n" + " LoooooooooooooooooooooooooooooooooooooooongVariable;"); + verifyFormat("LoooooooooooooooooooooooooooooooooooooooongType const\n" + " LoooooooooooooooooooooooooooooooooooooooongVariable;"); + + // Different ways of ()-initializiation. + verifyFormat("LoooooooooooooooooooooooooooooooooooooooongType\n" + " LoooooooooooooooooooooooooooooooooooooooongVariable(1);"); + verifyFormat("LoooooooooooooooooooooooooooooooooooooooongType\n" + " LoooooooooooooooooooooooooooooooooooooooongVariable(a);"); + verifyFormat("LoooooooooooooooooooooooooooooooooooooooongType\n" + " LoooooooooooooooooooooooooooooooooooooooongVariable({});"); +} + TEST_F(FormatTest, BreaksLongDeclarations) { verifyFormat("typedef LoooooooooooooooooooooooooooooooooooooooongType\n" - " AnotherNameForTheLongType;", - getGoogleStyle()); + " AnotherNameForTheLongType;"); verifyFormat("typedef LongTemplateType\n" - " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa;", - getGoogleStyle()); - verifyFormat("LoooooooooooooooooooooooooooooooooooooooongType\n" - " LoooooooooooooooooooooooooooooooooooooooongVariable;", - getGoogleStyle()); - verifyFormat("LoooooooooooooooooooooooooooooooooooooooongType const\n" - " LoooooooooooooooooooooooooooooooooooooooongVariable;", - getGoogleStyle()); + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa;"); verifyFormat("LoooooooooooooooooooooooooooooooooooooooongReturnType\n" - " LoooooooooooooooooooooooooooooooongFunctionDeclaration();", - getGoogleStyle()); + "LoooooooooooooooooooooooooooooooongFunctionDeclaration();"); verifyFormat("LoooooooooooooooooooooooooooooooooooooooongReturnType\n" "LooooooooooooooooooooooooooooooooooongFunctionDefinition() {}"); verifyFormat("LoooooooooooooooooooooooooooooooooooooooongReturnType const\n" @@ -8101,7 +8107,6 @@ TEST_F(FormatTest, ParsesConfiguration) { CHECK_PARSE_BOOL(ObjCSpaceAfterProperty); CHECK_PARSE_BOOL(ObjCSpaceBeforeProtocolList); CHECK_PARSE_BOOL(Cpp11BracedListStyle); - CHECK_PARSE_BOOL(IndentFunctionDeclarationAfterType); CHECK_PARSE_BOOL(SpacesInParentheses); CHECK_PARSE_BOOL(SpacesInAngles); CHECK_PARSE_BOOL(SpaceInEmptyParentheses); @@ -9044,7 +9049,7 @@ TEST_F(FormatTest, HandleConflictMarkers) { "=======\n" "Other \\\n" ">>>>>>>\n" - "End int i;\n", + " End int i;\n", format("#define Macro \\\n" "<<<<<<<\n" " Something \\\n" -- 2.7.4