Skip to content

Commit

Permalink
[clang-format] Improve brace wrapping and add an option to control in…
Browse files Browse the repository at this point in the history
…dentation of `export { ... }` (#110381)

`export { ... }` blocks can get a bit long, so I thought it would make
sense to have an option that makes it so their contents are not indented
(basically the same argument as for namespaces).

This is based on the `NamespaceIndentation` option, except that there is
no option to control the behaviour of `export` blocks when nested because
nesting them doesn’t really make sense.

Additionally, brace wrapping of short `export { ... }` blocks is now controlled by the
`AllowShortBlocksOnASingleLine` option. There is no separate option just for `export`
blocks because you can just write e.g. `export int x;` instead of `export { int x; }`.

This closes #121723.
  • Loading branch information
Sirraide authored Jan 18, 2025
1 parent eae5ca9 commit 106c483
Show file tree
Hide file tree
Showing 9 changed files with 189 additions and 22 deletions.
15 changes: 15 additions & 0 deletions clang/docs/ClangFormatStyleOptions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -3946,6 +3946,21 @@ the configuration (without a prefix: ``Auto``).
This is an experimental flag, that might go away or be renamed. Do
not use this in config files, etc. Use at your own risk.

.. _ExportBlockIndentation:

**ExportBlockIndentation** (``Boolean``) :versionbadge:`clang-format 20` :ref:`<ExportBlockIndentation>`
If ``true``, clang-format will indent the body of an ``export { ... }``
block. This doesn't affect the formatting of anything else related to
exported declarations.

.. code-block:: c++

true: false:
export { vs. export {
void foo(); void foo();
void bar(); void bar();
} }

.. _FixNamespaceComments:

**FixNamespaceComments** (``Boolean``) :versionbadge:`clang-format 5` :ref:`<FixNamespaceComments>`
Expand Down
1 change: 1 addition & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1224,6 +1224,7 @@ clang-format
- Adds ``VariableTemplates`` option.
- Adds support for bash globstar in ``.clang-format-ignore``.
- Adds ``WrapNamespaceBodyWithEmptyLines`` option.
- Adds the ``ExportBlockIndentation`` option.

libclang
--------
Expand Down
14 changes: 14 additions & 0 deletions clang/include/clang/Format/Format.h
Original file line number Diff line number Diff line change
Expand Up @@ -2676,6 +2676,19 @@ struct FormatStyle {
/// \version 3.7
bool ExperimentalAutoDetectBinPacking;

/// If ``true``, clang-format will indent the body of an ``export { ... }``
/// block. This doesn't affect the formatting of anything else related to
/// exported declarations.
/// \code
/// true: false:
/// export { vs. export {
/// void foo(); void foo();
/// void bar(); void bar();
/// } }
/// \endcode
/// \version 20
bool ExportBlockIndentation;

/// If ``true``, clang-format adds missing namespace end comments for
/// namespaces and fixes invalid existing ones. This doesn't affect short
/// namespaces, which are controlled by ``ShortNamespaceLines``.
Expand Down Expand Up @@ -5254,6 +5267,7 @@ struct FormatStyle {
EmptyLineBeforeAccessModifier == R.EmptyLineBeforeAccessModifier &&
ExperimentalAutoDetectBinPacking ==
R.ExperimentalAutoDetectBinPacking &&
ExportBlockIndentation == R.ExportBlockIndentation &&
FixNamespaceComments == R.FixNamespaceComments &&
ForEachMacros == R.ForEachMacros &&
IncludeStyle.IncludeBlocks == R.IncludeStyle.IncludeBlocks &&
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 @@ -1040,6 +1040,7 @@ template <> struct MappingTraits<FormatStyle> {
Style.EmptyLineBeforeAccessModifier);
IO.mapOptional("ExperimentalAutoDetectBinPacking",
Style.ExperimentalAutoDetectBinPacking);
IO.mapOptional("ExportBlockIndentation", Style.ExportBlockIndentation);
IO.mapOptional("FixNamespaceComments", Style.FixNamespaceComments);
IO.mapOptional("ForEachMacros", Style.ForEachMacros);
IO.mapOptional("IfMacros", Style.IfMacros);
Expand Down Expand Up @@ -1550,6 +1551,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
LLVMStyle.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Never;
LLVMStyle.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_LogicalBlock;
LLVMStyle.ExperimentalAutoDetectBinPacking = false;
LLVMStyle.ExportBlockIndentation = true;
LLVMStyle.FixNamespaceComments = true;
LLVMStyle.ForEachMacros.push_back("foreach");
LLVMStyle.ForEachMacros.push_back("Q_FOREACH");
Expand Down
5 changes: 5 additions & 0 deletions clang/lib/Format/TokenAnnotator.h
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,11 @@ class AnnotatedLine {
startsWith(tok::kw_export, tok::kw_namespace);
}

/// \c true if this line starts a C++ export block.
bool startsWithExportBlock() const {
return startsWith(tok::kw_export, tok::l_brace);
}

FormatToken *getFirstNonComment() const {
assert(First);
return First->is(tok::comment) ? First->getNextNonComment() : First;
Expand Down
8 changes: 5 additions & 3 deletions clang/lib/Format/UnwrappedLineFormatter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -432,8 +432,9 @@ class LineJoiner {

// Try to merge a control statement block with left brace unwrapped.
if (TheLine->Last->is(tok::l_brace) && FirstNonComment != TheLine->Last &&
FirstNonComment->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for,
TT_ForEachMacro)) {
(FirstNonComment->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for,
TT_ForEachMacro) ||
TheLine->startsWithExportBlock())) {
return Style.AllowShortBlocksOnASingleLine != FormatStyle::SBS_Never
? tryMergeSimpleBlock(I, E, Limit)
: 0;
Expand Down Expand Up @@ -832,7 +833,8 @@ class LineJoiner {
if (IsCtrlStmt(Line) ||
Line.First->isOneOf(tok::kw_try, tok::kw___try, tok::kw_catch,
tok::kw___finally, tok::r_brace,
Keywords.kw___except)) {
Keywords.kw___except) ||
Line.startsWithExportBlock()) {
if (IsSplitBlock)
return 0;
// Don't merge when we can't except the case when
Expand Down
49 changes: 30 additions & 19 deletions clang/lib/Format/UnwrappedLineParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1625,6 +1625,10 @@ void UnwrappedLineParser::parseStructuralElement(
parseNamespace();
return;
}
if (FormatTok->is(tok::l_brace)) {
parseCppExportBlock();
return;
}
if (FormatTok->is(Keywords.kw_import) && parseModuleImport())
return;
}
Expand Down Expand Up @@ -3105,6 +3109,26 @@ void UnwrappedLineParser::parseTryCatch() {
addUnwrappedLine();
}

void UnwrappedLineParser::parseNamespaceOrExportBlock(unsigned AddLevels) {
bool ManageWhitesmithsBraces =
AddLevels == 0u && Style.BreakBeforeBraces == FormatStyle::BS_Whitesmiths;

// If we're in Whitesmiths mode, indent the brace if we're not indenting
// the whole block.
if (ManageWhitesmithsBraces)
++Line->Level;

// Munch the semicolon after the block. This is more common than one would
// think. Putting the semicolon into its own line is very ugly.
parseBlock(/*MustBeDeclaration=*/true, AddLevels, /*MunchSemi=*/true,
/*KeepBraces=*/true, /*IfKind=*/nullptr, ManageWhitesmithsBraces);

addUnwrappedLine(AddLevels > 0 ? LineLevel::Remove : LineLevel::Keep);

if (ManageWhitesmithsBraces)
--Line->Level;
}

void UnwrappedLineParser::parseNamespace() {
assert(FormatTok->isOneOf(tok::kw_namespace, TT_NamespaceMacro) &&
"'namespace' expected");
Expand Down Expand Up @@ -3137,29 +3161,16 @@ void UnwrappedLineParser::parseNamespace() {
DeclarationScopeStack.size() > 1)
? 1u
: 0u;
bool ManageWhitesmithsBraces =
AddLevels == 0u &&
Style.BreakBeforeBraces == FormatStyle::BS_Whitesmiths;

// If we're in Whitesmiths mode, indent the brace if we're not indenting
// the whole block.
if (ManageWhitesmithsBraces)
++Line->Level;

// Munch the semicolon after a namespace. This is more common than one would
// think. Putting the semicolon into its own line is very ugly.
parseBlock(/*MustBeDeclaration=*/true, AddLevels, /*MunchSemi=*/true,
/*KeepBraces=*/true, /*IfKind=*/nullptr,
ManageWhitesmithsBraces);

addUnwrappedLine(AddLevels > 0 ? LineLevel::Remove : LineLevel::Keep);

if (ManageWhitesmithsBraces)
--Line->Level;
parseNamespaceOrExportBlock(AddLevels);
}
// FIXME: Add error handling.
}

void UnwrappedLineParser::parseCppExportBlock() {
parseNamespaceOrExportBlock(/*AddLevels=*/Style.ExportBlockIndentation ? 1
: 0);
}

void UnwrappedLineParser::parseNew() {
assert(FormatTok->is(tok::kw_new) && "'new' expected");
nextToken();
Expand Down
2 changes: 2 additions & 0 deletions clang/lib/Format/UnwrappedLineParser.h
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,8 @@ class UnwrappedLineParser {
void parseRequiresClause(FormatToken *RequiresToken);
void parseRequiresExpression(FormatToken *RequiresToken);
void parseConstraintExpression();
void parseCppExportBlock();
void parseNamespaceOrExportBlock(unsigned AddLevels);
void parseJavaEnumBody();
// Parses a record (aka class) as a top level element. If ParseAsExpr is true,
// parses the record as a child block, i.e. if the class declaration is an
Expand Down
115 changes: 115 additions & 0 deletions clang/unittests/Format/FormatTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9059,6 +9059,121 @@ TEST_F(FormatTest, AdaptiveOnePerLineFormatting) {
Style);
}

TEST_F(FormatTest, ExportBlockIndentation) {
FormatStyle Style = getLLVMStyleWithColumns(80);
Style.ExportBlockIndentation = true;
verifyFormat("export {\n"
" int x;\n"
" int y;\n"
"}",
"export {\n"
"int x;\n"
"int y;\n"
"}",
Style);

Style.ExportBlockIndentation = false;
verifyFormat("export {\n"
"int x;\n"
"int y;\n"
"}",
"export {\n"
" int x;\n"
" int y;\n"
"}",
Style);
}

TEST_F(FormatTest, ShortExportBlocks) {
FormatStyle Style = getLLVMStyleWithColumns(80);
Style.ExportBlockIndentation = false;

Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Never;
verifyFormat("export {\n"
"}",
Style);

verifyFormat("export {\n"
"int x;\n"
"}",
Style);

verifyFormat("export {\n"
"int x;\n"
"}",
"export\n"
"{\n"
"int x;\n"
"}",
Style);

verifyFormat("export {\n"
"}",
"export {}", Style);

verifyFormat("export {\n"
"int x;\n"
"}",
"export { int x; }", Style);

Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Always;
verifyFormat("export {}",
"export {\n"
"}",
Style);

verifyFormat("export { int x; }",
"export {\n"
"int x;\n"
"}",
Style);

verifyFormat("export { int x; }",
"export\n"
"{\n"
"int x;\n"
"}",
Style);

verifyFormat("export {}",
"export {\n"
"}",
Style);

verifyFormat("export { int x; }",
"export {\n"
"int x;\n"
"}",
Style);

Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Empty;
verifyFormat("export {}",
"export {\n"
"}",
Style);

verifyFormat("export {\n"
"int x;\n"
"}",
Style);

verifyFormat("export {\n"
"int x;\n"
"}",
"export\n"
"{\n"
"int x;\n"
"}",
Style);

verifyFormat("export {}", Style);

verifyFormat("export {\n"
"int x;\n"
"}",
"export { int x; }", Style);
}

TEST_F(FormatTest, FormatsBuilderPattern) {
verifyFormat("return llvm::StringSwitch<Reference::Kind>(name)\n"
" .StartsWith(\".eh_frame_hdr\", ORDER_EH_FRAMEHDR)\n"
Expand Down

0 comments on commit 106c483

Please sign in to comment.