galenelias created this revision.
galenelias added a project: clang-format.
Herald added projects: All, clang.
Herald added a subscriber: cfe-commits.
Herald added reviewers: rymiel, HazardyKnusperkeks, owenpan, MyDeveloperDay.
galenelias requested review of this revision.
This resolves https://github.com/llvm/llvm-project/issues/55475.
This adds a new `AlignConsecutiveShortCaseLabels` option in line with the
existing AlignConsecutiveStyle options, which when
`AllowShortCaseLabelsOnASingleLine` is enabled, will align the tokens after the
case statement's colon.
For the implementation, I went with re-using the `AlignTokens` method, however
since consecutive case labels require traversing scopes, the default behavior
to recurse upon entering a new scope (and breaking formatting across scopes)
doesn't work. I opted to add a new `IgnoreNestedScopes` parameter to prevent
this scope traversal, but it feels a touch hacky.
The other options would be to either write a new version of the alignment code,
or to try to leverage one of the other existing aligning functions (seems the
other one-off ones are `alignTrailingComments` and `alignConsecutiveMacros`).
Using the logic from `WhitespaceManager::alignConsecutiveMacros` technically
seemed to work here, so I could factor the token matching function out of that
and re-use it without any other edits, but other than not traversing scopes, I
wasn't quite sure what special logic it was doing that might not make sense to
re-use generically, so opted for trying to leverage the more general purpose
AlignTokens instead.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D151761
Files:
clang/docs/ClangFormatStyleOptions.rst
clang/docs/ReleaseNotes.rst
clang/include/clang/Format/Format.h
clang/lib/Format/Format.cpp
clang/lib/Format/WhitespaceManager.cpp
clang/lib/Format/WhitespaceManager.h
clang/unittests/Format/ConfigParseTest.cpp
clang/unittests/Format/FormatTest.cpp
Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -19192,6 +19192,68 @@
BracedAlign);
}
+TEST_F(FormatTest, AlignConsecutiveShortCaseLabels) {
+ FormatStyle Alignment = getLLVMStyle();
+ Alignment.AllowShortCaseLabelsOnASingleLine = true;
+ Alignment.AlignConsecutiveShortCaseLabels.Enabled = true;
+ verifyFormat("switch (level) {\n"
+ "case log::info: return \"info\";\n"
+ "case log::warning: return \"warning\";\n"
+ "default: return \"default\";\n"
+ "}",
+ Alignment);
+
+ // Make sure we don't incorrectly align correctly across nested switch cases
+ EXPECT_EQ("switch (level) {\n"
+ "case log::info: return \"info\";\n"
+ "case log::warning: return \"warning\";\n"
+ "case log::other:\n"
+ " switch (sublevel) {\n"
+ " case log::info: return \"info\";\n"
+ " case log::warning: return \"warning\";\n"
+ " }\n"
+ " break;\n"
+ "case log::error: return \"error\";\n"
+ "default: return \"default\";\n"
+ "}",
+ format("switch (level) {\n"
+ "case log::info: return \"info\";\n"
+ "case log::warning: return \"warning\";\n"
+ "case log::other: switch (sublevel) {\n"
+ " case log::info: return \"info\";\n"
+ " case log::warning: return \"warning\";\n"
+ "}\n"
+ "break;\n"
+ "case log::error: return \"error\";\n"
+ "default: return \"default\";\n"
+ "}",
+ Alignment));
+
+ Alignment.AlignConsecutiveShortCaseLabels.AcrossEmptyLines = true;
+
+ EXPECT_EQ("switch (level) {\n"
+ "case log::info: return \"info\";\n"
+ "\n"
+ "case log::warning: return \"warning\";\n"
+ "}",
+ format("switch (level) {\n"
+ "case log::info: return \"info\";\n"
+ "\n"
+ "case log::warning: return \"warning\";\n"
+ "}",
+ Alignment));
+
+ Alignment.AlignConsecutiveShortCaseLabels.AcrossEmptyLines = false;
+ Alignment.AlignConsecutiveShortCaseLabels.AcrossComments = true;
+
+ verifyFormat("switch (level) {\n"
+ "case log::info: return \"info\";\n"
+ "//\n"
+ "case log::warning: return \"warning\";\n"
+ "}",
+ Alignment);
+}
+
TEST_F(FormatTest, AlignWithLineBreaks) {
auto Style = getLLVMStyleWithColumns(120);
Index: clang/unittests/Format/ConfigParseTest.cpp
===================================================================
--- clang/unittests/Format/ConfigParseTest.cpp
+++ clang/unittests/Format/ConfigParseTest.cpp
@@ -317,6 +317,7 @@
CHECK_ALIGN_CONSECUTIVE(AlignConsecutiveBitFields);
CHECK_ALIGN_CONSECUTIVE(AlignConsecutiveMacros);
CHECK_ALIGN_CONSECUTIVE(AlignConsecutiveDeclarations);
+ CHECK_ALIGN_CONSECUTIVE(AlignConsecutiveShortCaseLabels);
#undef CHECK_ALIGN_CONSECUTIVE
Index: clang/lib/Format/WhitespaceManager.h
===================================================================
--- clang/lib/Format/WhitespaceManager.h
+++ clang/lib/Format/WhitespaceManager.h
@@ -232,6 +232,9 @@
/// Align consecutive declarations over all \c Changes.
void alignChainedConditionals();
+ /// Align consecutive short case labels over all \c Changes.
+ void alignConsecutiveShortCaseLabels();
+
/// Align trailing comments over all \c Changes.
void alignTrailingComments();
Index: clang/lib/Format/WhitespaceManager.cpp
===================================================================
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -113,6 +113,7 @@
alignConsecutiveDeclarations();
alignConsecutiveBitFields();
alignConsecutiveAssignments();
+ alignConsecutiveShortCaseLabels();
alignChainedConditionals();
alignTrailingComments();
alignEscapedNewlines();
@@ -281,7 +282,8 @@
template <typename F>
static void
AlignTokenSequence(const FormatStyle &Style, unsigned Start, unsigned End,
- unsigned Column, bool RightJustify, F &&Matches,
+ unsigned Column, bool RightJustify, bool IgnoreNestedScopes,
+ F &&Matches,
SmallVector<WhitespaceManager::Change, 16> &Changes) {
bool FoundMatchOnLine = false;
int Shift = 0;
@@ -309,7 +311,7 @@
SmallVector<unsigned, 16> ScopeStack;
for (unsigned i = Start; i != End; ++i) {
- if (ScopeStack.size() != 0 &&
+ if (!IgnoreNestedScopes && ScopeStack.size() != 0 &&
Changes[i].indentAndNestingLevel() <
Changes[ScopeStack.back()].indentAndNestingLevel()) {
ScopeStack.pop_back();
@@ -322,8 +324,9 @@
Changes[PreviousNonComment].Tok->is(tok::comment)) {
--PreviousNonComment;
}
- if (i != Start && Changes[i].indentAndNestingLevel() >
- Changes[PreviousNonComment].indentAndNestingLevel()) {
+ if (!IgnoreNestedScopes && i != Start &&
+ Changes[i].indentAndNestingLevel() >
+ Changes[PreviousNonComment].indentAndNestingLevel()) {
ScopeStack.push_back(i);
}
@@ -498,12 +501,16 @@
// right-justified. It is used to align compound assignments like `+=` and `=`.
// When RightJustify and ACS.PadOperators are true, operators in each block to
// be aligned will be padded on the left to the same length before aligning.
+// The special processing of nested scopes can be disabled by passing
+// 'IgnoreNestedScopes', which is needed when aligning tokens which span scopes
+// such as case labels.
template <typename F>
static unsigned AlignTokens(const FormatStyle &Style, F &&Matches,
SmallVector<WhitespaceManager::Change, 16> &Changes,
unsigned StartAt,
const FormatStyle::AlignConsecutiveStyle &ACS = {},
- bool RightJustify = false) {
+ bool RightJustify = false,
+ bool IgnoreNestedScopes = false) {
// We arrange each line in 3 parts. The operator to be aligned (the anchor),
// and text to its left and right. In the aligned text the width of each part
// will be the maximum of that over the block that has been aligned. Maximum
@@ -552,8 +559,8 @@
auto AlignCurrentSequence = [&] {
if (StartOfSequence > 0 && StartOfSequence < EndOfSequence) {
AlignTokenSequence(Style, StartOfSequence, EndOfSequence,
- WidthLeft + WidthAnchor, RightJustify, Matches,
- Changes);
+ WidthLeft + WidthAnchor, RightJustify,
+ IgnoreNestedScopes, Matches, Changes);
}
WidthLeft = 0;
WidthAnchor = 0;
@@ -564,8 +571,10 @@
unsigned i = StartAt;
for (unsigned e = Changes.size(); i != e; ++i) {
- if (Changes[i].indentAndNestingLevel() < IndentAndNestingLevel)
+ if (!IgnoreNestedScopes &&
+ Changes[i].indentAndNestingLevel() < IndentAndNestingLevel) {
break;
+ }
if (Changes[i].NewlinesBefore != 0) {
CommasBeforeMatch = 0;
@@ -597,10 +606,11 @@
if (Changes[i].Tok->is(tok::comma)) {
++CommasBeforeMatch;
- } else if (Changes[i].indentAndNestingLevel() > IndentAndNestingLevel) {
+ } else if (!IgnoreNestedScopes &&
+ Changes[i].indentAndNestingLevel() > IndentAndNestingLevel) {
// Call AlignTokens recursively, skipping over this scope block.
- unsigned StoppedAt =
- AlignTokens(Style, Matches, Changes, i, ACS, RightJustify);
+ unsigned StoppedAt = AlignTokens(Style, Matches, Changes, i, ACS,
+ RightJustify, IgnoreNestedScopes);
i = StoppedAt - 1;
continue;
}
@@ -859,6 +869,20 @@
Changes, /*StartAt=*/0, Style.AlignConsecutiveBitFields);
}
+void WhitespaceManager::alignConsecutiveShortCaseLabels() {
+ if (!Style.AlignConsecutiveShortCaseLabels.Enabled)
+ return;
+
+ AlignTokens(
+ Style,
+ [&](Change const &C) {
+ return (C.Tok->SpacesRequiredBefore != 0 && C.Tok->Previous &&
+ C.Tok->Previous->is(TT_CaseLabelColon));
+ },
+ Changes, /*StartAt=*/0, Style.AlignConsecutiveShortCaseLabels,
+ /*RightJustify=*/false, /*IgnoreNestedScopes=*/true);
+}
+
void WhitespaceManager::alignConsecutiveDeclarations() {
if (!Style.AlignConsecutiveDeclarations.Enabled)
return;
Index: clang/lib/Format/Format.cpp
===================================================================
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -846,6 +846,8 @@
IO.mapOptional("AlignConsecutiveDeclarations",
Style.AlignConsecutiveDeclarations);
IO.mapOptional("AlignConsecutiveMacros", Style.AlignConsecutiveMacros);
+ IO.mapOptional("AlignConsecutiveShortCaseLabels",
+ Style.AlignConsecutiveShortCaseLabels);
IO.mapOptional("AlignEscapedNewlines", Style.AlignEscapedNewlines);
IO.mapOptional("AlignOperands", Style.AlignOperands);
IO.mapOptional("AlignTrailingComments", Style.AlignTrailingComments);
@@ -1319,6 +1321,7 @@
LLVMStyle.AlignConsecutiveBitFields = {};
LLVMStyle.AlignConsecutiveDeclarations = {};
LLVMStyle.AlignConsecutiveMacros = {};
+ LLVMStyle.AlignConsecutiveShortCaseLabels = {};
LLVMStyle.AlignTrailingComments = {};
LLVMStyle.AlignTrailingComments.Kind = FormatStyle::TCAS_Always;
LLVMStyle.AlignTrailingComments.OverEmptyLines = 0;
Index: clang/include/clang/Format/Format.h
===================================================================
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -295,6 +295,17 @@
/// \endcode
/// \version 3.8
AlignConsecutiveStyle AlignConsecutiveDeclarations;
+ /// Style of aligning consecutive short case labels.
+ /// Only applies if ``AllowShortCaseLabelsOnASingleLine`` is ``true``.
+ ///
+ /// ``Consecutive`` will result in formattings like:
+ /// \code
+ /// case log::info: return "info:";
+ /// case log::warning: return "warning:";
+ /// default: return "";
+ /// \endcode
+ /// \version 17
+ AlignConsecutiveStyle AlignConsecutiveShortCaseLabels;
/// Different styles for aligning escaped newlines.
enum EscapedNewlineAlignmentStyle : int8_t {
@@ -4292,6 +4303,8 @@
AlignConsecutiveBitFields == R.AlignConsecutiveBitFields &&
AlignConsecutiveDeclarations == R.AlignConsecutiveDeclarations &&
AlignConsecutiveMacros == R.AlignConsecutiveMacros &&
+ AlignConsecutiveShortCaseLabels ==
+ R.AlignConsecutiveShortCaseLabels &&
AlignEscapedNewlines == R.AlignEscapedNewlines &&
AlignOperands == R.AlignOperands &&
AlignTrailingComments == R.AlignTrailingComments &&
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -657,6 +657,8 @@
- Fix all known issues associated with ``LambdaBodyIndentation: OuterScope``.
- Add ``BracedInitializerIndentWidth`` which can be used to configure
the indentation level of the contents of braced init lists.
+- Add ``AlignConsecutiveShortCaseLabels`` which can be used to align case
+ labels in conjunction with ``AllowShortCaseLabelsOnASingleLine``.
libclang
--------
Index: clang/docs/ClangFormatStyleOptions.rst
===================================================================
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -785,6 +785,131 @@
bbb >>= 2;
+.. _AlignConsecutiveShortCaseLabels:
+
+**AlignConsecutiveShortCaseLabels** (``AlignConsecutiveStyle``) :versionbadge:`clang-format 17` :ref:`¶ <AlignConsecutiveShortCaseLabels>`
+ Style of aligning consecutive short case labels.
+ Only applies if ``AllowShortCaseLabelsOnASingleLine`` is ``true``.
+
+ ``Consecutive`` will result in formattings like:
+
+ .. code-block:: c++
+
+ case log::info: return "info:";
+ case log::warning: return "warning:";
+ default: return "";
+
+ Nested configuration flags:
+
+ Alignment options.
+
+ They can also be read as a whole for compatibility. The choices are:
+ - None
+ - Consecutive
+ - AcrossEmptyLines
+ - AcrossComments
+ - AcrossEmptyLinesAndComments
+
+ For example, to align across empty lines and not across comments, either
+ of these work.
+
+ .. code-block:: c++
+
+ AlignConsecutiveMacros: AcrossEmptyLines
+
+ AlignConsecutiveMacros:
+ Enabled: true
+ AcrossEmptyLines: true
+ AcrossComments: false
+
+ * ``bool Enabled`` Whether aligning is enabled.
+
+ .. code-block:: c++
+
+ #define SHORT_NAME 42
+ #define LONGER_NAME 0x007f
+ #define EVEN_LONGER_NAME (2)
+ #define foo(x) (x * x)
+ #define bar(y, z) (y + z)
+
+ int a = 1;
+ int somelongname = 2;
+ double c = 3;
+
+ int aaaa : 1;
+ int b : 12;
+ int ccc : 8;
+
+ int aaaa = 12;
+ float b = 23;
+ std::string ccc;
+
+ * ``bool AcrossEmptyLines`` Whether to align across empty lines.
+
+ .. code-block:: c++
+
+ true:
+ int a = 1;
+ int somelongname = 2;
+ double c = 3;
+
+ int d = 3;
+
+ false:
+ int a = 1;
+ int somelongname = 2;
+ double c = 3;
+
+ int d = 3;
+
+ * ``bool AcrossComments`` Whether to align across comments.
+
+ .. code-block:: c++
+
+ true:
+ int d = 3;
+ /* A comment. */
+ double e = 4;
+
+ false:
+ int d = 3;
+ /* A comment. */
+ double e = 4;
+
+ * ``bool AlignCompound`` Only for ``AlignConsecutiveAssignments``. Whether compound assignments
+ like ``+=`` are aligned along with ``=``.
+
+ .. code-block:: c++
+
+ true:
+ a &= 2;
+ bbb = 2;
+
+ false:
+ a &= 2;
+ bbb = 2;
+
+ * ``bool PadOperators`` Only for ``AlignConsecutiveAssignments``. Whether short assignment
+ operators are left-padded to the same length as long ones in order to
+ put all assignment operators to the right of the left hand side.
+
+ .. code-block:: c++
+
+ true:
+ a >>= 2;
+ bbb = 2;
+
+ a = 2;
+ bbb >>= 2;
+
+ false:
+ a >>= 2;
+ bbb = 2;
+
+ a = 2;
+ bbb >>= 2;
+
+
.. _AlignEscapedNewlines:
**AlignEscapedNewlines** (``EscapedNewlineAlignmentStyle``) :versionbadge:`clang-format 5` :ref:`¶ <AlignEscapedNewlines>`
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
