[cfe-dev] clang-format: break after 'inline' in function definition

Michael Hofmann kmhofmann at gmail.com
Sat Dec 20 09:51:36 PST 2014


I am trying to add a style option to clang-format, in order to get one
step closer to mapping our coding style to a .clang-format file.
Our guidelines mandate that for function definitions and non-member
function declarations, the following elements (if they exist) need to
be on a separate line each:
- template specification
- inline keyword [only legal in function definitions]
- function return type
- function name (and arguments, potentially on separate lines)

This is to increase discoverability of the return type as well as the
function name. Consider a complex function definition such as:
template <typename BidirIt, typename IndexType>
inline
std::iterator_traits<BidirIt>::value_type
Foo<ScalarType>::bar(BidirIt begin, BidirIt end, IndexType index)
{ ...

With the currently available clang-format options
  AlwaysBreakAfterDefinitionReturnType: true
  AlwaysBreakTemplateDeclarations: true
one can get as far as:

template <typename BidirIt, typename IndexType>
inline std::iterator_traits<BidirIt>::value_type
Foo<ScalarType>::bar(BidirIt begin, BidirIt end, IndexType index)
{ ...

but there is no good reason why the inline keyword should reside on
the same line as the return type (they are not logically linked in any
way). Hence our guideline to have it on a separate line, which makes
spotting the return type easier, too.

I have tried to add a new boolean option
'AlwaysBreakAfterDefinitionInline', somewhat mimicking the
implementation of 'AlwaysBreakAfterDefinitionReturnType'. The
intention is to break after the 'inline' keyword in a definition, but
not in a declaration (where the keyword is not supposed to be anyway).
See the patch below, which appears to do the right thing using our
coding conventions.

However, the new unit test, which I added as a copy-&-paste from the
AlwaysBreakAfterDefinitionReturnType test, is broken. clang-format (in
combination with LLVM style) seems to indent the lines right after the
inline keyword, which gives the following error:

Value of: format(test::messUp(Code), Style)
  Actual: "inline\n    const char *\n    f(void) {\n  return
\"\";\n}\ninline const char *bar(void);\n"
Expected: Code.str()
Which is: "inline\nconst char *f(void) {\n  return \"\";\n}\ninline
const char *bar(void);\n"
...

Since this is my first excursion into the clang-format codebase, it is
not clear to me how this line breaking is linked to indentation.
Can someone help me out there? Or ideally implement the option correctly? :)

Thanks,
Michael

---

Index: include/clang/Format/Format.h
===================================================================
--- include/clang/Format/Format.h    (revision 224222)
+++ include/clang/Format/Format.h    (working copy)
@@ -275,6 +275,10 @@
   /// template declaration.
   bool AlwaysBreakTemplateDeclarations;

+  /// \brief If \c true, always break after the inline keyword of a function
+  /// definition.
+  bool AlwaysBreakAfterDefinitionInline;
+
   /// \brief If \c true, always break before multiline string literals.
   bool AlwaysBreakBeforeMultilineStrings;

@@ -426,6 +430,8 @@
                R.AlwaysBreakAfterDefinitionReturnType &&
            AlwaysBreakTemplateDeclarations ==
                R.AlwaysBreakTemplateDeclarations &&
+           AlwaysBreakAfterDefinitionInline ==
+               R.AlwaysBreakAfterDefinitionInline &&
            AlwaysBreakBeforeMultilineStrings ==
                R.AlwaysBreakBeforeMultilineStrings &&
            BinPackParameters == R.BinPackParameters &&
Index: lib/Format/Format.cpp
===================================================================
--- lib/Format/Format.cpp    (revision 224222)
+++ lib/Format/Format.cpp    (working copy)
@@ -192,6 +192,8 @@
                    Style.AlwaysBreakAfterDefinitionReturnType);
     IO.mapOptional("AlwaysBreakTemplateDeclarations",
                    Style.AlwaysBreakTemplateDeclarations);
+    IO.mapOptional("AlwaysBreakAfterDefinitionInline",
+                   Style.AlwaysBreakAfterDefinitionInline);
     IO.mapOptional("AlwaysBreakBeforeMultilineStrings",
                    Style.AlwaysBreakBeforeMultilineStrings);
     IO.mapOptional("BreakBeforeBinaryOperators",
@@ -340,6 +342,7 @@
   LLVMStyle.AlwaysBreakAfterDefinitionReturnType = false;
   LLVMStyle.AlwaysBreakBeforeMultilineStrings = false;
   LLVMStyle.AlwaysBreakTemplateDeclarations = false;
+  LLVMStyle.AlwaysBreakAfterDefinitionInline = false;
   LLVMStyle.BinPackParameters = true;
   LLVMStyle.BinPackArguments = true;
   LLVMStyle.BreakBeforeBinaryOperators = FormatStyle::BOS_None;
Index: lib/Format/TokenAnnotator.cpp
===================================================================
--- lib/Format/TokenAnnotator.cpp    (revision 224222)
+++ lib/Format/TokenAnnotator.cpp    (working copy)
@@ -1409,6 +1409,11 @@
       // and tok::lbrace.
       Current->MustBreakBefore = true;

+    if (Style.AlwaysBreakAfterDefinitionInline &&
+        Current->Previous && Current->Previous->is(tok::kw_inline) &&
+        !Line.Last->isOneOf(tok::semi, tok::comment))  // Only for definitions.
+      Current->MustBreakBefore = true;
+
     Current->CanBreakBefore =
         Current->MustBreakBefore || canBreakBefore(Line, *Current);
     unsigned ChildSize = 0;
Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp    (revision 224222)
+++ unittests/Format/FormatTest.cpp    (working copy)
@@ -4503,6 +4503,40 @@
                AfterType);
 }

+TEST_F(FormatTest, AlwaysBreakAfterDefinitionInline) {
+  FormatStyle AfterType = getLLVMStyle();
+  AfterType.AlwaysBreakAfterDefinitionInline = true;
+  verifyFormat("inline\n" // Break here.
+               "const char *f(void) {\n"
+               "  return \"\";\n"
+               "}\n"
+               "inline const char *bar(void);\n",  // No break here.
+               AfterType);
+  verifyFormat("template <class T>\n"
+               "inline\n"  // Break here.
+               "T *f(T &c) {\n"
+               "  return NULL;\n"
+               "}\n"
+               "template <class T> inline T *f(T &c);\n",  // No break here.
+               AfterType);
+  AfterType.BreakBeforeBraces = FormatStyle::BS_Stroustrup;
+  verifyFormat("inline\n"  // Break here.
+               "const char *f(void)\n"
+               "{\n"
+               "  return \"\";\n"
+               "}\n"
+               "inline const char *bar(void);\n",  // No break here.
+               AfterType);
+  verifyFormat("template <class T>\n"
+               "inline\n"
+               "T *f(T &c)\n"  // Break here.
+               "{\n"
+               "  return NULL;\n"
+               "}\n"
+               "template <class T> inline T *f(T &c);\n",  // No break here.
+               AfterType);
+}
+
 TEST_F(FormatTest, AlwaysBreakBeforeMultilineStrings) {
   FormatStyle NoBreak = getLLVMStyle();
   NoBreak.AlwaysBreakBeforeMultilineStrings = false;
@@ -8674,6 +8708,7 @@
   CHECK_PARSE_BOOL(AllowShortIfStatementsOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortLoopsOnASingleLine);
   CHECK_PARSE_BOOL(AlwaysBreakAfterDefinitionReturnType);
+  CHECK_PARSE_BOOL(AlwaysBreakAfterDefinitionInline);
   CHECK_PARSE_BOOL(AlwaysBreakTemplateDeclarations);
   CHECK_PARSE_BOOL(BinPackParameters);
   CHECK_PARSE_BOOL(BinPackArguments);



More information about the cfe-dev mailing list