<div dir="ltr">I thought about that, but it's really hard to a) come up with decent names for the flags and b) there's already precedence for naming them like this</div><div class="gmail_extra"><br><br><div class="gmail_quote">
On Tue, May 14, 2013 at 9:46 PM, Alexander Kornienko <span dir="ltr"><<a href="mailto:alexfh@google.com" target="_blank">alexfh@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><p dir="ltr">How about decomposing brace breaking styles to a set of orthogonal flags (break before braces in: functions, class definitions, namespaces, after if/for/while, before else, after else, may be something else). Then it would make code a bit clearer and allow specifying other styles not listed here (e.g., the one used by Microsoft).</p>
<div><div class="h5">
<div class="gmail_quote">On May 13, 2013 2:54 PM, "Manuel Klimek" <<a href="mailto:klimek@google.com" target="_blank">klimek@google.com</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Author: klimek<br>
Date: Mon May 13 07:51:40 2013<br>
New Revision: 181700<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=181700&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=181700&view=rev</a><br>
Log:<br>
Implements brace breaking styles.<br>
<br>
We now support "Linux" and "Stroustrup" brace breaking styles, which<br>
gets us one step closer to support formatting WebKit, KDE & Linux code.<br>
<br>
Linux brace breaking style:<br>
namespace a<br>
{<br>
class A<br>
{<br>
void f()<br>
{<br>
if (x) {<br>
f();<br>
} else {<br>
g();<br>
}<br>
}<br>
}<br>
}<br>
<br>
Stroustrup brace breaking style:<br>
namespace a {<br>
class A {<br>
void f()<br>
{<br>
if (x) {<br>
f();<br>
} else {<br>
g();<br>
}<br>
}<br>
}<br>
}<br>
<br>
Modified:<br>
cfe/trunk/include/clang/Format/Format.h<br>
cfe/trunk/lib/Format/Format.cpp<br>
cfe/trunk/lib/Format/UnwrappedLineParser.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=181700&r1=181699&r2=181700&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=181700&r1=181699&r2=181700&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Format/Format.h (original)<br>
+++ cfe/trunk/include/clang/Format/Format.h Mon May 13 07:51:40 2013<br>
@@ -101,6 +101,20 @@ struct FormatStyle {<br>
/// tab characters.<br>
bool UseTab;<br>
<br>
+ /// \brief Different ways to attach braces to their surrounding context.<br>
+ enum BraceBreakingStyle {<br>
+ /// Always attach braces to surrounding context.<br>
+ BS_Attach,<br>
+ /// Like \c Attach, but break before braces on function, namespace and<br>
+ /// class definitions.<br>
+ BS_Linux,<br>
+ /// Like \c Attach, but break before function definitions.<br>
+ BS_Stroustrup<br>
+ };<br>
+<br>
+ /// \brief The brace breaking style to use.<br>
+ BraceBreakingStyle BreakBeforeBraces;<br>
+<br>
bool operator==(const FormatStyle &R) const {<br>
return AccessModifierOffset == R.AccessModifierOffset &&<br>
AlignEscapedNewlinesLeft == R.AlignEscapedNewlinesLeft &&<br>
@@ -122,7 +136,8 @@ struct FormatStyle {<br>
SpacesBeforeTrailingComments == R.SpacesBeforeTrailingComments &&<br>
Standard == R.Standard &&<br>
IndentWidth == R.IndentWidth &&<br>
- UseTab == R.UseTab;<br>
+ UseTab == R.UseTab &&<br>
+ BreakBeforeBraces == R.BreakBeforeBraces;<br>
}<br>
<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=181700&r1=181699&r2=181700&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=181700&r1=181699&r2=181700&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Format/Format.cpp (original)<br>
+++ cfe/trunk/lib/Format/Format.cpp Mon May 13 07:51:40 2013<br>
@@ -36,11 +36,21 @@ namespace llvm {<br>
namespace yaml {<br>
template <><br>
struct ScalarEnumerationTraits<clang::format::FormatStyle::LanguageStandard> {<br>
- static void enumeration(IO &io,<br>
- clang::format::FormatStyle::LanguageStandard &value) {<br>
- io.enumCase(value, "C++03", clang::format::FormatStyle::LS_Cpp03);<br>
- io.enumCase(value, "C++11", clang::format::FormatStyle::LS_Cpp11);<br>
- io.enumCase(value, "Auto", clang::format::FormatStyle::LS_Auto);<br>
+ static void enumeration(IO &IO,<br>
+ clang::format::FormatStyle::LanguageStandard &Value) {<br>
+ IO.enumCase(Value, "C++03", clang::format::FormatStyle::LS_Cpp03);<br>
+ IO.enumCase(Value, "C++11", clang::format::FormatStyle::LS_Cpp11);<br>
+ IO.enumCase(Value, "Auto", clang::format::FormatStyle::LS_Auto);<br>
+ }<br>
+};<br>
+<br>
+template<><br>
+struct ScalarEnumerationTraits<clang::format::FormatStyle::BraceBreakingStyle> {<br>
+ static void<br>
+ enumeration(IO &IO, clang::format::FormatStyle::BraceBreakingStyle &Value) {<br>
+ IO.enumCase(Value, "Attach", clang::format::FormatStyle::BS_Attach);<br>
+ IO.enumCase(Value, "Linux", clang::format::FormatStyle::BS_Linux);<br>
+ IO.enumCase(Value, "Stroustrup", clang::format::FormatStyle::BS_Stroustrup);<br>
}<br>
};<br>
<br>
@@ -87,6 +97,7 @@ template <> struct MappingTraits<clang::<br>
IO.mapOptional("Standard", Style.Standard);<br>
IO.mapOptional("IndentWidth", Style.IndentWidth);<br>
IO.mapOptional("UseTab", Style.UseTab);<br>
+ IO.mapOptional("BreakBeforeBraces", Style.BreakBeforeBraces);<br>
}<br>
};<br>
}<br>
@@ -115,6 +126,7 @@ FormatStyle getLLVMStyle() {<br>
LLVMStyle.Standard = FormatStyle::LS_Cpp03;<br>
LLVMStyle.IndentWidth = 2;<br>
LLVMStyle.UseTab = false;<br>
+ LLVMStyle.BreakBeforeBraces = FormatStyle::BS_Attach;<br>
return LLVMStyle;<br>
}<br>
<br>
@@ -138,6 +150,7 @@ FormatStyle getGoogleStyle() {<br>
GoogleStyle.Standard = FormatStyle::LS_Auto;<br>
GoogleStyle.IndentWidth = 2;<br>
GoogleStyle.UseTab = false;<br>
+ GoogleStyle.BreakBeforeBraces = FormatStyle::BS_Attach;<br>
return GoogleStyle;<br>
}<br>
<br>
<br>
Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp?rev=181700&r1=181699&r2=181700&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp?rev=181700&r1=181699&r2=181700&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp (original)<br>
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp Mon May 13 07:51:40 2013<br>
@@ -402,6 +402,10 @@ void UnwrappedLineParser::parseStructura<br>
// structural element.<br>
// FIXME: Figure out cases where this is not true, and add projections for<br>
// them (the one we know is missing are lambdas).<br>
+ if (Style.BreakBeforeBraces == FormatStyle::BS_Linux ||<br>
+ Style.BreakBeforeBraces == FormatStyle::BS_Stroustrup)<br>
+ addUnwrappedLine();<br>
+<br>
parseBlock(/*MustBeDeclaration=*/ false);<br>
addUnwrappedLine();<br>
return;<br>
@@ -577,6 +581,9 @@ void UnwrappedLineParser::parseNamespace<br>
if (<a href="http://FormatTok.Tok.is" target="_blank">FormatTok.Tok.is</a>(tok::identifier))<br>
nextToken();<br>
if (<a href="http://FormatTok.Tok.is" target="_blank">FormatTok.Tok.is</a>(tok::l_brace)) {<br>
+ if (Style.BreakBeforeBraces == FormatStyle::BS_Linux)<br>
+ addUnwrappedLine();<br>
+<br>
parseBlock(/*MustBeDeclaration=*/ true, 0);<br>
// Munch the semicolon after a namespace. This is more common than one would<br>
// think. Puttin the semicolon into its own line is very ugly.<br>
@@ -751,8 +758,12 @@ void UnwrappedLineParser::parseRecord()<br>
}<br>
}<br>
}<br>
- if (<a href="http://FormatTok.Tok.is" target="_blank">FormatTok.Tok.is</a>(tok::l_brace))<br>
+ if (<a href="http://FormatTok.Tok.is" target="_blank">FormatTok.Tok.is</a>(tok::l_brace)) {<br>
+ if (Style.BreakBeforeBraces == FormatStyle::BS_Linux)<br>
+ addUnwrappedLine();<br>
+<br>
parseBlock(/*MustBeDeclaration=*/ true);<br>
+ }<br>
// We fall through to parsing a structural element afterwards, so<br>
// class A {} n, m;<br>
// will end up in one unwrapped line.<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=181700&r1=181699&r2=181700&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=181700&r1=181699&r2=181700&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)<br>
+++ cfe/trunk/unittests/Format/FormatTest.cpp Mon May 13 07:51:40 2013<br>
@@ -4037,6 +4037,42 @@ TEST_F(FormatTest, ConfigurableUseOfTab)<br>
Tab);<br>
}<br>
<br>
+TEST_F(FormatTest, LinuxBraceBreaking) {<br>
+ FormatStyle BreakBeforeBrace = getLLVMStyle();<br>
+ BreakBeforeBrace.BreakBeforeBraces = FormatStyle::BS_Linux;<br>
+ verifyFormat("namespace a\n"<br>
+ "{\n"<br>
+ "class A\n"<br>
+ "{\n"<br>
+ " void f()\n"<br>
+ " {\n"<br>
+ " if (true) {\n"<br>
+ " a();\n"<br>
+ " b();\n"<br>
+ " }\n"<br>
+ " }\n"<br>
+ "}\n"<br>
+ "}",<br>
+ BreakBeforeBrace);<br>
+}<br>
+<br>
+TEST_F(FormatTest, StroustrupBraceBreaking) {<br>
+ FormatStyle BreakBeforeBrace = getLLVMStyle();<br>
+ BreakBeforeBrace.BreakBeforeBraces = FormatStyle::BS_Stroustrup;<br>
+ verifyFormat("namespace a {\n"<br>
+ "class A {\n"<br>
+ " void f()\n"<br>
+ " {\n"<br>
+ " if (true) {\n"<br>
+ " a();\n"<br>
+ " b();\n"<br>
+ " }\n"<br>
+ " }\n"<br>
+ "}\n"<br>
+ "}",<br>
+ BreakBeforeBrace);<br>
+}<br>
+<br>
bool allStylesEqual(ArrayRef<FormatStyle> Styles) {<br>
for (size_t i = 1; i < Styles.size(); ++i)<br>
if (!(Styles[0] == Styles[i]))<br>
@@ -4113,6 +4149,14 @@ TEST_F(FormatTest, ParsesConfiguration)<br>
CHECK_PARSE("BasedOnStyle: LLVM", ColumnLimit, BaseStyle.ColumnLimit);<br>
CHECK_PARSE("BasedOnStyle: LLVM\nColumnLimit: 1234", ColumnLimit, 1234u);<br>
<br>
+ Style.BreakBeforeBraces = FormatStyle::BS_Stroustrup;<br>
+ CHECK_PARSE("BreakBeforeBraces: Attach", BreakBeforeBraces,<br>
+ FormatStyle::BS_Attach);<br>
+ CHECK_PARSE("BreakBeforeBraces: Linux", BreakBeforeBraces,<br>
+ FormatStyle::BS_Linux);<br>
+ CHECK_PARSE("BreakBeforeBraces: Stroustrup", BreakBeforeBraces,<br>
+ FormatStyle::BS_Stroustrup);<br>
+<br>
#undef CHECK_PARSE<br>
#undef CHECK_PARSE_BOOL<br>
}<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div>
</div></div></div>
</blockquote></div><br></div>