As for Chromium and WebKit coding styles, I am already looking at them specifically, as they will require some things that are much easier in Google/LLVM. As for configuration, the coding styles are currently hard-coded mostly because it is easiest, makes clang-format easy to use and avoids configuration files running out of sync with the clang-format version. Once we are a bit further along (maybe soon-ish), I agree that config files are a better solution at least as an additional option.
On Tue, Dec 18, 2012 at 10:45 PM, Nico Weber <[email protected]> wrote: > Out of interest, what's the plan for adding more coding styles? It looks > like there's an coding style enum somewhere; are projects who don't use one > of the existing coding styles expected to contribute patches to clang > itself for their styles? Or will there be some config file eventually? > > I'm asking because webkit style for initializers prefers one line per > initializer (and leading commas): > http://www.webkit.org/coding/coding-style.html#punctuation > > Nico > > On Tue, Dec 18, 2012 at 1:05 PM, Daniel Jasper <[email protected]> wrote: > >> Author: djasper >> Date: Tue Dec 18 15:05:13 2012 >> New Revision: 170457 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=170457&view=rev >> Log: >> Better support for constructor initializers. >> >> We used to format initializers like this (with a sort of hacky >> implementation): >> Constructor() >> : Val1(A), >> Val2(B) { >> >> and now format like this (with a somewhat better solution): >> Constructor() >> : Val1(A), Val2(B) { >> >> assuming this would not fit on a single line. Also added tests. >> >> As a side effect we now first analyze whether an UnwrappedLine needs to be >> split at all. If not, not splitting it is the best solution by >> definition. As >> this should be a very common case in normal code, not exploring the entire >> solution space can provide significant speedup. >> >> 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=170457&r1=170456&r2=170457&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/Format/Format.cpp (original) >> +++ cfe/trunk/lib/Format/Format.cpp Tue Dec 18 15:05:13 2012 >> @@ -37,6 +37,7 @@ >> TT_OverloadedOperator, >> TT_PointerOrReference, >> TT_ConditionalExpr, >> + TT_CtorInitializerColon, >> TT_LineComment, >> TT_BlockComment >> }; >> @@ -73,7 +74,6 @@ >> } >> >> struct OptimizationParameters { >> - unsigned PenaltyExtraLine; >> unsigned PenaltyIndentLevel; >> }; >> >> @@ -83,13 +83,9 @@ >> const UnwrappedLine &Line, >> const std::vector<TokenAnnotation> &Annotations, >> tooling::Replacements &Replaces, bool >> StructuralError) >> - : Style(Style), >> - SourceMgr(SourceMgr), >> - Line(Line), >> - Annotations(Annotations), >> - Replaces(Replaces), >> + : Style(Style), SourceMgr(SourceMgr), Line(Line), >> + Annotations(Annotations), Replaces(Replaces), >> StructuralError(StructuralError) { >> - Parameters.PenaltyExtraLine = 100; >> Parameters.PenaltyIndentLevel = 5; >> } >> >> @@ -100,8 +96,6 @@ >> // Initialize state dependent on indent. >> IndentState State; >> State.Column = Indent; >> - State.CtorInitializerOnNewLine = false; >> - State.InCtorInitializer = false; >> State.ConsumedTokens = 0; >> State.Indent.push_back(Indent + 4); >> State.LastSpace.push_back(Indent); >> @@ -110,11 +104,33 @@ >> // The first token has already been indented and thus consumed. >> moveStateToNextToken(State); >> >> + // Check whether the UnwrappedLine can be put onto a single line. If >> so, >> + // this is bound to be the optimal solution (by definition) and we >> don't >> + // need to analyze the entire solution space. >> + unsigned Columns = State.Column; >> + bool FitsOnALine = true; >> + for (unsigned i = 1, n = Line.Tokens.size(); i != n; ++i) { >> + Columns += (Annotations[i].SpaceRequiredBefore ? 1 : 0) + >> + Line.Tokens[i].Tok.getLength(); >> + // A special case for the colon of a constructor initializer as >> this only >> + // needs to be put on a new line if the line needs to be split. >> + if (Columns > Style.ColumnLimit || >> + (Annotations[i].MustBreakBefore && >> + Annotations[i].Type != >> TokenAnnotation::TT_CtorInitializerColon)) { >> + FitsOnALine = false; >> + break; >> + } >> + } >> + >> // Start iterating at 1 as we have correctly formatted of Token #0 >> above. >> for (unsigned i = 1, n = Line.Tokens.size(); i != n; ++i) { >> - unsigned NoBreak = calcPenalty(State, false, UINT_MAX); >> - unsigned Break = calcPenalty(State, true, NoBreak); >> - addTokenToState(Break < NoBreak, false, State); >> + if (FitsOnALine) { >> + addTokenToState(false, false, State); >> + } else { >> + unsigned NoBreak = calcPenalty(State, false, UINT_MAX); >> + unsigned Break = calcPenalty(State, true, NoBreak); >> + addTokenToState(Break < NoBreak, false, State); >> + } >> } >> } >> >> @@ -146,9 +162,6 @@ >> /// on a level. >> std::vector<unsigned> FirstLessLess; >> >> - bool CtorInitializerOnNewLine; >> - bool InCtorInitializer; >> - >> /// \brief Comparison operator to be able to used \c IndentState in >> \c map. >> bool operator<(const IndentState &Other) const { >> if (Other.ConsumedTokens != ConsumedTokens) >> @@ -212,11 +225,8 @@ >> >> State.LastSpace[ParenLevel] = State.Indent[ParenLevel]; >> if (Current.Tok.is(tok::colon) && >> - Annotations[Index].Type != >> TokenAnnotation::TT_ConditionalExpr) { >> + Annotations[Index].Type != TokenAnnotation::TT_ConditionalExpr) >> State.Indent[ParenLevel] += 2; >> - State.CtorInitializerOnNewLine = true; >> - State.InCtorInitializer = true; >> - } >> } else { >> unsigned Spaces = Annotations[Index].SpaceRequiredBefore ? 1 : 0; >> if (Annotations[Index].Type == TokenAnnotation::TT_LineComment) >> @@ -228,10 +238,7 @@ >> if (Previous.Tok.is(tok::l_paren) || >> Annotations[Index - 1].Type == >> TokenAnnotation::TT_TemplateOpener) >> State.Indent[ParenLevel] = State.Column; >> - if (Current.Tok.is(tok::colon)) { >> - State.Indent[ParenLevel] = State.Column + 3; >> - State.InCtorInitializer = true; >> - } >> + >> // Top-level spaces are exempt as that mostly leads to better >> results. >> State.Column += Spaces; >> if (Spaces > 0 && ParenLevel != 0) >> @@ -279,10 +286,8 @@ >> "Tried to calculate penalty for splitting after the last >> token"); >> const FormatToken &Left = Line.Tokens[Index]; >> const FormatToken &Right = Line.Tokens[Index + 1]; >> - if (Left.Tok.is(tok::semi)) >> + if (Left.Tok.is(tok::semi) || Left.Tok.is(tok::comma)) >> return 0; >> - if (Left.Tok.is(tok::comma)) >> - return 1; >> if (Left.Tok.is(tok::equal) || Left.Tok.is(tok::l_paren) || >> Left.Tok.is(tok::pipepipe) || Left.Tok.is(tok::ampamp)) >> return 2; >> @@ -313,18 +318,10 @@ >> if (NewLine && !Annotations[State.ConsumedTokens].CanBreakBefore) >> return UINT_MAX; >> >> - if (State.ConsumedTokens > 0 && !NewLine && >> - State.CtorInitializerOnNewLine && >> - Line.Tokens[State.ConsumedTokens - 1].Tok.is(tok::comma)) >> - return UINT_MAX; >> - >> - if (NewLine && State.InCtorInitializer && >> !State.CtorInitializerOnNewLine) >> - return UINT_MAX; >> - >> unsigned CurrentPenalty = 0; >> if (NewLine) { >> CurrentPenalty += Parameters.PenaltyIndentLevel * >> State.Indent.size() + >> - Parameters.PenaltyExtraLine + >> splitPenalty(State.ConsumedTokens - 1); >> + splitPenalty(State.ConsumedTokens - 1); >> } >> >> addTokenToState(NewLine, true, State); >> @@ -413,9 +410,7 @@ >> public: >> TokenAnnotator(const UnwrappedLine &Line, const FormatStyle &Style, >> SourceManager &SourceMgr) >> - : Line(Line), >> - Style(Style), >> - SourceMgr(SourceMgr) { >> + : Line(Line), Style(Style), SourceMgr(SourceMgr) { >> } >> >> /// \brief A parser that gathers additional information about tokens. >> @@ -427,9 +422,7 @@ >> public: >> AnnotatingParser(const SmallVector<FormatToken, 16> &Tokens, >> std::vector<TokenAnnotation> &Annotations) >> - : Tokens(Tokens), >> - Annotations(Annotations), >> - Index(0) { >> + : Tokens(Tokens), Annotations(Annotations), Index(0) { >> } >> >> bool parseAngle() { >> @@ -496,6 +489,10 @@ >> switch (Tokens[CurrentIndex].Tok.getKind()) { >> case tok::l_paren: >> parseParens(); >> + if (Index < Tokens.size() && Tokens[Index].Tok.is(tok::colon)) { >> + Annotations[Index].Type = >> TokenAnnotation::TT_CtorInitializerColon; >> + next(); >> + } >> break; >> case tok::l_square: >> parseSquare(); >> @@ -557,7 +554,10 @@ >> Annotation.CanBreakBefore = >> canBreakBetween(Line.Tokens[i - 1], Line.Tokens[i]); >> >> - if (Line.Tokens[i].Tok.is(tok::colon)) { >> + if (Annotation.Type == TokenAnnotation::TT_CtorInitializerColon) { >> + Annotation.MustBreakBefore = true; >> + Annotation.SpaceRequiredBefore = true; >> + } else if (Line.Tokens[i].Tok.is(tok::colon)) { >> Annotation.SpaceRequiredBefore = >> Line.Tokens[0].Tok.isNot(tok::kw_case) && i != e - 1; >> } else if (Annotations[i - 1].Type == >> TokenAnnotation::TT_UnaryOperator) { >> @@ -769,9 +769,7 @@ >> class LexerBasedFormatTokenSource : public FormatTokenSource { >> public: >> LexerBasedFormatTokenSource(Lexer &Lex, SourceManager &SourceMgr) >> - : GreaterStashed(false), >> - Lex(Lex), >> - SourceMgr(SourceMgr), >> + : GreaterStashed(false), Lex(Lex), SourceMgr(SourceMgr), >> IdentTable(Lex.getLangOpts()) { >> Lex.SetKeepWhitespaceMode(true); >> } >> @@ -831,10 +829,7 @@ >> public: >> Formatter(const FormatStyle &Style, Lexer &Lex, SourceManager >> &SourceMgr, >> const std::vector<CharSourceRange> &Ranges) >> - : Style(Style), >> - Lex(Lex), >> - SourceMgr(SourceMgr), >> - Ranges(Ranges), >> + : Style(Style), Lex(Lex), SourceMgr(SourceMgr), Ranges(Ranges), >> StructuralError(false) { >> } >> >> >> Modified: cfe/trunk/unittests/Format/FormatTest.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=170457&r1=170456&r2=170457&view=diff >> >> ============================================================================== >> --- cfe/trunk/unittests/Format/FormatTest.cpp (original) >> +++ cfe/trunk/unittests/Format/FormatTest.cpp Tue Dec 18 15:05:13 2012 >> @@ -374,6 +374,41 @@ >> " parameter, parameter, parameter)), >> SecondLongCall(parameter));"); >> } >> >> +TEST_F(FormatTest, ConstructorInitializers) { >> + verifyFormat("Constructor() : Initializer(FitsOnTheLine) {\n}"); >> + >> + verifyFormat( >> + "SomeClass::Constructor()\n" >> + " : aaaaaaaaaaaaa(aaaaaaaaaaaaaa), >> aaaaaaaaaaaaaaa(aaaaaaaaaaaa) {\n" >> + "}"); >> + >> + verifyFormat( >> + "SomeClass::Constructor()\n" >> + " : >> aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa),\n" >> + " aaaaaaaaaaaaaaa(aaaaaaaaaaaa) {\n" >> + "}"); >> + >> + verifyFormat("Constructor()\n" >> + " : >> aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaa),\n" >> + " >> aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaa,\n" >> + " >> aaaaaaaaaaaaaaaaaaaaaaaaaaa),\n" >> + " aaaaaaaaaaaaaaaaaaaaaaa() {\n" >> + "}"); >> + >> + // Here a line could be saved by splitting the second initializer onto >> two >> + // lines, but that is not desireable. >> + verifyFormat("Constructor()\n" >> + " : >> aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaa),\n" >> + " aaaaaaaaaaa(aaaaaaaaaaa),\n" >> + " >> aaaaaaaaaaaaaaaaaaaaat(aaaaaaaaaaaaaaaaaaaaaaaaaaaa) {\n" >> + "}"); >> + >> + verifyGoogleFormat("MyClass::MyClass(int var)\n" >> + " : some_var_(var), // 4 space indent\n" >> + " some_other_var_(var + 1) { // lined up\n" >> + "}"); >> +} >> + >> TEST_F(FormatTest, BreaksAsHighAsPossible) { >> verifyFormat( >> "if ((aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa && >> aaaaaaaaaaaaaaaaaaaaaaaaaa) ||\n" >> @@ -461,13 +496,11 @@ >> } >> >> TEST_F(FormatTest, WrapsAtFunctionCallsIfNecessary) { >> - verifyFormat( >> - "LoooooooooooooooooooooooooooooooooooooongObject\n" >> - " .looooooooooooooooooooooooooooooooooooooongFunction();"); >> + verifyFormat("LoooooooooooooooooooooooooooooooooooooongObject\n" >> + " >> .looooooooooooooooooooooooooooooooooooooongFunction();"); >> >> - verifyFormat( >> - "LoooooooooooooooooooooooooooooooooooooongObject\n" >> - " ->looooooooooooooooooooooooooooooooooooooongFunction();"); >> + verifyFormat("LoooooooooooooooooooooooooooooooooooooongObject\n" >> + " >> ->looooooooooooooooooooooooooooooooooooooongFunction();"); >> >> verifyFormat( >> >> "LooooooooooooooooooooooooooooooooongObject->shortFunction(Parameter1,\n" >> @@ -485,10 +518,9 @@ >> "function(LoooooooooooooooooooooooooooooooooooongObject\n" >> " >> ->loooooooooooooooooooooooooooooooooooooooongFunction());"); >> >> - verifyFormat( >> - "if (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaa) ||\n" >> - " aaaa.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) {\n" >> - "}"); >> + verifyFormat("if >> (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaa) ||\n" >> + " aaaa.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) {\n" >> + "}"); >> } >> >> 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
