>From what I've seen, Clang's style is to not break after the return type even >in a function declaration. As you say, it's sort of an issue if the >declarations stack up next to each other, but we generally don't do that >anyway, and couldn't do it if the functions are doc-commented.
Jordan On May 6, 2013, at 1:27 , Daniel Jasper <[email protected]> wrote: > Author: djasper > Date: Mon May 6 03:27:33 2013 > New Revision: 181187 > > URL: http://llvm.org/viewvc/llvm-project?rev=181187&view=rev > Log: > Change indentation when breaking after a type. > > clang-format did not indent any declarations/definitions when breaking > after the type. With this change, it indents for all declarations but > does not indent for function definitions, i.e.: > > Before: > const SomeLongTypeName& > some_long_variable_name; > typedef SomeLongTypeName > SomeLongTypeAlias; > const SomeLongReturnType* > SomeLongFunctionName(); > const SomeLongReturnType* > SomeLongFunctionName() { ... } > > After: > const SomeLongTypeName& > some_long_variable_name; > typedef SomeLongTypeName > SomeLongTypeAlias; > const SomeLongReturnType* > SomeLongFunctionName(); > const SomeLongReturnType* > SomeLongFunctionName() { ... } > > While it might seem inconsistent to indent function declarations, but > not definitions, there are two reasons for that: > - Function declarations are very similar to declarations of function > type variables, so there is another side to consistency to consider. > - There can be many function declarations on subsequent lines and not > indenting can make them harder to identify. Function definitions > are already separated by their body and not indenting > makes the function name slighly easier to find. > > 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=181187&r1=181186&r2=181187&view=diff > ============================================================================== > --- cfe/trunk/lib/Format/Format.cpp (original) > +++ cfe/trunk/lib/Format/Format.cpp Mon May 6 03:27:33 2013 > @@ -359,7 +359,8 @@ private: > State.Stack.back().VariablePos != 0) { > State.Column = State.Stack.back().VariablePos; > } else if (Previous.ClosesTemplateDeclaration || > - (Current.Type == TT_StartOfName && State.ParenLevel == 0)) { > + (Current.Type == TT_StartOfName && State.ParenLevel == 0 && > + Line.StartsDefinition)) { > State.Column = State.Stack.back().Indent; > } else if (Current.Type == TT_ObjCSelectorName) { > if (State.Stack.back().ColonPos > Current.FormatTok.TokenLength) { > > Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=181187&r1=181186&r2=181187&view=diff > ============================================================================== > --- cfe/trunk/lib/Format/TokenAnnotator.cpp (original) > +++ cfe/trunk/lib/Format/TokenAnnotator.cpp Mon May 6 03:27:33 2013 > @@ -245,24 +245,25 @@ private: > } > > bool parseBrace() { > - // Lines are fine to end with '{'. > - if (CurrentToken == NULL) > - return true; > - ScopedContextCreator ContextCreator(*this, tok::l_brace, 1); > - AnnotatedToken *Left = CurrentToken->Parent; > - while (CurrentToken != NULL) { > - if (CurrentToken->is(tok::r_brace)) { > - Left->MatchingParen = CurrentToken; > - CurrentToken->MatchingParen = Left; > - next(); > - return true; > + if (CurrentToken != NULL) { > + ScopedContextCreator ContextCreator(*this, tok::l_brace, 1); > + AnnotatedToken *Left = CurrentToken->Parent; > + while (CurrentToken != NULL) { > + if (CurrentToken->is(tok::r_brace)) { > + Left->MatchingParen = CurrentToken; > + CurrentToken->MatchingParen = Left; > + next(); > + return true; > + } > + if (CurrentToken->isOneOf(tok::r_paren, tok::r_square)) > + return false; > + updateParameterCount(Left, CurrentToken); > + if (!consumeToken()) > + return false; > } > - if (CurrentToken->isOneOf(tok::r_paren, tok::r_square)) > - return false; > - updateParameterCount(Left, CurrentToken); > - if (!consumeToken()) > - return false; > } > + // No closing "}" found, this probably starts a definition. > + Line.StartsDefinition = true; > return true; > } > > > Modified: cfe/trunk/lib/Format/TokenAnnotator.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.h?rev=181187&r1=181186&r2=181187&view=diff > ============================================================================== > --- cfe/trunk/lib/Format/TokenAnnotator.h (original) > +++ cfe/trunk/lib/Format/TokenAnnotator.h Mon May 6 03:27:33 2013 > @@ -209,8 +209,8 @@ public: > AnnotatedLine(const UnwrappedLine &Line) > : First(Line.Tokens.front()), Level(Line.Level), > InPPDirective(Line.InPPDirective), > - MustBeDeclaration(Line.MustBeDeclaration), > - MightBeFunctionDecl(false) { > + MustBeDeclaration(Line.MustBeDeclaration), > MightBeFunctionDecl(false), > + StartsDefinition(false) { > assert(!Line.Tokens.empty()); > AnnotatedToken *Current = &First; > for (std::list<FormatToken>::const_iterator I = ++Line.Tokens.begin(), > @@ -226,7 +226,8 @@ public: > : First(Other.First), Type(Other.Type), Level(Other.Level), > InPPDirective(Other.InPPDirective), > MustBeDeclaration(Other.MustBeDeclaration), > - MightBeFunctionDecl(Other.MightBeFunctionDecl) { > + MightBeFunctionDecl(Other.MightBeFunctionDecl), > + StartsDefinition(Other.StartsDefinition) { > Last = &First; > while (!Last->Children.empty()) { > Last->Children[0].Parent = Last; > @@ -242,6 +243,7 @@ public: > bool InPPDirective; > bool MustBeDeclaration; > bool MightBeFunctionDecl; > + bool StartsDefinition; > }; > > 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=181187&r1=181186&r2=181187&view=diff > ============================================================================== > --- cfe/trunk/unittests/Format/FormatTest.cpp (original) > +++ cfe/trunk/unittests/Format/FormatTest.cpp Mon May 6 03:27:33 2013 > @@ -1520,7 +1520,7 @@ TEST_F(FormatTest, MixingPreprocessorDir > TEST_F(FormatTest, LayoutStatementsAroundPreprocessorDirectives) { > EXPECT_EQ("int\n" > "#define A\n" > - "a;", > + " a;", > format("int\n#define A\na;")); > verifyFormat("functionCallTo(\n" > " someOtherFunction(\n" > @@ -1713,7 +1713,7 @@ TEST_F(FormatTest, MemoizationTests) { > "CFRunLoopTimerCreate(CFAllocatorRef allocato, CFAbsoluteTime > fireDate,\n" > " CFTimeInterval interval, CFOptionFlags flags,\n" > " CFIndex order, CFRunLoopTimerCallBack callout,\n" > - " CFRunLoopTimerContext *context);"); > + " CFRunLoopTimerContext *context) {}"); > > // Deep nesting somewhat works around our memoization. > verifyFormat( > @@ -1755,8 +1755,9 @@ TEST_F(FormatTest, BreaksFunctionDeclara > " Cccccccccccccc > cccccccccccccc);"); > > // 2) break after return type. > - verifyFormat("Aaaaaaaaaaaaaaaaaaaaaaaa\n" > - "bbbbbbbbbbbbbb(Cccccccccccccc cccccccccccccccccccccccccc);"); > + verifyFormat( > + "Aaaaaaaaaaaaaaaaaaaaaaaa\n" > + " bbbbbbbbbbbbbb(Cccccccccccccc cccccccccccccccccccccccccc);"); > > // 3) break after (. > verifyFormat( > @@ -1766,8 +1767,8 @@ TEST_F(FormatTest, BreaksFunctionDeclara > // 4) break before after nested name specifiers. > verifyFormat( > "Aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" > - "SomeClasssssssssssssssssssssssssssssssssssssss::\n" > - " bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb(Cccccccccccccc cccccccccc);"); > + " SomeClasssssssssssssssssssssssssssssssssssssss::\n" > + " bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb(Cccccccccccccc > cccccccccc);"); > > // However, there are exceptions, if a sufficient amount of lines can be > // saved. > @@ -1780,9 +1781,9 @@ TEST_F(FormatTest, BreaksFunctionDeclara > " Cccccccccccccc > cccccccccc);"); > verifyFormat( > "Aaaaaaaaaaaaaaaaaa\n" > - "bbbbbbbbbbbbbb(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);"); > verifyFormat( > "Aaaaaaaaaa bbbbbbbbbbbbbbbbbbbbbbbbbbbbbb(Cccccccccccccc cccccccccc,\n" > " Cccccccccccccc cccccccccc,\n" > @@ -1954,12 +1955,9 @@ TEST_F(FormatTest, DoesNotBreakTrailingA > " aaaaaaaaaaaaaaaaaaaaaaaaa));"); > verifyFormat("bool aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" > " __attribute__((unused));"); > - > - // FIXME: This is bad indentation, but generally hard to distinguish from a > - // function declaration. > verifyFormat( > "bool aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" > - "GUARDED_BY(aaaaaaaaaaaa);"); > + " GUARDED_BY(aaaaaaaaaaaa);"); > } > > TEST_F(FormatTest, BreaksAccordingToOperatorPrecedence) { > @@ -2121,8 +2119,8 @@ TEST_F(FormatTest, DeclarationsOfMultipl > // FIXME: If multiple variables are defined, the "*" needs to move to the > new > // line. Also fix indent for breaking after the type, this looks bad. > verifyFormat("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa *\n" > - "aaaaaaaaaaaaaaaaaaaaaaaaaaaaa = aaaaaaaaaaaaaaaaaaa,\n" > - " *b = bbbbbbbbbbbbbbbbbbb;"); > + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaa = aaaaaaaaaaaaaaaaaaa,\n" > + " *b = bbbbbbbbbbbbbbbbbbb;"); > > // Not ideal, but pointer-with-type does not allow much here. > verifyGoogleFormat( > @@ -2707,6 +2705,19 @@ TEST_F(FormatTest, FormatsFunctionTypes) > } > > TEST_F(FormatTest, BreaksLongDeclarations) { > + verifyFormat("typedef LoooooooooooooooooooooooooooooooooooooooongType\n" > + " AnotherNameForTheLongType;"); > + verifyFormat("LoooooooooooooooooooooooooooooooooooooooongType\n" > + " LoooooooooooooooooooooooooooooooooooooooongVariable;"); > + verifyFormat("LoooooooooooooooooooooooooooooooooooooooongReturnType\n" > + " > LoooooooooooooooooooooooooooooooongFunctionDeclaration();"); > + verifyFormat("LoooooooooooooooooooooooooooooooooooooooongReturnType\n" > + "LooooooooooooooooooooooooooooooooooongFunctionDefinition() > {}"); > + > + // FIXME: Without the comment, this breaks after "(". > + verifyFormat("LoooooooooooooooooooooooooooooooooooooooongType // break\n" > + " > (*LoooooooooooooooooooooooooooongFunctionTypeVarialbe)();"); > + > verifyFormat("int *someFunction(int LoooooooooooooooooooongParam1,\n" > " int LoooooooooooooooooooongParam2) {}"); > verifyFormat( > @@ -2722,7 +2733,7 @@ TEST_F(FormatTest, BreaksLongDeclaration > " AnotherLongParameterName) {}"); > verifyFormat( > "aaaaaaaaaaaaaaaa::aaaaaaaaaaaaaaaa<aaaaaaaaaaaaa, aaaaaaaaaaaa>\n" > - "aaaaaaaaaaaaaaaaaaaaaaa;"); > + " aaaaaaaaaaaaaaaaaaaaaaa;"); > > verifyGoogleFormat( > "TypeSpecDecl* TypeSpecDecl::Create(ASTContext& C, DeclContext* DC,\n" > > > _______________________________________________ > 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
