<div dir="ltr">By default, a line break somewhere within a statement or a declaration is a continuation and is indented by whatever is configured in ContinuationIndentWidth. Not indenting when wrapping after the template declaration or return type is special cased. Take look at the function getNewLineColumn in ContinuationIndenter.cpp, the special case here is the if at line 546.<div><br></div><div>As for the patch itself, how important is it not to break after inline in declarations? It seems to add unnecessary complexity and is wrong as implemented (won't break in "inline void f() { // comment").</div><div><br></div><div>Cheers,</div><div>Daniel</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Sat, Dec 20, 2014 at 6:51 PM, Michael Hofmann <span dir="ltr"><<a href="mailto:kmhofmann@gmail.com" target="_blank">kmhofmann@gmail.com</a>></span> wrote:<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I am trying to add a style option to clang-format, in order to get one<br>
step closer to mapping our coding style to a .clang-format file.<br>
Our guidelines mandate that for function definitions and non-member<br>
function declarations, the following elements (if they exist) need to<br>
be on a separate line each:<br>
- template specification<br>
- inline keyword [only legal in function definitions]<br>
- function return type<br>
- function name (and arguments, potentially on separate lines)<br>
<br>
This is to increase discoverability of the return type as well as the<br>
function name. Consider a complex function definition such as:<br>
template <typename BidirIt, typename IndexType><br>
inline<br>
std::iterator_traits<BidirIt>::value_type<br>
Foo<ScalarType>::bar(BidirIt begin, BidirIt end, IndexType index)<br>
{ ...<br>
<br>
With the currently available clang-format options<br>
AlwaysBreakAfterDefinitionReturnType: true<br>
AlwaysBreakTemplateDeclarations: true<br>
one can get as far as:<br>
<br>
template <typename BidirIt, typename IndexType><br>
inline std::iterator_traits<BidirIt>::value_type<br>
Foo<ScalarType>::bar(BidirIt begin, BidirIt end, IndexType index)<br>
{ ...<br>
<br>
but there is no good reason why the inline keyword should reside on<br>
the same line as the return type (they are not logically linked in any<br>
way). Hence our guideline to have it on a separate line, which makes<br>
spotting the return type easier, too.<br>
<br>
I have tried to add a new boolean option<br>
'AlwaysBreakAfterDefinitionInline', somewhat mimicking the<br>
implementation of 'AlwaysBreakAfterDefinitionReturnType'. The<br>
intention is to break after the 'inline' keyword in a definition, but<br>
not in a declaration (where the keyword is not supposed to be anyway).<br>
See the patch below, which appears to do the right thing using our<br>
coding conventions.<br>
<br>
However, the new unit test, which I added as a copy-&-paste from the<br>
AlwaysBreakAfterDefinitionReturnType test, is broken. clang-format (in<br>
combination with LLVM style) seems to indent the lines right after the<br>
inline keyword, which gives the following error:<br>
<br>
Value of: format(test::messUp(Code), Style)<br>
Actual: "inline\n const char *\n f(void) {\n return<br>
\"\";\n}\ninline const char *bar(void);\n"<br>
Expected: Code.str()<br>
Which is: "inline\nconst char *f(void) {\n return \"\";\n}\ninline<br>
const char *bar(void);\n"<br>
...<br>
<br>
Since this is my first excursion into the clang-format codebase, it is<br>
not clear to me how this line breaking is linked to indentation.<br>
Can someone help me out there? Or ideally implement the option correctly? :)<br>
<br>
Thanks,<br>
Michael<br>
<br>
---<br>
<br>
Index: include/clang/Format/Format.h<br>
===================================================================<br>
--- include/clang/Format/Format.h (revision 224222)<br>
+++ include/clang/Format/Format.h (working copy)<br>
@@ -275,6 +275,10 @@<br>
/// template declaration.<br>
bool AlwaysBreakTemplateDeclarations;<br>
<br>
+ /// \brief If \c true, always break after the inline keyword of a function<br>
+ /// definition.<br>
+ bool AlwaysBreakAfterDefinitionInline;<br>
+<br>
/// \brief If \c true, always break before multiline string literals.<br>
bool AlwaysBreakBeforeMultilineStrings;<br>
<br>
@@ -426,6 +430,8 @@<br>
R.AlwaysBreakAfterDefinitionReturnType &&<br>
AlwaysBreakTemplateDeclarations ==<br>
R.AlwaysBreakTemplateDeclarations &&<br>
+ AlwaysBreakAfterDefinitionInline ==<br>
+ R.AlwaysBreakAfterDefinitionInline &&<br>
AlwaysBreakBeforeMultilineStrings ==<br>
R.AlwaysBreakBeforeMultilineStrings &&<br>
BinPackParameters == R.BinPackParameters &&<br>
Index: lib/Format/Format.cpp<br>
===================================================================<br>
--- lib/Format/Format.cpp (revision 224222)<br>
+++ lib/Format/Format.cpp (working copy)<br>
@@ -192,6 +192,8 @@<br>
Style.AlwaysBreakAfterDefinitionReturnType);<br>
IO.mapOptional("AlwaysBreakTemplateDeclarations",<br>
Style.AlwaysBreakTemplateDeclarations);<br>
+ IO.mapOptional("AlwaysBreakAfterDefinitionInline",<br>
+ Style.AlwaysBreakAfterDefinitionInline);<br>
IO.mapOptional("AlwaysBreakBeforeMultilineStrings",<br>
Style.AlwaysBreakBeforeMultilineStrings);<br>
IO.mapOptional("BreakBeforeBinaryOperators",<br>
@@ -340,6 +342,7 @@<br>
LLVMStyle.AlwaysBreakAfterDefinitionReturnType = false;<br>
LLVMStyle.AlwaysBreakBeforeMultilineStrings = false;<br>
LLVMStyle.AlwaysBreakTemplateDeclarations = false;<br>
+ LLVMStyle.AlwaysBreakAfterDefinitionInline = false;<br>
LLVMStyle.BinPackParameters = true;<br>
LLVMStyle.BinPackArguments = true;<br>
LLVMStyle.BreakBeforeBinaryOperators = FormatStyle::BOS_None;<br>
Index: lib/Format/TokenAnnotator.cpp<br>
===================================================================<br>
--- lib/Format/TokenAnnotator.cpp (revision 224222)<br>
+++ lib/Format/TokenAnnotator.cpp (working copy)<br>
@@ -1409,6 +1409,11 @@<br>
// and tok::lbrace.<br>
Current->MustBreakBefore = true;<br>
<br>
+ if (Style.AlwaysBreakAfterDefinitionInline &&<br>
+ Current->Previous && Current->Previous->is(tok::kw_inline) &&<br>
+ !Line.Last->isOneOf(tok::semi, tok::comment)) // Only for definitions.<br>
+ Current->MustBreakBefore = true;<br>
+<br>
Current->CanBreakBefore =<br>
Current->MustBreakBefore || canBreakBefore(Line, *Current);<br>
unsigned ChildSize = 0;<br>
Index: unittests/Format/FormatTest.cpp<br>
===================================================================<br>
--- unittests/Format/FormatTest.cpp (revision 224222)<br>
+++ unittests/Format/FormatTest.cpp (working copy)<br>
@@ -4503,6 +4503,40 @@<br>
AfterType);<br>
}<br>
<br>
+TEST_F(FormatTest, AlwaysBreakAfterDefinitionInline) {<br>
+ FormatStyle AfterType = getLLVMStyle();<br>
+ AfterType.AlwaysBreakAfterDefinitionInline = true;<br>
+ verifyFormat("inline\n" // Break here.<br>
+ "const char *f(void) {\n"<br>
+ " return \"\";\n"<br>
+ "}\n"<br>
+ "inline const char *bar(void);\n", // No break here.<br>
+ AfterType);<br>
+ verifyFormat("template <class T>\n"<br>
+ "inline\n" // Break here.<br>
+ "T *f(T &c) {\n"<br>
+ " return NULL;\n"<br>
+ "}\n"<br>
+ "template <class T> inline T *f(T &c);\n", // No break here.<br>
+ AfterType);<br>
+ AfterType.BreakBeforeBraces = FormatStyle::BS_Stroustrup;<br>
+ verifyFormat("inline\n" // Break here.<br>
+ "const char *f(void)\n"<br>
+ "{\n"<br>
+ " return \"\";\n"<br>
+ "}\n"<br>
+ "inline const char *bar(void);\n", // No break here.<br>
+ AfterType);<br>
+ verifyFormat("template <class T>\n"<br>
+ "inline\n"<br>
+ "T *f(T &c)\n" // Break here.<br>
+ "{\n"<br>
+ " return NULL;\n"<br>
+ "}\n"<br>
+ "template <class T> inline T *f(T &c);\n", // No break here.<br>
+ AfterType);<br>
+}<br>
+<br>
TEST_F(FormatTest, AlwaysBreakBeforeMultilineStrings) {<br>
FormatStyle NoBreak = getLLVMStyle();<br>
NoBreak.AlwaysBreakBeforeMultilineStrings = false;<br>
@@ -8674,6 +8708,7 @@<br>
CHECK_PARSE_BOOL(AllowShortIfStatementsOnASingleLine);<br>
CHECK_PARSE_BOOL(AllowShortLoopsOnASingleLine);<br>
CHECK_PARSE_BOOL(AlwaysBreakAfterDefinitionReturnType);<br>
+ CHECK_PARSE_BOOL(AlwaysBreakAfterDefinitionInline);<br>
CHECK_PARSE_BOOL(AlwaysBreakTemplateDeclarations);<br>
CHECK_PARSE_BOOL(BinPackParameters);<br>
CHECK_PARSE_BOOL(BinPackArguments);<br>
_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@cs.uiuc.edu">cfe-dev@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev</a><br>
</blockquote></div></div>