[cfe-commits] r172413 - in /cfe/trunk: lib/Format/Format.cpp unittests/Format/FormatTest.cpp
Douglas Gregor
dgregor at apple.com
Mon Jan 14 06:52:12 PST 2013
On Jan 14, 2013, at 6:14 AM, Daniel Jasper <djasper at google.com> wrote:
> Author: djasper
> Date: Mon Jan 14 08:14:23 2013
> New Revision: 172413
>
> URL: http://llvm.org/viewvc/llvm-project?rev=172413&view=rev
> Log:
> Put short if statements on a single line.
>
> Before: if (a)
> return;
> After: if (a) return;
>
> Not yet sure, whether this is always desired, but we can add options and
> make this a style parameter as we go along.
I'd prefer this to be a style parameter…
- Doug
> 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=172413&r1=172412&r2=172413&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Format/Format.cpp (original)
> +++ cfe/trunk/lib/Format/Format.cpp Mon Jan 14 08:14:23 2013
> @@ -69,12 +69,9 @@
> CanBreakBefore(false), MustBreakBefore(false),
> ClosesTemplateDeclaration(false), Parent(NULL) {}
>
> - bool is(tok::TokenKind Kind) const {
> - return FormatTok.Tok.is(Kind);
> - }
> - bool isNot(tok::TokenKind Kind) const {
> - return FormatTok.Tok.isNot(Kind);
> - }
> + bool is(tok::TokenKind Kind) const { return FormatTok.Tok.is(Kind); }
> + bool isNot(tok::TokenKind Kind) const { return FormatTok.Tok.isNot(Kind); }
> +
> bool isObjCAtKeyword(tok::ObjCKeywordKind Kind) const {
> return FormatTok.Tok.isObjCAtKeyword(Kind);
> }
> @@ -185,8 +182,8 @@
> /// \p RootToken fits into \p Limit columns on a single line.
> ///
> /// If true, sets \p Length to the required length.
> -static bool fitsIntoLimit(const AnnotatedToken &RootToken,
> - unsigned Limit, unsigned* Length = 0) {
> +static bool fitsIntoLimit(const AnnotatedToken &RootToken, unsigned Limit,
> + unsigned *Length = 0) {
> unsigned Columns = RootToken.FormatTok.TokenLength;
> const AnnotatedToken *Tok = &RootToken;
> while (!Tok->Children.empty()) {
> @@ -781,8 +778,8 @@
> Tok->Type = TT_ObjCMethodExpr;
> break;
> case tok::l_paren: {
> - bool ParensWereObjCReturnType =
> - Tok->Parent && Tok->Parent->Type == TT_ObjCMethodSpecifier;
> + bool ParensWereObjCReturnType = Tok->Parent && Tok->Parent->Type ==
> + TT_ObjCMethodSpecifier;
> if (!parseParens())
> return false;
> if (CurrentToken != NULL && CurrentToken->is(tok::colon)) {
> @@ -1347,8 +1344,8 @@
>
> class Formatter : public UnwrappedLineConsumer {
> public:
> - Formatter(DiagnosticsEngine &Diag, const FormatStyle &Style,
> - Lexer &Lex, SourceManager &SourceMgr,
> + Formatter(DiagnosticsEngine &Diag, const FormatStyle &Style, Lexer &Lex,
> + SourceManager &SourceMgr,
> const std::vector<CharSourceRange> &Ranges)
> : Diag(Diag), Style(Style), Lex(Lex), SourceMgr(SourceMgr),
> Ranges(Ranges) {}
> @@ -1407,25 +1404,46 @@
> // 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 LengthLine1 = 0;
> - if (!fitsIntoLimit(I->First, Limit, &LengthLine1))
> + unsigned Length = 0;
> + if (!fitsIntoLimit(I->First, Limit, &Length))
> return false;
> + Limit -= Length + 1; // One space.
> + if (I + 1 == E)
> + return true;
> +
> + if (I->Last->is(tok::l_brace)) {
> + tryMergeSimpleBlock(I, E, Limit);
> + } else if (I->First.is(tok::kw_if)) {
> + tryMergeSimpleIf(I, E, Limit);
> + }
> + return true;
> + }
>
> + void tryMergeSimpleIf(std::vector<AnnotatedLine>::iterator &I,
> + std::vector<AnnotatedLine>::iterator E,
> + unsigned Limit) {
> + AnnotatedLine &Line = *I;
> + if (!fitsIntoLimit((I + 1)->First, Limit))
> + return;
> + if ((I + 1)->First.is(tok::kw_if) || (I + 1)->First.Type == TT_LineComment)
> + return;
> + // Only inline simple if's (no nested if or else).
> + if (I + 2 != E && (I + 2)->First.is(tok::kw_else))
> + return;
> + join(Line, *(++I));
> + }
> +
> + void tryMergeSimpleBlock(std::vector<AnnotatedLine>::iterator &I,
> + std::vector<AnnotatedLine>::iterator E,
> + unsigned Limit){
> // Check that we still have three lines and they fit into the limit.
> - if (I + 1 == E || I + 2 == E)
> - return true;
> - unsigned LengthLine2 = 0;
> - unsigned LengthLine3 = 0;
> - if (!fitsIntoLimit((I + 1)->First, Limit, &LengthLine2) ||
> - !fitsIntoLimit((I + 2)->First, Limit, &LengthLine3))
> - return true;
> - if (LengthLine1 + LengthLine2 + LengthLine3 + 2 > Limit) // Two spaces.
> - return true;
> + if (I + 2 == E || !nextTwoLinesFitInto(I, Limit))
> + return;
>
> // First, check that the current line allows merging. This is the case if
> // we're not in a control flow statement and the last token is an opening
> // brace.
> - AnnotatedLine& Line = *I;
> + AnnotatedLine &Line = *I;
> bool AllowedTokens =
> Line.First.isNot(tok::kw_if) && Line.First.isNot(tok::kw_while) &&
> Line.First.isNot(tok::kw_do) && Line.First.isNot(tok::r_brace) &&
> @@ -1434,17 +1452,17 @@
> // This gets rid of all ObjC @ keywords and methods.
> Line.First.isNot(tok::at) && Line.First.isNot(tok::minus) &&
> Line.First.isNot(tok::plus);
> - if (Line.Last->isNot(tok::l_brace) || !AllowedTokens)
> - return true;
> + if (!AllowedTokens)
> + return;
>
> // Second, check that the next line does not contain any braces - if it
> // does, readability declines when putting it into a single line.
> const AnnotatedToken *Tok = &(I + 1)->First;
> if ((I + 1)->Last->Type == TT_LineComment || Tok->MustBreakBefore)
> - return true;
> + return;
> do {
> if (Tok->is(tok::l_brace) || Tok->is(tok::r_brace))
> - return true;
> + return;
> Tok = Tok->Children.empty() ? NULL : &Tok->Children.back();
> } while (Tok != NULL);
>
> @@ -1452,7 +1470,7 @@
> Tok = &(I + 2)->First;
> if (!Tok->Children.empty() || Tok->isNot(tok::r_brace) ||
> Tok->MustBreakBefore)
> - return true;
> + return;
>
> // If the merged line fits, we use that instead and skip the next two lines.
> Line.Last->Children.push_back((I + 1)->First);
> @@ -1464,7 +1482,16 @@
> join(Line, *(I + 1));
> join(Line, *(I + 2));
> I += 2;
> - return true;
> + }
> +
> + bool nextTwoLinesFitInto(std::vector<AnnotatedLine>::iterator I,
> + unsigned Limit) {
> + unsigned LengthLine1 = 0;
> + unsigned LengthLine2 = 0;
> + if (!fitsIntoLimit((I + 1)->First, Limit, &LengthLine1) ||
> + !fitsIntoLimit((I + 2)->First, Limit, &LengthLine2))
> + return false;
> + return LengthLine1 + LengthLine2 + 1 <= Limit; // One space.
> }
>
> void join(AnnotatedLine &A, const AnnotatedLine &B) {
>
> Modified: cfe/trunk/unittests/Format/FormatTest.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=172413&r1=172412&r2=172413&view=diff
> ==============================================================================
> --- cfe/trunk/unittests/Format/FormatTest.cpp (original)
> +++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Jan 14 08:14:23 2013
> @@ -130,12 +130,14 @@
> //===----------------------------------------------------------------------===//
>
> TEST_F(FormatTest, FormatIfWithoutCompountStatement) {
> - verifyFormat("if (true)\n f();\ng();");
> - verifyFormat("if (a)\n if (b)\n if (c)\n g();\nh();");
> + verifyFormat("if (true) f();\ng();");
> + verifyFormat("if (a)\n if (b)\n if (c) g();\nh();");
> verifyFormat("if (a)\n if (b) {\n f();\n }\ng();");
> verifyFormat("if (a)\n"
> " // comment\n"
> " f();");
> + verifyFormat("if (a) return;", getLLVMStyleWithColumns(14));
> + verifyFormat("if (a)\n return;", getLLVMStyleWithColumns(13));
> }
>
> TEST_F(FormatTest, ParseIfElse) {
> @@ -152,8 +154,7 @@
> verifyFormat("if (true)\n"
> " if (true)\n"
> " if (true) {\n"
> - " if (true)\n"
> - " f();\n"
> + " if (true) f();\n"
> " } else {\n"
> " g();\n"
> " }\n"
> @@ -1454,8 +1455,7 @@
>
> verifyFormat("@implementation Foo\n"
> "+ (id)init {\n"
> - " if (true)\n"
> - " return nil;\n"
> + " if (true) return nil;\n"
> "}\n"
> "// Look, a comment!\n"
> "- (int)answerWith:(int)i {\n"
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list