https://github.com/owenca updated
https://github.com/llvm/llvm-project/pull/133576
>From 3b352123c47cb382539fefc1bcd49228c17d994f Mon Sep 17 00:00:00 2001
From: Owen Pan <owenpi...@gmail.com>
Date: Sat, 29 Mar 2025 00:30:49 -0700
Subject: [PATCH 1/3] [clang-format] Add an option for editing enum trailing
commas
---
clang/docs/ClangFormatStyleOptions.rst | 34 ++++++++++
clang/docs/ReleaseNotes.rst | 2 +
clang/include/clang/Format/Format.h | 28 ++++++++
clang/lib/Format/Format.cpp | 76 ++++++++++++++++++++++
clang/unittests/Format/ConfigParseTest.cpp | 8 +++
clang/unittests/Format/FormatTest.cpp | 32 +++++++++
6 files changed, 180 insertions(+)
diff --git a/clang/docs/ClangFormatStyleOptions.rst
b/clang/docs/ClangFormatStyleOptions.rst
index 9ecac68ae72bf..211bb3eeeb6e6 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -3976,6 +3976,40 @@ the configuration (without a prefix: ``Auto``).
+.. _EnumTrailingComma:
+
+**EnumTrailingComma** (``EnumTrailingCommaStyle``) :versionbadge:`clang-format
21` :ref:`¶ <EnumTrailingComma>`
+ Insert a comma (if missing) or remove the comma at the end of an ``enum``
+ enumerator list.
+
+ Possible values:
+
+ * ``ETC_Leave`` (in configuration: ``Leave``)
+ Don't insert or remove trailing commas.
+
+ .. code-block:: c++
+
+ enum { a, b, c, };
+ enum Color { red, green, blue };
+
+ * ``ETC_Insert`` (in configuration: ``Insert``)
+ Insert trailing commas.
+
+ .. code-block:: c++
+
+ enum { a, b, c, };
+ enum Color { red, green, blue, };
+
+ * ``ETC_Remove`` (in configuration: ``Remove``)
+ Remove trailing commas.
+
+ .. code-block:: c++
+
+ enum { a, b, c };
+ enum Color { red, green, blue };
+
+
+
.. _ExperimentalAutoDetectBinPacking:
**ExperimentalAutoDetectBinPacking** (``Boolean``) :versionbadge:`clang-format
3.7` :ref:`¶ <ExperimentalAutoDetectBinPacking>`
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 04ec2cfef679c..27f6a93e31643 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -480,6 +480,8 @@ clang-format
- Allow specifying the language (C, C++, or Objective-C) for a ``.h`` file by
adding a special comment (e.g. ``// clang-format Language: ObjC``) near the
top of the file.
+- Add ``EnumTrailingComma`` option for inserting/removing commas at the end of
+ ``enum`` enumerator lists.
libclang
--------
diff --git a/clang/include/clang/Format/Format.h
b/clang/include/clang/Format/Format.h
index fec47a248abb4..c39006c0d6361 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -2704,6 +2704,33 @@ struct FormatStyle {
/// \version 12
EmptyLineBeforeAccessModifierStyle EmptyLineBeforeAccessModifier;
+ /// Styles for ``enum`` trailing commas.
+ enum EnumTrailingCommaStyle : int8_t {
+ /// Don't insert or remove trailing commas.
+ /// \code
+ /// enum { a, b, c, };
+ /// enum Color { red, green, blue };
+ /// \endcode
+ ETC_Leave,
+ /// Insert trailing commas.
+ /// \code
+ /// enum { a, b, c, };
+ /// enum Color { red, green, blue, };
+ /// \endcode
+ ETC_Insert,
+ /// Remove trailing commas.
+ /// \code
+ /// enum { a, b, c };
+ /// enum Color { red, green, blue };
+ /// \endcode
+ ETC_Remove,
+ };
+
+ /// Insert a comma (if missing) or remove the comma at the end of an ``enum``
+ /// enumerator list.
+ /// \version 21
+ EnumTrailingCommaStyle EnumTrailingComma;
+
/// If ``true``, clang-format detects whether function calls and
/// definitions are formatted with one parameter per line.
///
@@ -5323,6 +5350,7 @@ struct FormatStyle {
DisableFormat == R.DisableFormat &&
EmptyLineAfterAccessModifier == R.EmptyLineAfterAccessModifier &&
EmptyLineBeforeAccessModifier == R.EmptyLineBeforeAccessModifier &&
+ EnumTrailingComma == R.EnumTrailingComma &&
ExperimentalAutoDetectBinPacking ==
R.ExperimentalAutoDetectBinPacking &&
FixNamespaceComments == R.FixNamespaceComments &&
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 28aea86139e0d..5a875b8693574 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -361,6 +361,15 @@ struct ScalarEnumerationTraits<
}
};
+template <>
+struct ScalarEnumerationTraits<FormatStyle::EnumTrailingCommaStyle> {
+ static void enumeration(IO &IO, FormatStyle::EnumTrailingCommaStyle &Value) {
+ IO.enumCase(Value, "Leave", FormatStyle::ETC_Leave);
+ IO.enumCase(Value, "Insert", FormatStyle::ETC_Insert);
+ IO.enumCase(Value, "Remove", FormatStyle::ETC_Remove);
+ }
+};
+
template <>
struct ScalarEnumerationTraits<FormatStyle::IndentExternBlockStyle> {
static void enumeration(IO &IO, FormatStyle::IndentExternBlockStyle &Value) {
@@ -1042,6 +1051,7 @@ template <> struct MappingTraits<FormatStyle> {
Style.EmptyLineAfterAccessModifier);
IO.mapOptional("EmptyLineBeforeAccessModifier",
Style.EmptyLineBeforeAccessModifier);
+ IO.mapOptional("EnumTrailingComma", Style.EnumTrailingComma);
IO.mapOptional("ExperimentalAutoDetectBinPacking",
Style.ExperimentalAutoDetectBinPacking);
IO.mapOptional("FixNamespaceComments", Style.FixNamespaceComments);
@@ -1558,6 +1568,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind
Language) {
LLVMStyle.DisableFormat = false;
LLVMStyle.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Never;
LLVMStyle.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_LogicalBlock;
+ LLVMStyle.EnumTrailingComma = FormatStyle::ETC_Leave;
LLVMStyle.ExperimentalAutoDetectBinPacking = false;
LLVMStyle.FixNamespaceComments = true;
LLVMStyle.ForEachMacros.push_back("foreach");
@@ -2415,6 +2426,64 @@ class SemiRemover : public TokenAnalyzer {
}
};
+class EnumTrailingCommaEditor : public TokenAnalyzer {
+public:
+ EnumTrailingCommaEditor(const Environment &Env, const FormatStyle &Style)
+ : TokenAnalyzer(Env, Style) {}
+
+ std::pair<tooling::Replacements, unsigned>
+ analyze(TokenAnnotator &Annotator,
+ SmallVectorImpl<AnnotatedLine *> &AnnotatedLines,
+ FormatTokenLexer &Tokens) override {
+ AffectedRangeMgr.computeAffectedLines(AnnotatedLines);
+ tooling::Replacements Result;
+ editEnumTrailingComma(AnnotatedLines, Result);
+ return {Result, 0};
+ }
+
+private:
+ void editEnumTrailingComma(SmallVectorImpl<AnnotatedLine *> &Lines,
+ tooling::Replacements &Result) {
+ const auto &SourceMgr = Env.getSourceManager();
+ for (auto *Line : Lines) {
+ if (!Line->Children.empty())
+ editEnumTrailingComma(Line->Children, Result);
+ if (!Line->Affected)
+ continue;
+ for (const auto *Token = Line->First; Token && !Token->Finalized;
+ Token = Token->Next) {
+ if (Token->isNot(TT_EnumRBrace))
+ continue;
+ const auto *BeforeRBrace = Token->getPreviousNonComment();
+ assert(BeforeRBrace);
+ if (BeforeRBrace->is(TT_EnumLBrace)) // Empty braces.
+ continue;
+ if (Style.EnumTrailingComma == FormatStyle::ETC_Insert) {
+ if (BeforeRBrace->isNot(tok::comma)) {
+ cantFail(Result.add(tooling::Replacement(
+ SourceMgr, BeforeRBrace->Tok.getEndLoc(), 0, ",")));
+ }
+ } else {
+ assert(Style.EnumTrailingComma == FormatStyle::ETC_Remove);
+ if (BeforeRBrace->isNot(tok::comma))
+ continue;
+ auto *Next = BeforeRBrace->Next;
+ SourceLocation Start;
+ if (Next->NewlinesBefore == 0) {
+ Start = BeforeRBrace->Tok.getLocation();
+ Next->WhitespaceRange = BeforeRBrace->WhitespaceRange;
+ } else {
+ Start = BeforeRBrace->WhitespaceRange.getBegin();
+ }
+ const auto &Range = CharSourceRange::getCharRange(
+ Start, BeforeRBrace->Tok.getEndLoc());
+ cantFail(Result.add(tooling::Replacement(SourceMgr, Range, " ")));
+ }
+ }
+ }
+ }
+};
+
class JavaScriptRequoter : public TokenAnalyzer {
public:
JavaScriptRequoter(const Environment &Env, const FormatStyle &Style)
@@ -3812,6 +3881,13 @@ reformat(const FormatStyle &Style, StringRef Code,
});
}
+ if (Style.EnumTrailingComma != FormatStyle::ETC_Leave) {
+ Passes.emplace_back([&](const Environment &Env) {
+ return EnumTrailingCommaEditor(Env, Expanded)
+ .process(/*SkipAnnotation=*/true);
+ });
+ }
+
if (Style.FixNamespaceComments) {
Passes.emplace_back([&](const Environment &Env) {
return NamespaceEndCommentsFixer(Env, Expanded).process();
diff --git a/clang/unittests/Format/ConfigParseTest.cpp
b/clang/unittests/Format/ConfigParseTest.cpp
index 287191d04d885..2b08b794792e9 100644
--- a/clang/unittests/Format/ConfigParseTest.cpp
+++ b/clang/unittests/Format/ConfigParseTest.cpp
@@ -520,6 +520,14 @@ TEST(ConfigParseTest, ParsesConfiguration) {
CHECK_PARSE("EmptyLineBeforeAccessModifier: Always",
EmptyLineBeforeAccessModifier, FormatStyle::ELBAMS_Always);
+ Style.EnumTrailingComma = FormatStyle::ETC_Insert;
+ CHECK_PARSE("EnumTrailingComma: Leave", EnumTrailingComma,
+ FormatStyle::ETC_Leave);
+ CHECK_PARSE("EnumTrailingComma: Insert", EnumTrailingComma,
+ FormatStyle::ETC_Insert);
+ CHECK_PARSE("EnumTrailingComma: Remove", EnumTrailingComma,
+ FormatStyle::ETC_Remove);
+
Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
CHECK_PARSE("AlignAfterOpenBracket: Align", AlignAfterOpenBracket,
FormatStyle::BAS_Align);
diff --git a/clang/unittests/Format/FormatTest.cpp
b/clang/unittests/Format/FormatTest.cpp
index 0b90bd360b758..4dfa135120605 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -27902,6 +27902,38 @@ TEST_F(FormatTest, RemoveSemicolon) {
verifyFormat("STRUCT(T, B) { int i; };", Style);
}
+TEST_F(FormatTest, EnumTrailingComma) {
+ constexpr StringRef Code("enum : int { /**/ };\n"
+ "enum {\n"
+ " a,\n"
+ " b,\n"
+ " c, //\n"
+ "};\n"
+ "enum Color { red, green, blue /**/ };");
+ verifyFormat(Code);
+
+ auto Style = getLLVMStyle();
+ Style.EnumTrailingComma = FormatStyle::ETC_Insert;
+ verifyFormat("enum : int { /**/ };\n"
+ "enum {\n"
+ " a,\n"
+ " b,\n"
+ " c, //\n"
+ "};\n"
+ "enum Color { red, green, blue, /**/ };",
+ Code, Style);
+
+ Style.EnumTrailingComma = FormatStyle::ETC_Remove;
+ verifyFormat("enum : int { /**/ };\n"
+ "enum {\n"
+ " a,\n"
+ " b,\n"
+ " c //\n"
+ "};\n"
+ "enum Color { red, green, blue /**/ };",
+ Code, Style);
+}
+
TEST_F(FormatTest, BreakAfterAttributes) {
constexpr StringRef Code("[[maybe_unused]] const int i;\n"
"[[foo([[]])]] [[maybe_unused]]\n"
>From 9e16e891e6c32aeb3f00a27c4b1757eb2635ef43 Mon Sep 17 00:00:00 2001
From: Owen Pan <owenpi...@gmail.com>
Date: Sat, 29 Mar 2025 15:28:18 -0700
Subject: [PATCH 2/3] Add a warning and refactor the code that removes/replaces
a token
---
clang/docs/ClangFormatStyleOptions.rst | 7 +++
clang/include/clang/Format/Format.h | 6 ++
clang/lib/Format/Format.cpp | 83 ++++++++------------------
3 files changed, 39 insertions(+), 57 deletions(-)
diff --git a/clang/docs/ClangFormatStyleOptions.rst
b/clang/docs/ClangFormatStyleOptions.rst
index 211bb3eeeb6e6..3f8a5f49313b2 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -3982,6 +3982,13 @@ the configuration (without a prefix: ``Auto``).
Insert a comma (if missing) or remove the comma at the end of an ``enum``
enumerator list.
+ .. warning::
+
+ Setting this option to any value other than ``Leave`` could lead to
+ incorrect code formatting due to clang-format's lack of complete semantic
+ information. As such, extra care should be taken to review code changes
+ made by this option.
+
Possible values:
* ``ETC_Leave`` (in configuration: ``Leave``)
diff --git a/clang/include/clang/Format/Format.h
b/clang/include/clang/Format/Format.h
index c39006c0d6361..cea5e257659d6 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -2728,6 +2728,12 @@ struct FormatStyle {
/// Insert a comma (if missing) or remove the comma at the end of an ``enum``
/// enumerator list.
+ /// \warning
+ /// Setting this option to any value other than ``Leave`` could lead to
+ /// incorrect code formatting due to clang-format's lack of complete
semantic
+ /// information. As such, extra care should be taken to review code changes
+ /// made by this option.
+ /// \endwarning
/// \version 21
EnumTrailingCommaStyle EnumTrailingComma;
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 5a875b8693574..a88419ec9db19 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -2214,6 +2214,21 @@ FormatStyle::GetLanguageStyle(FormatStyle::LanguageKind
Language) const {
namespace {
+void replaceToken(const FormatToken &Token, FormatToken *Next,
+ const SourceManager &SourceMgr, tooling::Replacements
&Result,
+ const char *Text = "") {
+ const auto &Tok = Token.Tok;
+ SourceLocation Start;
+ if (Next && Next->NewlinesBefore == 0 && Next->isNot(tok::eof)) {
+ Start = Tok.getLocation();
+ Next->WhitespaceRange = Token.WhitespaceRange;
+ } else {
+ Start = Token.WhitespaceRange.getBegin();
+ }
+ const auto &Range = CharSourceRange::getCharRange(Start, Tok.getEndLoc());
+ cantFail(Result.add(tooling::Replacement(SourceMgr, Range, Text)));
+}
+
class ParensRemover : public TokenAnalyzer {
public:
ParensRemover(const Environment &Env, const FormatStyle &Style)
@@ -2240,20 +2255,8 @@ class ParensRemover : public TokenAnalyzer {
continue;
for (const auto *Token = Line->First; Token && !Token->Finalized;
Token = Token->Next) {
- if (!Token->Optional || !Token->isOneOf(tok::l_paren, tok::r_paren))
- continue;
- auto *Next = Token->Next;
- assert(Next && Next->isNot(tok::eof));
- SourceLocation Start;
- if (Next->NewlinesBefore == 0) {
- Start = Token->Tok.getLocation();
- Next->WhitespaceRange = Token->WhitespaceRange;
- } else {
- Start = Token->WhitespaceRange.getBegin();
- }
- const auto &Range =
- CharSourceRange::getCharRange(Start, Token->Tok.getEndLoc());
- cantFail(Result.add(tooling::Replacement(SourceMgr, Range, " ")));
+ if (Token->Optional && Token->isOneOf(tok::l_paren, tok::r_paren))
+ replaceToken(*Token, Token->Next, SourceMgr, Result, " ");
}
}
}
@@ -2342,24 +2345,13 @@ class BracesRemover : public TokenAnalyzer {
const auto *NextLine = I + 1 == End ? nullptr : I[1];
for (const auto *Token = Line->First; Token && !Token->Finalized;
Token = Token->Next) {
- if (!Token->Optional)
- continue;
- if (!Token->isOneOf(tok::l_brace, tok::r_brace))
+ if (!Token->Optional || !Token->isOneOf(tok::l_brace, tok::r_brace))
continue;
auto *Next = Token->Next;
assert(Next || Token == Line->Last);
if (!Next && NextLine)
Next = NextLine->First;
- SourceLocation Start;
- if (Next && Next->NewlinesBefore == 0 && Next->isNot(tok::eof)) {
- Start = Token->Tok.getLocation();
- Next->WhitespaceRange = Token->WhitespaceRange;
- } else {
- Start = Token->WhitespaceRange.getBegin();
- }
- const auto &Range =
- CharSourceRange::getCharRange(Start, Token->Tok.getEndLoc());
- cantFail(Result.add(tooling::Replacement(SourceMgr, Range, "")));
+ replaceToken(*Token, Next, SourceMgr, Result);
}
}
}
@@ -2411,16 +2403,7 @@ class SemiRemover : public TokenAnalyzer {
assert(Next || Token == Line->Last);
if (!Next && NextLine)
Next = NextLine->First;
- SourceLocation Start;
- if (Next && Next->NewlinesBefore == 0 && Next->isNot(tok::eof)) {
- Start = Token->Tok.getLocation();
- Next->WhitespaceRange = Token->WhitespaceRange;
- } else {
- Start = Token->WhitespaceRange.getBegin();
- }
- const auto &Range =
- CharSourceRange::getCharRange(Start, Token->Tok.getEndLoc());
- cantFail(Result.add(tooling::Replacement(SourceMgr, Range, "")));
+ replaceToken(*Token, Next, SourceMgr, Result);
}
}
}
@@ -2458,26 +2441,12 @@ class EnumTrailingCommaEditor : public TokenAnalyzer {
assert(BeforeRBrace);
if (BeforeRBrace->is(TT_EnumLBrace)) // Empty braces.
continue;
- if (Style.EnumTrailingComma == FormatStyle::ETC_Insert) {
- if (BeforeRBrace->isNot(tok::comma)) {
- cantFail(Result.add(tooling::Replacement(
- SourceMgr, BeforeRBrace->Tok.getEndLoc(), 0, ",")));
- }
- } else {
- assert(Style.EnumTrailingComma == FormatStyle::ETC_Remove);
- if (BeforeRBrace->isNot(tok::comma))
- continue;
- auto *Next = BeforeRBrace->Next;
- SourceLocation Start;
- if (Next->NewlinesBefore == 0) {
- Start = BeforeRBrace->Tok.getLocation();
- Next->WhitespaceRange = BeforeRBrace->WhitespaceRange;
- } else {
- Start = BeforeRBrace->WhitespaceRange.getBegin();
- }
- const auto &Range = CharSourceRange::getCharRange(
- Start, BeforeRBrace->Tok.getEndLoc());
- cantFail(Result.add(tooling::Replacement(SourceMgr, Range, " ")));
+ if (BeforeRBrace->is(tok::comma)) {
+ if (Style.EnumTrailingComma == FormatStyle::ETC_Remove)
+ replaceToken(*BeforeRBrace, BeforeRBrace->Next, SourceMgr, Result);
+ } else if (Style.EnumTrailingComma == FormatStyle::ETC_Insert) {
+ cantFail(Result.add(tooling::Replacement(
+ SourceMgr, BeforeRBrace->Tok.getEndLoc(), 0, ",")));
}
}
}
>From 7cf39940779044ac4c72aa7cbd7ef0ee8d114197 Mon Sep 17 00:00:00 2001
From: Owen Pan <owenpi...@gmail.com>
Date: Sun, 30 Mar 2025 15:01:35 -0700
Subject: [PATCH 3/3] Change `const char *` to `StringRef`
---
clang/lib/Format/Format.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index a88419ec9db19..b74a8631efe0f 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -2216,7 +2216,7 @@ namespace {
void replaceToken(const FormatToken &Token, FormatToken *Next,
const SourceManager &SourceMgr, tooling::Replacements
&Result,
- const char *Text = "") {
+ StringRef Text = "") {
const auto &Tok = Token.Tok;
SourceLocation Start;
if (Next && Next->NewlinesBefore == 0 && Next->isNot(tok::eof)) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
