[clang] [clang-format] Add an option to control indentation of `export { ... }` (PR #110381)
via cfe-commits
cfe-commits at lists.llvm.org
Sat Sep 28 11:52:38 PDT 2024
https://github.com/Sirraide created https://github.com/llvm/llvm-project/pull/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 my first contribution to clang-format (I normally contribute to Clang), so I’m not entirely sure if what I did is the best approach.
I based this on the `NamespaceIndentation` option, except that I didn’t include an option to control the behaviour of export blocks when nested because nesting them doesn’t really make sense (now that I think about it, iirc they could be nested inside something else though—e.g. a namespace—do we care about that case?).
>From 570aff7459311a43cd9c9139de05dc2ab4cf762c Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Sat, 28 Sep 2024 20:36:38 +0200
Subject: [PATCH 1/2] [clang-format] Add an option to control indentation of
`export { ... }`
---
clang/docs/ClangFormatStyleOptions.rst | 15 ++++++++
clang/include/clang/Format/Format.h | 14 +++++++
clang/lib/Format/Format.cpp | 1 +
clang/lib/Format/UnwrappedLineParser.cpp | 48 ++++++++++++++----------
clang/lib/Format/UnwrappedLineParser.h | 2 +
clang/unittests/Format/FormatTest.cpp | 25 ++++++++++++
6 files changed, 86 insertions(+), 19 deletions(-)
diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst
index a427d7cd40fcdd..7bdb1d3383e056 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -3828,6 +3828,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>`
diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
index d8b62c7652a0f6..238a082e0f3670 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -2655,6 +2655,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``.
@@ -5131,6 +5144,7 @@ struct FormatStyle {
EmptyLineBeforeAccessModifier == R.EmptyLineBeforeAccessModifier &&
ExperimentalAutoDetectBinPacking ==
R.ExperimentalAutoDetectBinPacking &&
+ ExportBlockIndentation == R.ExportBlockIndentation &&
FixNamespaceComments == R.FixNamespaceComments &&
ForEachMacros == R.ForEachMacros &&
IncludeStyle.IncludeBlocks == R.IncludeStyle.IncludeBlocks &&
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index d2463b892fbb96..ee584b1ad0d1e5 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -1016,6 +1016,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);
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index 40f77266fabdca..306747290c0a7a 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -1616,6 +1616,10 @@ void UnwrappedLineParser::parseStructuralElement(
parseNamespace();
return;
}
+ if (FormatTok->is(tok::l_brace)) {
+ parseCXXExportBlock();
+ return;
+ }
if (FormatTok->is(Keywords.kw_import) && parseModuleImport())
return;
}
@@ -3075,6 +3079,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 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;
+}
+
void UnwrappedLineParser::parseNamespace() {
assert(FormatTok->isOneOf(tok::kw_namespace, TT_NamespaceMacro) &&
"'namespace' expected");
@@ -3107,29 +3131,15 @@ 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::parseCXXExportBlock() {
+ parseNamespaceOrExportBlock(Style.ExportBlockIndentation ? 1 : 0);
+}
+
void UnwrappedLineParser::parseNew() {
assert(FormatTok->is(tok::kw_new) && "'new' expected");
nextToken();
diff --git a/clang/lib/Format/UnwrappedLineParser.h b/clang/lib/Format/UnwrappedLineParser.h
index b7daf8d9f44012..e5cfe4f6810791 100644
--- a/clang/lib/Format/UnwrappedLineParser.h
+++ b/clang/lib/Format/UnwrappedLineParser.h
@@ -171,6 +171,8 @@ class UnwrappedLineParser {
void parseRequiresClause(FormatToken *RequiresToken);
void parseRequiresExpression(FormatToken *RequiresToken);
void parseConstraintExpression();
+ void parseCXXExportBlock();
+ 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
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 5d386c1bbdbcd9..add52cd7460a48 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -9040,6 +9040,31 @@ 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"
+ "}\n",
+ "export {\n"
+ " int x;\n"
+ " int y;\n"
+ "}\n",
+ Style);
+
+ Style.ExportBlockIndentation = false;
+ verifyFormat("export {\n"
+ " int x;\n"
+ " int y;\n"
+ "}\n",
+ "export {\n"
+ "int x;\n"
+ "int y;\n"
+ "}\n",
+ Style);
+}
+
TEST_F(FormatTest, FormatsBuilderPattern) {
verifyFormat("return llvm::StringSwitch<Reference::Kind>(name)\n"
" .StartsWith(\".eh_frame_hdr\", ORDER_EH_FRAMEHDR)\n"
>From 21b05638e33606e03dc218315e4a69bb15244333 Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Sat, 28 Sep 2024 20:47:48 +0200
Subject: [PATCH 2/2] Add release note
---
clang/docs/ReleaseNotes.rst | 1 +
1 file changed, 1 insertion(+)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 14907e7db18de3..2588523e326d13 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -554,6 +554,7 @@ clang-format
------------
- Adds ``BreakBinaryOperations`` option.
+- Adds the ``ExportBlockIndentation`` option.
libclang
--------
More information about the cfe-commits
mailing list