Skip to content

[clang-format] allow short function body on a single line #151428

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 1 commit into
base: main
Choose a base branch
from
Open

[clang-format] allow short function body on a single line #151428

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 19 additions & 0 deletions clang/docs/ClangFormatStyleOptions.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5405,6 +5405,25 @@ the configuration (without a prefix: ``Auto``).



.. _PutShortFunctionBodiesOnASingleLine:

**PutShortFunctionBodiesOnASingleLine** (``Boolean``) :versionbadge:`clang-format 20` :ref:`<PutShortFunctionBodiesOnASingleLine>`
Dependent on the value, function body can be put on a single line.
Automatically enabled when
`AllowShortFunctionsOnASingleLine` is set to `None` and
`AllowShortBlocksOnASingleLine` is set to `Always`.

.. code-block:: c++

true:
int f()
{ return 0; }

false:
int f() {
return 0;
}

.. _QualifierAlignment:

**QualifierAlignment** (``QualifierAlignmentStyle``) :versionbadge:`clang-format 14` :ref:`<QualifierAlignment>`
Expand Down
19 changes: 19 additions & 0 deletions clang/include/clang/Format/Format.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3829,6 +3829,23 @@ struct FormatStyle {
/// \version 13
int PPIndentWidth;

/// Dependent on the value, function body can be put on a single line.
/// Automatically enabled when
/// `AllowShortFunctionsOnASingleLine` is set to `None` and
/// `AllowShortBlocksOnASingleLine` is set to `Always`.
/// \code
/// true:
/// int f()
/// { return 0; }
///
/// false:
/// int f() {
/// return 0;
/// }
/// \endcode
/// \version 20
bool PutShortFunctionBodiesOnASingleLine;

/// Different specifiers and qualifiers alignment styles.
enum QualifierAlignmentStyle : int8_t {
/// Don't change specifiers/qualifiers to either Left or Right alignment
Expand Down Expand Up @@ -5461,6 +5478,8 @@ struct FormatStyle {
PenaltyExcessCharacter == R.PenaltyExcessCharacter &&
PenaltyReturnTypeOnItsOwnLine == R.PenaltyReturnTypeOnItsOwnLine &&
PointerAlignment == R.PointerAlignment &&
PutShortFunctionBodiesOnASingleLine ==
R.PutShortFunctionBodiesOnASingleLine &&
QualifierAlignment == R.QualifierAlignment &&
QualifierOrder == R.QualifierOrder &&
RawStringFormats == R.RawStringFormats &&
Expand Down
7 changes: 7 additions & 0 deletions clang/lib/Format/ContinuationIndenter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,13 @@ bool ContinuationIndenter::canBreak(const LineState &State) {
return true;
}

// `PutShortFunctionBodiesOnASingleLine = true` will wrap left brace into
// a new line, do not break before the left brace.
if (Current.is(TT_FunctionLBrace) && !Style.BraceWrapping.AfterFunction &&
Style.PutShortFunctionBodiesOnASingleLine) {
return false;
}

return !State.NoLineBreak && !CurrentState.NoLineBreak;
}

Expand Down
7 changes: 7 additions & 0 deletions clang/lib/Format/Format.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1144,6 +1144,8 @@ template <> struct MappingTraits<FormatStyle> {
Style.PenaltyReturnTypeOnItsOwnLine);
IO.mapOptional("PointerAlignment", Style.PointerAlignment);
IO.mapOptional("PPIndentWidth", Style.PPIndentWidth);
IO.mapOptional("PutShortFunctionBodiesOnASingleLine",
Style.PutShortFunctionBodiesOnASingleLine);
IO.mapOptional("QualifierAlignment", Style.QualifierAlignment);
// Default Order for Left/Right based Qualifier alignment.
if (Style.QualifierAlignment == FormatStyle::QAS_Right)
Expand Down Expand Up @@ -1643,6 +1645,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
LLVMStyle.PackConstructorInitializers = FormatStyle::PCIS_BinPack;
LLVMStyle.PointerAlignment = FormatStyle::PAS_Right;
LLVMStyle.PPIndentWidth = -1;
LLVMStyle.PutShortFunctionBodiesOnASingleLine = false;
LLVMStyle.QualifierAlignment = FormatStyle::QAS_Leave;
LLVMStyle.ReferenceAlignment = FormatStyle::RAS_Pointer;
LLVMStyle.ReflowComments = FormatStyle::RCS_Always;
Expand Down Expand Up @@ -3828,6 +3831,10 @@ reformat(const FormatStyle &Style, StringRef Code,
default:
break;
}
if (Expanded.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_None &&
Expanded.AllowShortBlocksOnASingleLine == FormatStyle::SBS_Always) {
Expanded.PutShortFunctionBodiesOnASingleLine = true;
}

if (Expanded.DisableFormat)
return {tooling::Replacements(), 0};
Expand Down
32 changes: 28 additions & 4 deletions clang/lib/Format/UnwrappedLineFormatter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,13 +242,13 @@ class LineJoiner {
if (Style.ColumnLimit > 0 && Indent > Style.ColumnLimit)
return 0;

unsigned Limit =
unsigned LimitStripIndent =
Style.ColumnLimit == 0 ? UINT_MAX : Style.ColumnLimit - Indent;
// If we already exceed the column limit, we set 'Limit' to 0. The different
// tryMerge..() functions can then decide whether to still do merging.
Limit = TheLine->Last->TotalLength > Limit
? 0
: Limit - TheLine->Last->TotalLength;
unsigned Limit = TheLine->Last->TotalLength > LimitStripIndent
? 0
: LimitStripIndent - TheLine->Last->TotalLength;

if (TheLine->Last->is(TT_FunctionLBrace) &&
TheLine->First == TheLine->Last &&
Expand All @@ -257,6 +257,12 @@ class LineJoiner {
return tryMergeSimpleBlock(I, E, Limit);
}

if (TheLine->Last->is(TT_FunctionLBrace) &&
TheLine->First == TheLine->Last &&
Style.PutShortFunctionBodiesOnASingleLine) {
return tryMergeSimpleBlock(I, E, Limit);
}

const auto *PreviousLine = I != AnnotatedLines.begin() ? I[-1] : nullptr;
// Handle empty record blocks where the brace has already been wrapped.
if (PreviousLine && TheLine->Last->is(tok::l_brace) &&
Expand Down Expand Up @@ -532,6 +538,24 @@ class LineJoiner {
}
return MergedLines;
}

// UnwrappedLineParser would move the left brace to a new line when
// PutShortFunctionBodiesOnASingleLine is enabled. However, if the
// function body cannot fit on a single line, and
// Style.BraceWrapping.AfterFunction is false, we should merge the
// function name and the left brace back onto the same line.
if (NextLine.First->is(TT_FunctionLBrace) &&
Style.PutShortFunctionBodiesOnASingleLine &&
!Style.BraceWrapping.AfterFunction) {
unsigned MergedLines = 0;
unsigned NextLineLimit =
NextLine.Last->TotalLength > LimitStripIndent
? 0
: LimitStripIndent - NextLine.Last->TotalLength;
MergedLines = tryMergeSimpleBlock(I + 1, E, NextLineLimit);
return MergedLines > 0 ? 0 : 1;
}

auto IsElseLine = [&TheLine]() -> bool {
const FormatToken *First = TheLine->First;
if (First->is(tok::kw_else))
Expand Down
4 changes: 4 additions & 0 deletions clang/lib/Format/UnwrappedLineParser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1921,6 +1921,10 @@ void UnwrappedLineParser::parseStructuralElement(
}
} else if (Style.BraceWrapping.AfterFunction) {
addUnwrappedLine();
} else if (Style.PutShortFunctionBodiesOnASingleLine) {
// Wrap the left brace here; we'll try to merge it back
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this needed? It seems wrong to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If Style.AllowShortFunctionBodiesOnSingleLine = true, I modified UnwrappedLineParser to parse

int main() {
  //...
}

into unwrapped lines ["int main()", "{", ..., "}"], not ["int main() {", ..., "}"] as usual. If clang-format can't put function body on a single line, I want to merge "int main()" and "{" back if possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason I parse the { in void foo() { as an unwrapped line is to allow merging the { and the function body into one line when the function body is very short.

// later if needed.
addUnwrappedLine();
}
if (!Previous || Previous->isNot(TT_TypeDeclarationParen))
FormatTok->setFinalizedType(TT_FunctionLBrace);
Expand Down
30 changes: 30 additions & 0 deletions clang/unittests/Format/FormatTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14941,11 +14941,24 @@ TEST_F(FormatTest, PullTrivialFunctionDefinitionsIntoSingleLine) {
FormatStyle DoNotMerge = getLLVMStyle();
DoNotMerge.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;

FormatStyle MergeBodyOnly = getLLVMStyle();
MergeBodyOnly.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
MergeBodyOnly.PutShortFunctionBodiesOnASingleLine = true;

FormatStyle MergeBodyIfPossible = getLLVMStyleWithColumns(20);
MergeBodyIfPossible.PutShortFunctionBodiesOnASingleLine = true;

verifyFormat("void f() { return 42; }");
verifyFormat("void f() {\n"
" return 42;\n"
"}",
DoNotMerge);
verifyFormat("void f()\n"
"{ return 42; }",
MergeBodyOnly);
verifyFormat("void long_function_name()\n"
"{ return 42; }",
MergeBodyIfPossible);
verifyFormat("void f() {\n"
" // Comment\n"
"}");
Expand All @@ -14966,6 +14979,23 @@ TEST_F(FormatTest, PullTrivialFunctionDefinitionsIntoSingleLine) {
" int a;\n"
"} // comment",
DoNotMerge);
verifyFormat("void f()\n"
"{} // comment",
MergeBodyOnly);
verifyFormat("void f()\n"
"{ int a; } // comment",
MergeBodyOnly);
verifyFormat("void long_function_name()\n"
"{} // comment",
MergeBodyIfPossible);
verifyFormat("void f() {\n"
" int a;\n"
"} // comment",
MergeBodyIfPossible);
MergeBodyIfPossible.ColumnLimit = 21;
verifyFormat("void f()\n"
"{ int a; } // comment",
MergeBodyIfPossible);
verifyFormat("void f() {\n"
"} // comment",
getLLVMStyleWithColumns(15));
Expand Down