[cfe-commits] r170457 - in /cfe/trunk: lib/Format/Format.cpp unittests/Format/FormatTest.cpp
Nico Weber
thakis at chromium.org
Tue Dec 18 13:45:50 PST 2012
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 <djasper at google.com> 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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121218/028c0b03/attachment.html>
More information about the cfe-commits
mailing list