Skip to content

Commit 106c483

Browse files
authored
[clang-format] Improve brace wrapping and add an option to control indentation 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.
1 parent eae5ca9 commit 106c483

File tree

9 files changed

+189
-22
lines changed

9 files changed

+189
-22
lines changed

clang/docs/ClangFormatStyleOptions.rst

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3946,6 +3946,21 @@ the configuration (without a prefix: ``Auto``).
39463946
This is an experimental flag, that might go away or be renamed. Do
39473947
not use this in config files, etc. Use at your own risk.
39483948

3949+
.. _ExportBlockIndentation:
3950+
3951+
**ExportBlockIndentation** (``Boolean``) :versionbadge:`clang-format 20` :ref:`<ExportBlockIndentation>`
3952+
If ``true``, clang-format will indent the body of an ``export { ... }``
3953+
block. This doesn't affect the formatting of anything else related to
3954+
exported declarations.
3955+
3956+
.. code-block:: c++
3957+
3958+
true: false:
3959+
export { vs. export {
3960+
void foo(); void foo();
3961+
void bar(); void bar();
3962+
} }
3963+
39493964
.. _FixNamespaceComments:
39503965

39513966
**FixNamespaceComments** (``Boolean``) :versionbadge:`clang-format 5` :ref:`<FixNamespaceComments>`

clang/docs/ReleaseNotes.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1224,6 +1224,7 @@ clang-format
12241224
- Adds ``VariableTemplates`` option.
12251225
- Adds support for bash globstar in ``.clang-format-ignore``.
12261226
- Adds ``WrapNamespaceBodyWithEmptyLines`` option.
1227+
- Adds the ``ExportBlockIndentation`` option.
12271228

12281229
libclang
12291230
--------

clang/include/clang/Format/Format.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2676,6 +2676,19 @@ struct FormatStyle {
26762676
/// \version 3.7
26772677
bool ExperimentalAutoDetectBinPacking;
26782678

2679+
/// If ``true``, clang-format will indent the body of an ``export { ... }``
2680+
/// block. This doesn't affect the formatting of anything else related to
2681+
/// exported declarations.
2682+
/// \code
2683+
/// true: false:
2684+
/// export { vs. export {
2685+
/// void foo(); void foo();
2686+
/// void bar(); void bar();
2687+
/// } }
2688+
/// \endcode
2689+
/// \version 20
2690+
bool ExportBlockIndentation;
2691+
26792692
/// If ``true``, clang-format adds missing namespace end comments for
26802693
/// namespaces and fixes invalid existing ones. This doesn't affect short
26812694
/// namespaces, which are controlled by ``ShortNamespaceLines``.
@@ -5254,6 +5267,7 @@ struct FormatStyle {
52545267
EmptyLineBeforeAccessModifier == R.EmptyLineBeforeAccessModifier &&
52555268
ExperimentalAutoDetectBinPacking ==
52565269
R.ExperimentalAutoDetectBinPacking &&
5270+
ExportBlockIndentation == R.ExportBlockIndentation &&
52575271
FixNamespaceComments == R.FixNamespaceComments &&
52585272
ForEachMacros == R.ForEachMacros &&
52595273
IncludeStyle.IncludeBlocks == R.IncludeStyle.IncludeBlocks &&

clang/lib/Format/Format.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1040,6 +1040,7 @@ template <> struct MappingTraits<FormatStyle> {
10401040
Style.EmptyLineBeforeAccessModifier);
10411041
IO.mapOptional("ExperimentalAutoDetectBinPacking",
10421042
Style.ExperimentalAutoDetectBinPacking);
1043+
IO.mapOptional("ExportBlockIndentation", Style.ExportBlockIndentation);
10431044
IO.mapOptional("FixNamespaceComments", Style.FixNamespaceComments);
10441045
IO.mapOptional("ForEachMacros", Style.ForEachMacros);
10451046
IO.mapOptional("IfMacros", Style.IfMacros);
@@ -1550,6 +1551,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
15501551
LLVMStyle.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Never;
15511552
LLVMStyle.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_LogicalBlock;
15521553
LLVMStyle.ExperimentalAutoDetectBinPacking = false;
1554+
LLVMStyle.ExportBlockIndentation = true;
15531555
LLVMStyle.FixNamespaceComments = true;
15541556
LLVMStyle.ForEachMacros.push_back("foreach");
15551557
LLVMStyle.ForEachMacros.push_back("Q_FOREACH");

clang/lib/Format/TokenAnnotator.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,11 @@ class AnnotatedLine {
154154
startsWith(tok::kw_export, tok::kw_namespace);
155155
}
156156

157+
/// \c true if this line starts a C++ export block.
158+
bool startsWithExportBlock() const {
159+
return startsWith(tok::kw_export, tok::l_brace);
160+
}
161+
157162
FormatToken *getFirstNonComment() const {
158163
assert(First);
159164
return First->is(tok::comment) ? First->getNextNonComment() : First;

clang/lib/Format/UnwrappedLineFormatter.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -432,8 +432,9 @@ class LineJoiner {
432432

433433
// Try to merge a control statement block with left brace unwrapped.
434434
if (TheLine->Last->is(tok::l_brace) && FirstNonComment != TheLine->Last &&
435-
FirstNonComment->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for,
436-
TT_ForEachMacro)) {
435+
(FirstNonComment->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for,
436+
TT_ForEachMacro) ||
437+
TheLine->startsWithExportBlock())) {
437438
return Style.AllowShortBlocksOnASingleLine != FormatStyle::SBS_Never
438439
? tryMergeSimpleBlock(I, E, Limit)
439440
: 0;
@@ -832,7 +833,8 @@ class LineJoiner {
832833
if (IsCtrlStmt(Line) ||
833834
Line.First->isOneOf(tok::kw_try, tok::kw___try, tok::kw_catch,
834835
tok::kw___finally, tok::r_brace,
835-
Keywords.kw___except)) {
836+
Keywords.kw___except) ||
837+
Line.startsWithExportBlock()) {
836838
if (IsSplitBlock)
837839
return 0;
838840
// Don't merge when we can't except the case when

clang/lib/Format/UnwrappedLineParser.cpp

Lines changed: 30 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1625,6 +1625,10 @@ void UnwrappedLineParser::parseStructuralElement(
16251625
parseNamespace();
16261626
return;
16271627
}
1628+
if (FormatTok->is(tok::l_brace)) {
1629+
parseCppExportBlock();
1630+
return;
1631+
}
16281632
if (FormatTok->is(Keywords.kw_import) && parseModuleImport())
16291633
return;
16301634
}
@@ -3105,6 +3109,26 @@ void UnwrappedLineParser::parseTryCatch() {
31053109
addUnwrappedLine();
31063110
}
31073111

3112+
void UnwrappedLineParser::parseNamespaceOrExportBlock(unsigned AddLevels) {
3113+
bool ManageWhitesmithsBraces =
3114+
AddLevels == 0u && Style.BreakBeforeBraces == FormatStyle::BS_Whitesmiths;
3115+
3116+
// If we're in Whitesmiths mode, indent the brace if we're not indenting
3117+
// the whole block.
3118+
if (ManageWhitesmithsBraces)
3119+
++Line->Level;
3120+
3121+
// Munch the semicolon after the block. This is more common than one would
3122+
// think. Putting the semicolon into its own line is very ugly.
3123+
parseBlock(/*MustBeDeclaration=*/true, AddLevels, /*MunchSemi=*/true,
3124+
/*KeepBraces=*/true, /*IfKind=*/nullptr, ManageWhitesmithsBraces);
3125+
3126+
addUnwrappedLine(AddLevels > 0 ? LineLevel::Remove : LineLevel::Keep);
3127+
3128+
if (ManageWhitesmithsBraces)
3129+
--Line->Level;
3130+
}
3131+
31083132
void UnwrappedLineParser::parseNamespace() {
31093133
assert(FormatTok->isOneOf(tok::kw_namespace, TT_NamespaceMacro) &&
31103134
"'namespace' expected");
@@ -3137,29 +3161,16 @@ void UnwrappedLineParser::parseNamespace() {
31373161
DeclarationScopeStack.size() > 1)
31383162
? 1u
31393163
: 0u;
3140-
bool ManageWhitesmithsBraces =
3141-
AddLevels == 0u &&
3142-
Style.BreakBeforeBraces == FormatStyle::BS_Whitesmiths;
3143-
3144-
// If we're in Whitesmiths mode, indent the brace if we're not indenting
3145-
// the whole block.
3146-
if (ManageWhitesmithsBraces)
3147-
++Line->Level;
3148-
3149-
// Munch the semicolon after a namespace. This is more common than one would
3150-
// think. Putting the semicolon into its own line is very ugly.
3151-
parseBlock(/*MustBeDeclaration=*/true, AddLevels, /*MunchSemi=*/true,
3152-
/*KeepBraces=*/true, /*IfKind=*/nullptr,
3153-
ManageWhitesmithsBraces);
3154-
3155-
addUnwrappedLine(AddLevels > 0 ? LineLevel::Remove : LineLevel::Keep);
3156-
3157-
if (ManageWhitesmithsBraces)
3158-
--Line->Level;
3164+
parseNamespaceOrExportBlock(AddLevels);
31593165
}
31603166
// FIXME: Add error handling.
31613167
}
31623168

3169+
void UnwrappedLineParser::parseCppExportBlock() {
3170+
parseNamespaceOrExportBlock(/*AddLevels=*/Style.ExportBlockIndentation ? 1
3171+
: 0);
3172+
}
3173+
31633174
void UnwrappedLineParser::parseNew() {
31643175
assert(FormatTok->is(tok::kw_new) && "'new' expected");
31653176
nextToken();

clang/lib/Format/UnwrappedLineParser.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,8 @@ class UnwrappedLineParser {
171171
void parseRequiresClause(FormatToken *RequiresToken);
172172
void parseRequiresExpression(FormatToken *RequiresToken);
173173
void parseConstraintExpression();
174+
void parseCppExportBlock();
175+
void parseNamespaceOrExportBlock(unsigned AddLevels);
174176
void parseJavaEnumBody();
175177
// Parses a record (aka class) as a top level element. If ParseAsExpr is true,
176178
// parses the record as a child block, i.e. if the class declaration is an

clang/unittests/Format/FormatTest.cpp

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9059,6 +9059,121 @@ TEST_F(FormatTest, AdaptiveOnePerLineFormatting) {
90599059
Style);
90609060
}
90619061

9062+
TEST_F(FormatTest, ExportBlockIndentation) {
9063+
FormatStyle Style = getLLVMStyleWithColumns(80);
9064+
Style.ExportBlockIndentation = true;
9065+
verifyFormat("export {\n"
9066+
" int x;\n"
9067+
" int y;\n"
9068+
"}",
9069+
"export {\n"
9070+
"int x;\n"
9071+
"int y;\n"
9072+
"}",
9073+
Style);
9074+
9075+
Style.ExportBlockIndentation = false;
9076+
verifyFormat("export {\n"
9077+
"int x;\n"
9078+
"int y;\n"
9079+
"}",
9080+
"export {\n"
9081+
" int x;\n"
9082+
" int y;\n"
9083+
"}",
9084+
Style);
9085+
}
9086+
9087+
TEST_F(FormatTest, ShortExportBlocks) {
9088+
FormatStyle Style = getLLVMStyleWithColumns(80);
9089+
Style.ExportBlockIndentation = false;
9090+
9091+
Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Never;
9092+
verifyFormat("export {\n"
9093+
"}",
9094+
Style);
9095+
9096+
verifyFormat("export {\n"
9097+
"int x;\n"
9098+
"}",
9099+
Style);
9100+
9101+
verifyFormat("export {\n"
9102+
"int x;\n"
9103+
"}",
9104+
"export\n"
9105+
"{\n"
9106+
"int x;\n"
9107+
"}",
9108+
Style);
9109+
9110+
verifyFormat("export {\n"
9111+
"}",
9112+
"export {}", Style);
9113+
9114+
verifyFormat("export {\n"
9115+
"int x;\n"
9116+
"}",
9117+
"export { int x; }", Style);
9118+
9119+
Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Always;
9120+
verifyFormat("export {}",
9121+
"export {\n"
9122+
"}",
9123+
Style);
9124+
9125+
verifyFormat("export { int x; }",
9126+
"export {\n"
9127+
"int x;\n"
9128+
"}",
9129+
Style);
9130+
9131+
verifyFormat("export { int x; }",
9132+
"export\n"
9133+
"{\n"
9134+
"int x;\n"
9135+
"}",
9136+
Style);
9137+
9138+
verifyFormat("export {}",
9139+
"export {\n"
9140+
"}",
9141+
Style);
9142+
9143+
verifyFormat("export { int x; }",
9144+
"export {\n"
9145+
"int x;\n"
9146+
"}",
9147+
Style);
9148+
9149+
Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Empty;
9150+
verifyFormat("export {}",
9151+
"export {\n"
9152+
"}",
9153+
Style);
9154+
9155+
verifyFormat("export {\n"
9156+
"int x;\n"
9157+
"}",
9158+
Style);
9159+
9160+
verifyFormat("export {\n"
9161+
"int x;\n"
9162+
"}",
9163+
"export\n"
9164+
"{\n"
9165+
"int x;\n"
9166+
"}",
9167+
Style);
9168+
9169+
verifyFormat("export {}", Style);
9170+
9171+
verifyFormat("export {\n"
9172+
"int x;\n"
9173+
"}",
9174+
"export { int x; }", Style);
9175+
}
9176+
90629177
TEST_F(FormatTest, FormatsBuilderPattern) {
90639178
verifyFormat("return llvm::StringSwitch<Reference::Kind>(name)\n"
90649179
" .StartsWith(\".eh_frame_hdr\", ORDER_EH_FRAMEHDR)\n"

0 commit comments

Comments
 (0)