[cfe-commits] r172413 - in /cfe/trunk: lib/Format/Format.cpp unittests/Format/FormatTest.cpp
Douglas Gregor
dgregor at apple.com
Mon Jan 14 07:47:09 PST 2013
On Jan 14, 2013, at 7:43 AM, Daniel Jasper <djasper at google.com> wrote:
> Sure. I'll add this as an option and add a default style for Chromium (we have other stuff we want to configure according to the Chromium style guide).
Thank you!
> Douglas: Should I set this to break or no break for the default LLVM Style?
Unfortunately, the style guide is itself inconsistent. Personally, I'd prefer to break there.
- Doug
>
> On Mon, Jan 14, 2013 at 3:52 PM, Douglas Gregor <dgregor at apple.com> wrote:
>
> 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130114/c8c98afc/attachment.html>
More information about the cfe-commits
mailing list