Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions clang/docs/ClangFormatStyleOptions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1926,6 +1926,15 @@ the configuration (without a prefix: ``Auto``).
void f() {
}

* ``SFS_StaticInlineOnly`` (in configuration: ``StaticInlineOnly``)
Only merge functions defined as static inline.

.. code-block:: c++

void f5(void) {
}
static inline int f6(int a, int b) { return a + b; }

* ``SFS_Empty`` (in configuration: ``Empty``)
Only merge empty functions.

Expand All @@ -1949,6 +1958,14 @@ the configuration (without a prefix: ``Auto``).
}
void f() {}

* ``SFS_StaticInline`` (in configuration: ``StaticInline``)
Merge functions defined as static inline, also merge empty functions.

.. code-block:: c++

void f5(void) {}
static inline int f6(int a, int b) { return a + b; }

* ``SFS_All`` (in configuration: ``All``)
Merge all functions fitting on a single line.

Expand Down
13 changes: 13 additions & 0 deletions clang/include/clang/Format/Format.h
Original file line number Diff line number Diff line change
Expand Up @@ -844,6 +844,13 @@ struct FormatStyle {
/// }
/// \endcode
SFS_InlineOnly,
/// Only merge functions defined as static inline.
/// \code
/// void f5(void) {
/// }
/// static inline int f6(int a, int b) { return a + b; }
/// \endcode
SFS_StaticInlineOnly,
/// Only merge empty functions.
/// \code
/// void f() {}
Expand All @@ -863,6 +870,12 @@ struct FormatStyle {
/// void f() {}
/// \endcode
SFS_Inline,
/// Merge functions defined as static inline, also merge empty functions.
/// \code
/// void f5(void) {}
/// static inline int f6(int a, int b) { return a + b; }
/// \endcode
SFS_StaticInline,
/// Merge all functions fitting on a single line.
/// \code
/// class Foo {
Expand Down
2 changes: 2 additions & 0 deletions clang/lib/Format/Format.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -622,6 +622,8 @@ template <> struct ScalarEnumerationTraits<FormatStyle::ShortFunctionStyle> {
IO.enumCase(Value, "Inline", FormatStyle::SFS_Inline);
IO.enumCase(Value, "InlineOnly", FormatStyle::SFS_InlineOnly);
IO.enumCase(Value, "Empty", FormatStyle::SFS_Empty);
IO.enumCase(Value, "StaticInlineOnly", FormatStyle::SFS_StaticInlineOnly);
IO.enumCase(Value, "StaticInline", FormatStyle::SFS_StaticInline);
}
};

Expand Down
6 changes: 4 additions & 2 deletions clang/lib/Format/TokenAnnotator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5690,8 +5690,10 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
return Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_None ||
Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_Empty ||
(Left.NestingLevel == 0 && Line.Level == 0 &&
Style.AllowShortFunctionsOnASingleLine &
FormatStyle::SFS_InlineOnly);
(Style.AllowShortFunctionsOnASingleLine ==
FormatStyle::SFS_InlineOnly ||
Style.AllowShortFunctionsOnASingleLine ==
FormatStyle::SFS_Inline));
}
} else if (Style.Language == FormatStyle::LK_Java) {
if (Right.is(tok::plus) && Left.is(tok::string_literal) && AfterRight &&
Expand Down
32 changes: 30 additions & 2 deletions clang/lib/Format/UnwrappedLineFormatter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -300,8 +300,9 @@ class LineJoiner {
return true;
}

if (Style.AllowShortFunctionsOnASingleLine &
FormatStyle::SFS_InlineOnly) {
if (Style.AllowShortFunctionsOnASingleLine ==
FormatStyle::SFS_InlineOnly ||
Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_Inline) {
// Just checking TheLine->Level != 0 is not enough, because it
// provokes treating functions inside indented namespaces as short.
if (Style.isJavaScript() && TheLine->Last->is(TT_FunctionLBrace))
Expand Down Expand Up @@ -335,6 +336,33 @@ class LineJoiner {
}
}

if (Style.AllowShortFunctionsOnASingleLine ==
FormatStyle::SFS_StaticInlineOnly ||
Style.AllowShortFunctionsOnASingleLine ==
FormatStyle::SFS_StaticInline) {
// Check if the current line belongs to a static inline function
const auto *FirstNonCommentToken =
TheLine ? TheLine->getFirstNonComment() : nullptr;

// Look for 'static' and 'inline' keywords in any order.
bool HasStatic = false;
bool HasInline = false;
const FormatToken *Tok = FirstNonCommentToken;

while (Tok && !Tok->is(TT_FunctionDeclarationName) &&
(!HasStatic || !HasInline)) {
if (Tok->is(tok::kw_static))
HasStatic = true;
if (Tok->is(tok::kw_inline))
HasInline = true;
Tok = Tok->Next;
}

// If we found both static and inline, allow merging.
if (HasStatic && HasInline)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this check into the ifs of the loop, exit early.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added additional check in while(). Please take a look.

return true;
}

return false;
};

Expand Down
6 changes: 6 additions & 0 deletions clang/unittests/Format/ConfigParseTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,12 @@ TEST(ConfigParseTest, ParsesConfiguration) {
AllowShortFunctionsOnASingleLine, FormatStyle::SFS_Empty);
CHECK_PARSE("AllowShortFunctionsOnASingleLine: All",
AllowShortFunctionsOnASingleLine, FormatStyle::SFS_All);
CHECK_PARSE("AllowShortFunctionsOnASingleLine: StaticInlineOnly",
AllowShortFunctionsOnASingleLine,
FormatStyle::SFS_StaticInlineOnly);
CHECK_PARSE("AllowShortFunctionsOnASingleLine: StaticInline",
AllowShortFunctionsOnASingleLine, FormatStyle::SFS_StaticInline);

// For backward compatibility:
CHECK_PARSE("AllowShortFunctionsOnASingleLine: false",
AllowShortFunctionsOnASingleLine, FormatStyle::SFS_None);
Expand Down
129 changes: 129 additions & 0 deletions clang/unittests/Format/FormatTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15120,6 +15120,135 @@ TEST_F(FormatTest, PullInlineFunctionDefinitionsIntoSingleLine) {
MergeInlineOnly);
}

TEST_F(FormatTest, PullStaticInlineFunctionDefinitionsIntoSingleLine) {
FormatStyle MergeStaticInlineOnly = getLLVMStyle();
MergeStaticInlineOnly.AllowShortFunctionsOnASingleLine =
FormatStyle::SFS_StaticInlineOnly;
verifyFormat("static inline int f() { return 42; }",
"static inline int f() {\n"
" return 42; \n"
"}",
MergeStaticInlineOnly);
verifyFormat("inline static int f() { return 42; }",
"inline static int f() \n"
"{\n"
" return 42; \n"
"}",
MergeStaticInlineOnly);
verifyNoChange("int f() {\n"
" return 42;\n"
"}",
MergeStaticInlineOnly);

verifyFormat("void f1(void) {\n"
"}",
"void f1(void)\n"
"{\n"
"}",
MergeStaticInlineOnly);

verifyNoChange("int f2(int a, int b) {\n"
" return a + b;\n"
"}",
MergeStaticInlineOnly);

verifyNoChange("static void f3(void) {\n"
"}\n",
MergeStaticInlineOnly);

verifyFormat("static int f4(int a, int b) {\n"
" return a + b;\n"
"}\n",
MergeStaticInlineOnly);

verifyNoChange("static inline void f5(void) {}", MergeStaticInlineOnly);

verifyFormat("static inline int f6(int a, int b) { return a + b; }",
"static inline int f6(int a, int b) \n"
"{ return a + b; }",
MergeStaticInlineOnly);

verifyFormat("int f(int a, int b) {\n"
" return a + b;\n"
"}",
"int f(int a, int b) { return a + b; }", MergeStaticInlineOnly);

FormatStyle MergeStaticInline = getLLVMStyle();
MergeStaticInline.AllowShortFunctionsOnASingleLine =
FormatStyle::SFS_StaticInline;
verifyFormat("static inline int f() { return 42; }",
"static inline int f() {\n"
" return 42; \n"
"}",
MergeStaticInline);
verifyFormat("inline static int f() { return 42; }",
"inline static int f() \n"
"{\n"
" return 42; \n"
"}",
MergeStaticInline);
verifyNoChange("int f() {\n"
" return 42;\n"
"}",
MergeStaticInline);

verifyFormat("void f1(void) {}",
"void f1(void)\n"
"{\n"
"}",
MergeStaticInline);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add some tests with attributes.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added some tests with different attributes in different places. If you have any idea what should be added - small example (just code line) would be appreciated.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about constexpr this implies inline, what should be done in this case?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no preference regarding constexpr because it was not mentioned in 'Inline' or 'InlineOnly' options before. Right now it won't be formatted if StaticInline is specified (I added unit test to show this). Please let me know if I need to change this.

// additional attribute tests
verifyFormat("inline static __attribute__((unused)) int f() { return 42; }",
"inline static __attribute__((unused)) int f() \n"
"{\n"
" return 42; \n"
"}",
MergeStaticInline);
verifyFormat("__attribute__((unused)) inline static int f() { return 42; }",
"__attribute__((unused)) inline static int f() \n"
"{\n"
" return 42; \n"
"}",
MergeStaticInline);
verifyFormat("inline static int f() __attribute__((unused)) { return 42; }",
"inline static int f() \n"
"__attribute__((unused)) \n"
"{\n"
" return 42; \n"
"}",
MergeStaticInline);
verifyFormat("__attribute__((unused)) inline static int f() { return 42; }",
"__attribute__((unused)) inline static int f() \n"
"{\n"
" return 42; \n"
"}",
MergeStaticInline);

verifyFormat("inline static const int f() { return 42; }",
"inline static const int f() \n"
"{\n"
" return 42; \n"
"}",
MergeStaticInline);

verifyFormat("_Noreturn static inline auto f() { return 42; }",
"_Noreturn static inline auto f() \n"
"{\n"
" return 42; \n"
"}",
MergeStaticInline);

verifyFormat("constexpr auto f() {\n"
" return 42;\n"
"}",
"constexpr auto f() \n"
"{\n"
" return 42; \n"
"}",
MergeStaticInline);
}

TEST_F(FormatTest, PullInlineOnlyFunctionDefinitionsIntoSingleLine) {
FormatStyle MergeInlineOnly = getLLVMStyle();
MergeInlineOnly.AllowShortFunctionsOnASingleLine =
Expand Down