Skip to content

[clang-format]: Add Custom to ShortFunctionStyle; add new AllowShortFunctionsOnASingleLineOptions for granular setup #134337

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

[clang-format]: Add Custom to ShortFunctionStyle; add new AllowShortFunctionsOnASingleLineOptions for granular setup #134337

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
df25a8b
[clang-format]: Add `Custom` to `ShortFunctionStyle`; add new AllowSh…
Mar 29, 2025
e9e4e92
PR-134337: fixed PR comments
Apr 5, 2025
b1fd412
Updated configuration with new suggestion - reuse existing option
Apr 8, 2025
7f80e4a
fixed documentation error
Apr 9, 2025
bbe2273
Fixed PR comments
Apr 10, 2025
5ff5810
marked options as deprecated
Apr 13, 2025
70eafea
fixed PR comments
Apr 16, 2025
dd6493d
added usage of 'other'
Apr 16, 2025
03fd9e9
Merge remote-tracking branch 'origin/main' into add_custom_AllowShort…
Apr 19, 2025
3c0fa14
fixed ShortFunctionStyle.Other case for JS
Apr 21, 2025
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
74 changes: 65 additions & 9 deletions clang/docs/ClangFormatStyleOptions.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1905,15 +1905,21 @@ the configuration (without a prefix: ``Auto``).
Dependent on the value, ``int f() { return 0; }`` can be put on a
single line.

Possible values:
Nested configuration flags:

Different styles for merging short functions containing at most one
statement.

* ``SFS_None`` (in configuration: ``None``)
They can be read as a whole for compatibility. The choices are:

* ``None``
Never merge functions into a single line.

* ``SFS_InlineOnly`` (in configuration: ``InlineOnly``)
* ``InlineOnly``
Only merge functions defined inside a class. Same as ``inline``,
except it does not implies ``empty``: i.e. top level empty functions
are not merged either.
are not merged either. This option is **deprecated** and is retained
for backwards compatibility. See ``Inline`` of ``ShortFunctionStyle``.

.. code-block:: c++

Expand All @@ -1926,8 +1932,10 @@ the configuration (without a prefix: ``Auto``).
void f() {
}

* ``SFS_Empty`` (in configuration: ``Empty``)
Only merge empty functions.
* ``Empty``
Only merge empty functions. This option is **deprecated** and is
retained for backwards compatibility. See ``Empty`` of
``ShortFunctionStyle``.

.. code-block:: c++

Expand All @@ -1936,8 +1944,10 @@ the configuration (without a prefix: ``Auto``).
bar2();
}

* ``SFS_Inline`` (in configuration: ``Inline``)
Only merge functions defined inside a class. Implies ``empty``.
* ``Inline``
Only merge functions defined inside a class. Implies ``empty``. This
option is **deprecated** and is retained for backwards compatibility.
See ``Inline`` and ``Empty`` of ``ShortFunctionStyle``.

.. code-block:: c++

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

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

.. code-block:: c++
Expand All @@ -1959,6 +1969,52 @@ the configuration (without a prefix: ``Auto``).
};
void f() { bar(); }

Also can be specified as a nested configuration flag:

.. code-block:: c++

# Example of usage:
AllowShortFunctionsOnASingleLine: InlineOnly

# or more granular control:
AllowShortFunctionsOnASingleLine:
Empty: false
Inline: true
Other: false

* ``bool Empty`` Merge top-level empty functions.

.. code-block:: c++

void f() {}
void f2() {
bar2();
}
void f3() { /* comment */ }

* ``bool Inline`` Merge functions defined inside a class.

.. code-block:: c++

class Foo {
void f() { foo(); }
void g() {}
};
void f() {
foo();
}
void f() {
}

* ``bool Other`` Merge all functions fitting on a single line. Please note that this
Copy link
Author

@irymarchyk irymarchyk Apr 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Should we expose this option to user? I am not sure what it means if we set only Other, but not Inline and not Empty - then C/C++ code will behave like SFS_All. Probably we should set it when user chooses 'All' in configuration.

control does not include Empty

.. code-block:: c++

class Foo {
void f() { foo(); }
};
void f() { bar(); }


.. _AllowShortIfStatementsOnASingleLine:
Expand Down
111 changes: 87 additions & 24 deletions clang/include/clang/Format/Format.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -827,50 +827,113 @@ struct FormatStyle {

/// Different styles for merging short functions containing at most one
/// statement.
enum ShortFunctionStyle : int8_t {
/// Never merge functions into a single line.
SFS_None,
/// Only merge functions defined inside a class. Same as ``inline``,
/// except it does not implies ``empty``: i.e. top level empty functions
/// are not merged either.
/// \code
/// class Foo {
/// void f() { foo(); }
/// };
/// void f() {
/// foo();
/// }
/// void f() {
/// }
/// \endcode
SFS_InlineOnly,
/// Only merge empty functions.
///
/// They can be read as a whole for compatibility. The choices are:
///
/// * ``None``
/// Never merge functions into a single line.
///
/// * ``InlineOnly``
/// Only merge functions defined inside a class. Same as ``inline``,
/// except it does not implies ``empty``: i.e. top level empty functions
/// are not merged either. This option is **deprecated** and is retained
/// for backwards compatibility. See ``Inline`` of ``ShortFunctionStyle``.
/// \code
/// class Foo {
/// void f() { foo(); }
/// };
/// void f() {
/// foo();
/// }
/// void f() {
/// }
/// \endcode
///
/// * ``Empty``
/// Only merge empty functions. This option is **deprecated** and is
/// retained for backwards compatibility. See ``Empty`` of
/// ``ShortFunctionStyle``.
/// \code
/// void f() {}
/// void f2() {
/// bar2();
/// }
/// \endcode
///
/// * ``Inline``
/// Only merge functions defined inside a class. Implies ``empty``. This
/// option is **deprecated** and is retained for backwards compatibility.
/// See ``Inline`` and ``Empty`` of ``ShortFunctionStyle``.
/// \code
/// class Foo {
/// void f() { foo(); }
/// };
/// void f() {
/// foo();
/// }
/// void f() {}
/// \endcode
///
/// * ``All``
/// Merge all functions fitting on a single line.
/// \code
/// class Foo {
/// void f() { foo(); }
/// };
/// void f() { bar(); }
/// \endcode
///
/// Also can be specified as a nested configuration flag:
/// \code
/// # Example of usage:
/// AllowShortFunctionsOnASingleLine: InlineOnly
///
/// # or more granular control:
/// AllowShortFunctionsOnASingleLine:
/// Empty: false
/// Inline: true
/// Other: false
/// \endcode
struct ShortFunctionStyle {
/// Merge top-level empty functions.
/// \code
/// void f() {}
/// void f2() {
/// bar2();
/// }
/// void f3() { /* comment */ }
/// \endcode
SFS_Empty,
/// Only merge functions defined inside a class. Implies ``empty``.
bool Empty;
/// Merge functions defined inside a class.
/// \code
/// class Foo {
/// void f() { foo(); }
/// void g() {}
/// };
/// void f() {
/// foo();
/// }
/// void f() {}
/// void f() {
/// }
/// \endcode
SFS_Inline,
/// Merge all functions fitting on a single line.
bool Inline;
/// Merge all functions fitting on a single line. Please note that this
/// control does not include Empty
/// \code
/// class Foo {
/// void f() { foo(); }
/// };
/// void f() { bar(); }
/// \endcode
SFS_All,
bool Other;

bool operator==(const ShortFunctionStyle &R) const {
return Empty == R.Empty && Inline == R.Inline && Other == R.Other;
}
bool operator!=(const ShortFunctionStyle &R) const { return !(*this == R); }
ShortFunctionStyle() : Empty(false), Inline(false), Other(false) {}
ShortFunctionStyle(bool Empty, bool Inline, bool Other)
: Empty(Empty), Inline(Inline), Other(Other) {}
};

/// Dependent on the value, ``int f() { return 0; }`` can be put on a
Expand Down
78 changes: 61 additions & 17 deletions clang/lib/Format/Format.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -613,15 +613,38 @@ template <> struct ScalarEnumerationTraits<FormatStyle::ShortBlockStyle> {
}
};

template <> struct ScalarEnumerationTraits<FormatStyle::ShortFunctionStyle> {
static void enumeration(IO &IO, FormatStyle::ShortFunctionStyle &Value) {
IO.enumCase(Value, "None", FormatStyle::SFS_None);
IO.enumCase(Value, "false", FormatStyle::SFS_None);
IO.enumCase(Value, "All", FormatStyle::SFS_All);
IO.enumCase(Value, "true", FormatStyle::SFS_All);
IO.enumCase(Value, "Inline", FormatStyle::SFS_Inline);
IO.enumCase(Value, "InlineOnly", FormatStyle::SFS_InlineOnly);
IO.enumCase(Value, "Empty", FormatStyle::SFS_Empty);
template <> struct MappingTraits<FormatStyle::ShortFunctionStyle> {
static void enumInput(IO &IO, FormatStyle::ShortFunctionStyle &Value) {
IO.enumCase(Value, "None", FormatStyle::ShortFunctionStyle({}));
IO.enumCase(Value, "Empty",
FormatStyle::ShortFunctionStyle({/*Empty=*/true,
/*Inline=*/false,
/*Other=*/false}));
IO.enumCase(Value, "Inline",
FormatStyle::ShortFunctionStyle({/*Empty=*/true,
/*Inline=*/true,
/*Other=*/false}));
IO.enumCase(Value, "InlineOnly",
FormatStyle::ShortFunctionStyle({/*Empty=*/false,
/*Inline=*/true,
/*Other=*/false}));
IO.enumCase(Value, "All",
FormatStyle::ShortFunctionStyle({/*Empty=*/true,
/*Inline=*/true,
/*Other=*/true}));

// For backward compatibility.
IO.enumCase(Value, "true",
FormatStyle::ShortFunctionStyle({/*Empty=*/true,
/*Inline=*/true,
/*Other=*/true}));
IO.enumCase(Value, "false", FormatStyle::ShortFunctionStyle({}));
}

static void mapping(IO &IO, FormatStyle::ShortFunctionStyle &Value) {
IO.mapOptional("Empty", Value.Empty);
IO.mapOptional("Inline", Value.Inline);
IO.mapOptional("Other", Value.Other);
}
};

Expand Down Expand Up @@ -1500,7 +1523,10 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
LLVMStyle.AllowShortCaseLabelsOnASingleLine = false;
LLVMStyle.AllowShortCompoundRequirementOnASingleLine = true;
LLVMStyle.AllowShortEnumsOnASingleLine = true;
LLVMStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
LLVMStyle.AllowShortFunctionsOnASingleLine =
FormatStyle::ShortFunctionStyle({/*Empty=*/true,
Copy link
Author

Choose a reason for hiding this comment

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

Please let me know if it is acceptable to make static functions like FormatStyle::ShortFunctionStyle::SFS_Empty() (or similar) so we don't have specify each member in constructor every time. This might be useful in the future - I want to add few options to ShortFunctionStyle, and I have to increase number of parameters in constructor. This will require changes in every place where it was used.

/*Inline=*/true,
/*Other=*/true});
LLVMStyle.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
LLVMStyle.AllowShortLambdasOnASingleLine = FormatStyle::SLS_All;
LLVMStyle.AllowShortLoopsOnASingleLine = false;
Expand Down Expand Up @@ -1787,7 +1813,10 @@ FormatStyle getGoogleStyle(FormatStyle::LanguageKind Language) {
GoogleStyle.AlignOperands = FormatStyle::OAS_DontAlign;
GoogleStyle.AlignTrailingComments = {};
GoogleStyle.AlignTrailingComments.Kind = FormatStyle::TCAS_Never;
GoogleStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
GoogleStyle.AllowShortFunctionsOnASingleLine =
FormatStyle::ShortFunctionStyle({/*Empty=*/true,
/*Inline=*/false,
/*Other=*/false});
GoogleStyle.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
GoogleStyle.BreakBeforeBinaryOperators = FormatStyle::BOS_NonAssignment;
Expand All @@ -1797,7 +1826,10 @@ FormatStyle getGoogleStyle(FormatStyle::LanguageKind Language) {
} else if (Language == FormatStyle::LK_JavaScript) {
GoogleStyle.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
GoogleStyle.AlignOperands = FormatStyle::OAS_DontAlign;
GoogleStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
GoogleStyle.AllowShortFunctionsOnASingleLine =
FormatStyle::ShortFunctionStyle({/*Empty=*/true,
/*Inline=*/false,
/*Other=*/false});
// TODO: still under discussion whether to switch to SLS_All.
GoogleStyle.AllowShortLambdasOnASingleLine = FormatStyle::SLS_Empty;
GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
Expand All @@ -1814,7 +1846,10 @@ FormatStyle getGoogleStyle(FormatStyle::LanguageKind Language) {
GoogleStyle.NamespaceIndentation = FormatStyle::NI_All;
GoogleStyle.SpacesInContainerLiterals = false;
} else if (Language == FormatStyle::LK_Proto) {
GoogleStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
GoogleStyle.AllowShortFunctionsOnASingleLine =
FormatStyle::ShortFunctionStyle({/*Empty=*/true,
/*Inline=*/false,
/*Other=*/false});
GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
// This affects protocol buffer options specifications and text protos.
// Text protos are currently mostly formatted inside C++ raw string literals
Expand All @@ -1833,7 +1868,10 @@ FormatStyle getGoogleStyle(FormatStyle::LanguageKind Language) {
GoogleStyle.IncludeStyle.IncludeBlocks =
tooling::IncludeStyle::IBS_Preserve;
} else if (Language == FormatStyle::LK_CSharp) {
GoogleStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
GoogleStyle.AllowShortFunctionsOnASingleLine =
FormatStyle::ShortFunctionStyle({/*Empty=*/true,
/*Inline=*/false,
/*Other=*/false});
GoogleStyle.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
GoogleStyle.BreakStringLiterals = false;
GoogleStyle.ColumnLimit = 100;
Expand Down Expand Up @@ -1892,7 +1930,10 @@ FormatStyle getChromiumStyle(FormatStyle::LanguageKind Language) {
ChromiumStyle.AllowShortLoopsOnASingleLine = false;
} else {
ChromiumStyle.AllowAllParametersOfDeclarationOnNextLine = false;
ChromiumStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
ChromiumStyle.AllowShortFunctionsOnASingleLine =
FormatStyle::ShortFunctionStyle({/*Empty=*/true,
/*Inline=*/true,
/*Other=*/false});
ChromiumStyle.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
ChromiumStyle.AllowShortLoopsOnASingleLine = false;
ChromiumStyle.BinPackParameters = FormatStyle::BPPS_OnePerLine;
Expand All @@ -1906,7 +1947,10 @@ FormatStyle getChromiumStyle(FormatStyle::LanguageKind Language) {
FormatStyle getMozillaStyle() {
FormatStyle MozillaStyle = getLLVMStyle();
MozillaStyle.AllowAllParametersOfDeclarationOnNextLine = false;
MozillaStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
MozillaStyle.AllowShortFunctionsOnASingleLine =
FormatStyle::ShortFunctionStyle({/*Empty=*/true,
/*Inline=*/true,
/*Other=*/false});
MozillaStyle.AlwaysBreakAfterDefinitionReturnType =
FormatStyle::DRTBS_TopLevel;
MozillaStyle.BinPackArguments = false;
Expand Down Expand Up @@ -1988,7 +2032,7 @@ FormatStyle getMicrosoftStyle(FormatStyle::LanguageKind Language) {
Style.BraceWrapping.BeforeWhile = false;
Style.PenaltyReturnTypeOnItsOwnLine = 1000;
Style.AllowShortEnumsOnASingleLine = false;
Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
Style.AllowShortFunctionsOnASingleLine = FormatStyle::ShortFunctionStyle({});
Style.AllowShortCaseLabelsOnASingleLine = false;
Style.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
Style.AllowShortLoopsOnASingleLine = false;
Expand Down
Loading