On Mon, Jan 14, 2013 at 8:44 AM, Nico Weber <[email protected]> wrote:
> On Mon, Jan 14, 2013 at 8:24 AM, Daniel Jasper <[email protected]> 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 > > [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 >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
