<div dir="ltr"><div class="gmail_default" style>On Mon, Jan 14, 2013 at 8:44 AM, Nico Weber <span dir="ltr"><<a href="mailto:thakis@chromium.org" target="_blank" class="cremed">thakis@chromium.org</a>></span> wrote:<br>
</div><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">On Mon, Jan 14, 2013 at 8:24 AM, Daniel Jasper <<a href="mailto:djasper@google.com" class="cremed">djasper@google.com</a>> wrote:<br>
> Author: djasper<br>
> Date: Mon Jan 14 10:24:39 2013<br>
> New Revision: 172431<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=172431&view=rev" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project?rev=172431&view=rev</a><br>
> Log:<br>
> Make single-line if statements optional.<br>
<br>
</div>Thanks!<br>
<br>
Out of curiosity, do you know how common this style is in Google's<br>
internal code? I haven't seen it much.<br></blockquote><div><br></div><div style>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....</div>
<div style><br></div><div style>My two cents, and it applies both to Google's internal coding style, and my feelings on LLVM's coding style.</div><div style>-Chandler</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5"><br>
><br>
> Now, "if (a) return;" is only allowed, if this option is set.<br>
><br>
> Also add a Chromium style which is currently identical to Google style<br>
> except for this option.<br>
><br>
> Modified:<br>
> cfe/trunk/include/clang/Format/Format.h<br>
> cfe/trunk/lib/Format/Format.cpp<br>
> cfe/trunk/unittests/Format/FormatTest.cpp<br>
><br>
> Modified: cfe/trunk/include/clang/Format/Format.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=172431&r1=172430&r2=172431&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=172431&r1=172430&r2=172431&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/include/clang/Format/Format.h (original)<br>
> +++ cfe/trunk/include/clang/Format/Format.h Mon Jan 14 10:24:39 2013<br>
> @@ -61,6 +61,9 @@<br>
> /// initializer on its own line.<br>
> bool ConstructorInitializerAllOnOneLineOrOnePerLine;<br>
><br>
> + /// \brief If true, "if (a) return;" can be put on a single line.<br>
> + bool AllowShortIfStatementsOnASingleLine;<br>
> +<br>
> /// \brief Add a space in front of an Objective-C protocol list, i.e. use<br>
> /// Foo <Protocol> instead of Foo<Protocol>.<br>
> bool ObjCSpaceBeforeProtocolList;<br>
> @@ -78,6 +81,10 @@<br>
> /// <a href="http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml" target="_blank" class="cremed">http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml</a>.<br>
> FormatStyle getGoogleStyle();<br>
><br>
> +/// \brief Returns a format style complying with Chromium's style guide:<br>
> +/// <a href="http://www.chromium.org/developers/coding-style" target="_blank" class="cremed">http://www.chromium.org/developers/coding-style</a>.<br>
> +FormatStyle getChromiumStyle();<br>
> +<br>
> /// \brief Reformats the given \p Ranges in the token stream coming out of<br>
> /// \c Lex.<br>
> ///<br>
><br>
> Modified: cfe/trunk/lib/Format/Format.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=172431&r1=172430&r2=172431&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=172431&r1=172430&r2=172431&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/Format/Format.cpp (original)<br>
> +++ cfe/trunk/lib/Format/Format.cpp Mon Jan 14 10:24:39 2013<br>
> @@ -117,6 +117,7 @@<br>
> LLVMStyle.IndentCaseLabels = false;<br>
> LLVMStyle.SpacesBeforeTrailingComments = 1;<br>
> LLVMStyle.ConstructorInitializerAllOnOneLineOrOnePerLine = false;<br>
> + LLVMStyle.AllowShortIfStatementsOnASingleLine = false;<br>
> LLVMStyle.ObjCSpaceBeforeProtocolList = true;<br>
> LLVMStyle.ObjCSpaceBeforeReturnType = true;<br>
> return LLVMStyle;<br>
> @@ -132,11 +133,18 @@<br>
> GoogleStyle.IndentCaseLabels = true;<br>
> GoogleStyle.SpacesBeforeTrailingComments = 2;<br>
> GoogleStyle.ConstructorInitializerAllOnOneLineOrOnePerLine = true;<br>
> + GoogleStyle.AllowShortIfStatementsOnASingleLine = true;<br>
> GoogleStyle.ObjCSpaceBeforeProtocolList = false;<br>
> GoogleStyle.ObjCSpaceBeforeReturnType = false;<br>
> return GoogleStyle;<br>
> }<br>
><br>
> +FormatStyle getChromiumStyle() {<br>
> + FormatStyle ChromiumStyle = getGoogleStyle();<br>
> + ChromiumStyle.AllowShortIfStatementsOnASingleLine = false;<br>
> + return ChromiumStyle;<br>
> +}<br>
> +<br>
> struct OptimizationParameters {<br>
> unsigned PenaltyIndentLevel;<br>
> unsigned PenaltyLevelDecrease;<br>
> @@ -1441,6 +1449,8 @@<br>
> void tryMergeSimpleIf(std::vector<AnnotatedLine>::iterator &I,<br>
> std::vector<AnnotatedLine>::iterator E,<br>
> unsigned Limit) {<br>
> + if (!Style.AllowShortIfStatementsOnASingleLine)<br>
> + return;<br>
> AnnotatedLine &Line = *I;<br>
> if (!fitsIntoLimit((I + 1)->First, Limit))<br>
> return;<br>
><br>
> Modified: cfe/trunk/unittests/Format/FormatTest.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=172431&r1=172430&r2=172431&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=172431&r1=172430&r2=172431&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/unittests/Format/FormatTest.cpp (original)<br>
> +++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Jan 14 10:24:39 2013<br>
> @@ -78,6 +78,12 @@<br>
> return Style;<br>
> }<br>
><br>
> + FormatStyle getGoogleStyleWithColumns(unsigned ColumnLimit) {<br>
> + FormatStyle Style = getGoogleStyle();<br>
> + Style.ColumnLimit = ColumnLimit;<br>
> + return Style;<br>
> + }<br>
> +<br>
> void verifyFormat(llvm::StringRef Code,<br>
> const FormatStyle &Style = getLLVMStyle()) {<br>
> EXPECT_EQ(Code.str(), format(messUp(Code), Style));<br>
> @@ -130,16 +136,16 @@<br>
> //===----------------------------------------------------------------------===//<br>
><br>
> TEST_F(FormatTest, FormatIfWithoutCompountStatement) {<br>
> - verifyFormat("if (true) f();\ng();");<br>
> - verifyFormat("if (a)\n if (b)\n if (c) g();\nh();");<br>
> + verifyFormat("if (true)\n f();\ng();");<br>
> + verifyFormat("if (a)\n if (b)\n if (c)\n g();\nh();");<br>
> verifyFormat("if (a)\n if (b) {\n f();\n }\ng();");<br>
> - verifyFormat("if (a)\n"<br>
> - " // comment\n"<br>
> - " f();");<br>
> - verifyFormat("if (a) return;", getLLVMStyleWithColumns(14));<br>
> - verifyFormat("if (a)\n return;", getLLVMStyleWithColumns(13));<br>
> + verifyGoogleFormat("if (a)\n"<br>
> + " // comment\n"<br>
> + " f();");<br>
> + verifyFormat("if (a) return;", getGoogleStyleWithColumns(14));<br>
> + verifyFormat("if (a)\n return;", getGoogleStyleWithColumns(13));<br>
> verifyFormat("if (aaaaaaaaa)\n"<br>
> - " return;", getLLVMStyleWithColumns(14));<br>
> + " return;", getGoogleStyleWithColumns(14));<br>
> }<br>
><br>
> TEST_F(FormatTest, ParseIfElse) {<br>
> @@ -156,7 +162,8 @@<br>
> verifyFormat("if (true)\n"<br>
> " if (true)\n"<br>
> " if (true) {\n"<br>
> - " if (true) f();\n"<br>
> + " if (true)\n"<br>
> + " f();\n"<br>
> " } else {\n"<br>
> " g();\n"<br>
> " }\n"<br>
> @@ -1461,7 +1468,8 @@<br>
><br>
> verifyFormat("@implementation Foo\n"<br>
> "+ (id)init {\n"<br>
> - " if (true) return nil;\n"<br>
> + " if (true)\n"<br>
> + " return nil;\n"<br>
> "}\n"<br>
> "// Look, a comment!\n"<br>
> "- (int)answerWith:(int)i {\n"<br>
><br>
><br>
> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@cs.uiuc.edu" class="cremed">cfe-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank" class="cremed">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" class="cremed">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank" class="cremed">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</div></div></blockquote></div><br></div></div>