[cfe-commits] r172431 - in /cfe/trunk: include/clang/Format/Format.h lib/Format/Format.cpp unittests/Format/FormatTest.cpp
Chandler Carruth
chandlerc at google.com
Wed Jan 16 02:06:46 PST 2013
On Mon, Jan 14, 2013 at 8:44 AM, Nico Weber <thakis at chromium.org> wrote:
> On Mon, Jan 14, 2013 at 8:24 AM, Daniel Jasper <djasper at google.com> wrote:
> > Author: djasper
> > Date: Mon Jan 14 10:24:39 2013
> > New Revision: 172431
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=172431&view=rev
> > Log:
> > Make single-line if statements optional.
>
> Thanks!
>
> Out of curiosity, do you know how common this style is in Google's
> internal code? I haven't seen it much.
>
For what it's worth, despite having written a lot of single-line if
statements, I don't think this is worth options in the formatter. I think
consistency and simplicity should trump here, and since there trivially
exist cases that can't be on a single line, it seems of very low value to
support folding onto a single line....
My two cents, and it applies both to Google's internal coding style, and my
feelings on LLVM's coding style.
-Chandler
>
> >
> > Now, "if (a) return;" is only allowed, if this option is set.
> >
> > Also add a Chromium style which is currently identical to Google style
> > except for this option.
> >
> > Modified:
> > cfe/trunk/include/clang/Format/Format.h
> > cfe/trunk/lib/Format/Format.cpp
> > cfe/trunk/unittests/Format/FormatTest.cpp
> >
> > Modified: cfe/trunk/include/clang/Format/Format.h
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=172431&r1=172430&r2=172431&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/include/clang/Format/Format.h (original)
> > +++ cfe/trunk/include/clang/Format/Format.h Mon Jan 14 10:24:39 2013
> > @@ -61,6 +61,9 @@
> > /// initializer on its own line.
> > bool ConstructorInitializerAllOnOneLineOrOnePerLine;
> >
> > + /// \brief If true, "if (a) return;" can be put on a single line.
> > + bool AllowShortIfStatementsOnASingleLine;
> > +
> > /// \brief Add a space in front of an Objective-C protocol list, i.e.
> use
> > /// Foo <Protocol> instead of Foo<Protocol>.
> > bool ObjCSpaceBeforeProtocolList;
> > @@ -78,6 +81,10 @@
> > /// http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml.
> > FormatStyle getGoogleStyle();
> >
> > +/// \brief Returns a format style complying with Chromium's style guide:
> > +/// http://www.chromium.org/developers/coding-style.
> > +FormatStyle getChromiumStyle();
> > +
> > /// \brief Reformats the given \p Ranges in the token stream coming out
> of
> > /// \c Lex.
> > ///
> >
> > Modified: cfe/trunk/lib/Format/Format.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=172431&r1=172430&r2=172431&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/lib/Format/Format.cpp (original)
> > +++ cfe/trunk/lib/Format/Format.cpp Mon Jan 14 10:24:39 2013
> > @@ -117,6 +117,7 @@
> > LLVMStyle.IndentCaseLabels = false;
> > LLVMStyle.SpacesBeforeTrailingComments = 1;
> > LLVMStyle.ConstructorInitializerAllOnOneLineOrOnePerLine = false;
> > + LLVMStyle.AllowShortIfStatementsOnASingleLine = false;
> > LLVMStyle.ObjCSpaceBeforeProtocolList = true;
> > LLVMStyle.ObjCSpaceBeforeReturnType = true;
> > return LLVMStyle;
> > @@ -132,11 +133,18 @@
> > GoogleStyle.IndentCaseLabels = true;
> > GoogleStyle.SpacesBeforeTrailingComments = 2;
> > GoogleStyle.ConstructorInitializerAllOnOneLineOrOnePerLine = true;
> > + GoogleStyle.AllowShortIfStatementsOnASingleLine = true;
> > GoogleStyle.ObjCSpaceBeforeProtocolList = false;
> > GoogleStyle.ObjCSpaceBeforeReturnType = false;
> > return GoogleStyle;
> > }
> >
> > +FormatStyle getChromiumStyle() {
> > + FormatStyle ChromiumStyle = getGoogleStyle();
> > + ChromiumStyle.AllowShortIfStatementsOnASingleLine = false;
> > + return ChromiumStyle;
> > +}
> > +
> > struct OptimizationParameters {
> > unsigned PenaltyIndentLevel;
> > unsigned PenaltyLevelDecrease;
> > @@ -1441,6 +1449,8 @@
> > void tryMergeSimpleIf(std::vector<AnnotatedLine>::iterator &I,
> > std::vector<AnnotatedLine>::iterator E,
> > unsigned Limit) {
> > + if (!Style.AllowShortIfStatementsOnASingleLine)
> > + return;
> > AnnotatedLine &Line = *I;
> > if (!fitsIntoLimit((I + 1)->First, Limit))
> > return;
> >
> > Modified: cfe/trunk/unittests/Format/FormatTest.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=172431&r1=172430&r2=172431&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/unittests/Format/FormatTest.cpp (original)
> > +++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Jan 14 10:24:39 2013
> > @@ -78,6 +78,12 @@
> > return Style;
> > }
> >
> > + FormatStyle getGoogleStyleWithColumns(unsigned ColumnLimit) {
> > + FormatStyle Style = getGoogleStyle();
> > + Style.ColumnLimit = ColumnLimit;
> > + return Style;
> > + }
> > +
> > void verifyFormat(llvm::StringRef Code,
> > const FormatStyle &Style = getLLVMStyle()) {
> > EXPECT_EQ(Code.str(), format(messUp(Code), Style));
> > @@ -130,16 +136,16 @@
> >
> //===----------------------------------------------------------------------===//
> >
> > TEST_F(FormatTest, FormatIfWithoutCompountStatement) {
> > - verifyFormat("if (true) f();\ng();");
> > - verifyFormat("if (a)\n if (b)\n if (c) g();\nh();");
> > + verifyFormat("if (true)\n f();\ng();");
> > + verifyFormat("if (a)\n if (b)\n if (c)\n 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));
> > + verifyGoogleFormat("if (a)\n"
> > + " // comment\n"
> > + " f();");
> > + verifyFormat("if (a) return;", getGoogleStyleWithColumns(14));
> > + verifyFormat("if (a)\n return;", getGoogleStyleWithColumns(13));
> > verifyFormat("if (aaaaaaaaa)\n"
> > - " return;", getLLVMStyleWithColumns(14));
> > + " return;", getGoogleStyleWithColumns(14));
> > }
> >
> > TEST_F(FormatTest, ParseIfElse) {
> > @@ -156,7 +162,8 @@
> > verifyFormat("if (true)\n"
> > " if (true)\n"
> > " if (true) {\n"
> > - " if (true) f();\n"
> > + " if (true)\n"
> > + " f();\n"
> > " } else {\n"
> > " g();\n"
> > " }\n"
> > @@ -1461,7 +1468,8 @@
> >
> > verifyFormat("@implementation Foo\n"
> > "+ (id)init {\n"
> > - " if (true) return nil;\n"
> > + " if (true)\n"
> > + " 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
> _______________________________________________
> 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/20130116/077f1204/attachment.html>
More information about the cfe-commits
mailing list