[clang] Add support for aligning BlockComments in declarations (PR #109497)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 20 16:47:35 PDT 2024
https://github.com/JessehMSFT updated https://github.com/llvm/llvm-project/pull/109497
>From 38333491868dfad9c84719d9dd8fd872a2aa7584 Mon Sep 17 00:00:00 2001
From: Jesse Harvey <jesseh at microsoft.com>
Date: Fri, 20 Sep 2024 16:40:35 -0700
Subject: [PATCH] Add support for aligning BlockComments in declarations
---
clang/include/clang/Format/Format.h | 25 +++++++-
clang/lib/Format/Format.cpp | 22 ++++---
clang/lib/Format/WhitespaceManager.cpp | 11 +++-
clang/unittests/Format/ConfigParseTest.cpp | 73 +++++++++++-----------
clang/unittests/Format/FormatTest.cpp | 49 ++++++++++++++-
5 files changed, 134 insertions(+), 46 deletions(-)
diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
index d8b62c7652a0f6..f7b6bfffd61314 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -260,12 +260,35 @@ struct FormatStyle {
/// bbb >>= 2;
/// \endcode
bool PadOperators;
+ /// Only for ``AlignConsecutiveDeclarations``. Whether block comments
+ /// are aligned in declarations.
+ /// \code
+ /// true:
+ /// someLongFunction(int /*a*/,
+ /// bool b,
+ /// const std::string& c)
+ ///
+ /// const bool ret = someLongFunction(4 /*a*/,
+ /// true /*b*/,
+ /// str /*c*/);
+ ///
+ /// false:
+ /// someLongFunction(int /*a*/,
+ /// bool b,
+ /// const std::string& c)
+ ///
+ /// const bool ret = someLongFunction(4 /*a*/,
+ /// true /*b*/,
+ /// str /*c*/);
+ /// \endcode
+ bool AlignBlockComments;
bool operator==(const AlignConsecutiveStyle &R) const {
return Enabled == R.Enabled && AcrossEmptyLines == R.AcrossEmptyLines &&
AcrossComments == R.AcrossComments &&
AlignCompound == R.AlignCompound &&
AlignFunctionPointers == R.AlignFunctionPointers &&
- PadOperators == R.PadOperators;
+ PadOperators == R.PadOperators &&
+ AlignBlockComments == R.AlignBlockComments;
}
bool operator!=(const AlignConsecutiveStyle &R) const {
return !(*this == R);
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index d2463b892fbb96..ab8eadd0171aa6 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -48,39 +48,46 @@ template <> struct MappingTraits<FormatStyle::AlignConsecutiveStyle> {
FormatStyle::AlignConsecutiveStyle(
{/*Enabled=*/false, /*AcrossEmptyLines=*/false,
/*AcrossComments=*/false, /*AlignCompound=*/false,
- /*AlignFunctionPointers=*/false, /*PadOperators=*/true}));
+ /*AlignFunctionPointers=*/false, /*PadOperators=*/true,
+ /*AlignBlockComments=*/false}));
IO.enumCase(Value, "Consecutive",
FormatStyle::AlignConsecutiveStyle(
{/*Enabled=*/true, /*AcrossEmptyLines=*/false,
/*AcrossComments=*/false, /*AlignCompound=*/false,
- /*AlignFunctionPointers=*/false, /*PadOperators=*/true}));
+ /*AlignFunctionPointers=*/false, /*PadOperators=*/true,
+ /*AlignBlockComments=*/false}));
IO.enumCase(Value, "AcrossEmptyLines",
FormatStyle::AlignConsecutiveStyle(
{/*Enabled=*/true, /*AcrossEmptyLines=*/true,
/*AcrossComments=*/false, /*AlignCompound=*/false,
- /*AlignFunctionPointers=*/false, /*PadOperators=*/true}));
+ /*AlignFunctionPointers=*/false, /*PadOperators=*/true,
+ /*AlignBlockComments=*/false}));
IO.enumCase(Value, "AcrossComments",
FormatStyle::AlignConsecutiveStyle(
{/*Enabled=*/true, /*AcrossEmptyLines=*/false,
/*AcrossComments=*/true, /*AlignCompound=*/false,
- /*AlignFunctionPointers=*/false, /*PadOperators=*/true}));
+ /*AlignFunctionPointers=*/false, /*PadOperators=*/true,
+ /*AlignBlockComments=*/false}));
IO.enumCase(Value, "AcrossEmptyLinesAndComments",
FormatStyle::AlignConsecutiveStyle(
{/*Enabled=*/true, /*AcrossEmptyLines=*/true,
/*AcrossComments=*/true, /*AlignCompound=*/false,
- /*AlignFunctionPointers=*/false, /*PadOperators=*/true}));
+ /*AlignFunctionPointers=*/false, /*PadOperators=*/true,
+ /*AlignBlockComments=*/false}));
// For backward compatibility.
IO.enumCase(Value, "true",
FormatStyle::AlignConsecutiveStyle(
{/*Enabled=*/true, /*AcrossEmptyLines=*/false,
/*AcrossComments=*/false, /*AlignCompound=*/false,
- /*AlignFunctionPointers=*/false, /*PadOperators=*/true}));
+ /*AlignFunctionPointers=*/false, /*PadOperators=*/true,
+ /*AlignBlockComments=*/false}));
IO.enumCase(Value, "false",
FormatStyle::AlignConsecutiveStyle(
{/*Enabled=*/false, /*AcrossEmptyLines=*/false,
/*AcrossComments=*/false, /*AlignCompound=*/false,
- /*AlignFunctionPointers=*/false, /*PadOperators=*/true}));
+ /*AlignFunctionPointers=*/false, /*PadOperators=*/true,
+ /*AlignBlockComments=*/false}));
}
static void mapping(IO &IO, FormatStyle::AlignConsecutiveStyle &Value) {
@@ -90,6 +97,7 @@ template <> struct MappingTraits<FormatStyle::AlignConsecutiveStyle> {
IO.mapOptional("AlignCompound", Value.AlignCompound);
IO.mapOptional("AlignFunctionPointers", Value.AlignFunctionPointers);
IO.mapOptional("PadOperators", Value.PadOperators);
+ IO.mapOptional("AlignBlockComments", Value.AlignBlockComments);
}
};
diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp
index fd4a40a86082e2..62d1cfdcff3c27 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -1005,6 +1005,12 @@ void WhitespaceManager::alignConsecutiveTableGenDefinitions() {
TT_InheritanceColon);
}
+static bool shouldAlignBlockComment(const FormatToken &Tok) {
+ return Tok.is(TT_BlockComment) && Tok.Previous &&
+ !Tok.Previous->isOneOf(TT_StartOfName, TT_BlockComment) && Tok.Next &&
+ Tok.Next->isOneOf(tok::comma, tok::r_paren, TT_BlockComment);
+}
+
void WhitespaceManager::alignConsecutiveDeclarations() {
if (!Style.AlignConsecutiveDeclarations.Enabled)
return;
@@ -1022,7 +1028,10 @@ void WhitespaceManager::alignConsecutiveDeclarations() {
if (C.Tok->is(TT_FunctionDeclarationName))
return true;
if (C.Tok->isNot(TT_StartOfName))
- return false;
+ if (!Style.AlignConsecutiveDeclarations.AlignBlockComments ||
+ !shouldAlignBlockComment(*C.Tok)) {
+ return false;
+ }
if (C.Tok->Previous &&
C.Tok->Previous->is(TT_StatementAttributeLikeMacro))
return false;
diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp
index b8bdfaaa74e10e..745d187dddce50 100644
--- a/clang/unittests/Format/ConfigParseTest.cpp
+++ b/clang/unittests/Format/ConfigParseTest.cpp
@@ -300,49 +300,50 @@ TEST(ConfigParseTest, ParsesConfiguration) {
#define CHECK_ALIGN_CONSECUTIVE(FIELD) \
do { \
Style.FIELD.Enabled = true; \
- CHECK_PARSE( \
- #FIELD ": None", FIELD, \
- FormatStyle::AlignConsecutiveStyle( \
- {/*Enabled=*/false, /*AcrossEmptyLines=*/false, \
- /*AcrossComments=*/false, /*AlignCompound=*/false, \
- /*AlignFunctionPointers=*/false, /*PadOperators=*/true})); \
- CHECK_PARSE( \
- #FIELD ": Consecutive", FIELD, \
- FormatStyle::AlignConsecutiveStyle( \
- {/*Enabled=*/true, /*AcrossEmptyLines=*/false, \
- /*AcrossComments=*/false, /*AlignCompound=*/false, \
- /*AlignFunctionPointers=*/false, /*PadOperators=*/true})); \
- CHECK_PARSE( \
- #FIELD ": AcrossEmptyLines", FIELD, \
- FormatStyle::AlignConsecutiveStyle( \
- {/*Enabled=*/true, /*AcrossEmptyLines=*/true, \
- /*AcrossComments=*/false, /*AlignCompound=*/false, \
- /*AlignFunctionPointers=*/false, /*PadOperators=*/true})); \
- CHECK_PARSE( \
- #FIELD ": AcrossEmptyLinesAndComments", FIELD, \
- FormatStyle::AlignConsecutiveStyle( \
- {/*Enabled=*/true, /*AcrossEmptyLines=*/true, \
- /*AcrossComments=*/true, /*AlignCompound=*/false, \
- /*AlignFunctionPointers=*/false, /*PadOperators=*/true})); \
+ CHECK_PARSE(#FIELD ": None", FIELD, \
+ FormatStyle::AlignConsecutiveStyle( \
+ {/*Enabled=*/false, /*AcrossEmptyLines=*/false, \
+ /*AcrossComments=*/false, /*AlignCompound=*/false, \
+ /*AlignFunctionPointers=*/false, /*PadOperators=*/true, \
+ /*AlignBlockComments=*/false})); \
+ CHECK_PARSE(#FIELD ": Consecutive", FIELD, \
+ FormatStyle::AlignConsecutiveStyle( \
+ {/*Enabled=*/true, /*AcrossEmptyLines=*/false, \
+ /*AcrossComments=*/false, /*AlignCompound=*/false, \
+ /*AlignFunctionPointers=*/false, /*PadOperators=*/true, \
+ /*AlignBlockComments=*/false})); \
+ CHECK_PARSE(#FIELD ": AcrossEmptyLines", FIELD, \
+ FormatStyle::AlignConsecutiveStyle( \
+ {/*Enabled=*/true, /*AcrossEmptyLines=*/true, \
+ /*AcrossComments=*/false, /*AlignCompound=*/false, \
+ /*AlignFunctionPointers=*/false, /*PadOperators=*/true, \
+ /*AlignBlockComments=*/false})); \
+ CHECK_PARSE(#FIELD ": AcrossEmptyLinesAndComments", FIELD, \
+ FormatStyle::AlignConsecutiveStyle( \
+ {/*Enabled=*/true, /*AcrossEmptyLines=*/true, \
+ /*AcrossComments=*/true, /*AlignCompound=*/false, \
+ /*AlignFunctionPointers=*/false, /*PadOperators=*/true, \
+ /*AlignBlockComments=*/false})); \
/* For backwards compability, false / true should still parse */ \
- CHECK_PARSE( \
- #FIELD ": false", FIELD, \
- FormatStyle::AlignConsecutiveStyle( \
- {/*Enabled=*/false, /*AcrossEmptyLines=*/false, \
- /*AcrossComments=*/false, /*AlignCompound=*/false, \
- /*AlignFunctionPointers=*/false, /*PadOperators=*/true})); \
- CHECK_PARSE( \
- #FIELD ": true", FIELD, \
- FormatStyle::AlignConsecutiveStyle( \
- {/*Enabled=*/true, /*AcrossEmptyLines=*/false, \
- /*AcrossComments=*/false, /*AlignCompound=*/false, \
- /*AlignFunctionPointers=*/false, /*PadOperators=*/true})); \
+ CHECK_PARSE(#FIELD ": false", FIELD, \
+ FormatStyle::AlignConsecutiveStyle( \
+ {/*Enabled=*/false, /*AcrossEmptyLines=*/false, \
+ /*AcrossComments=*/false, /*AlignCompound=*/false, \
+ /*AlignFunctionPointers=*/false, /*PadOperators=*/true, \
+ /*AlignBlockComments=*/false})); \
+ CHECK_PARSE(#FIELD ": true", FIELD, \
+ FormatStyle::AlignConsecutiveStyle( \
+ {/*Enabled=*/true, /*AcrossEmptyLines=*/false, \
+ /*AcrossComments=*/false, /*AlignCompound=*/false, \
+ /*AlignFunctionPointers=*/false, /*PadOperators=*/true, \
+ /*AlignBlockComments=*/false})); \
\
CHECK_PARSE_NESTED_BOOL(FIELD, Enabled); \
CHECK_PARSE_NESTED_BOOL(FIELD, AcrossEmptyLines); \
CHECK_PARSE_NESTED_BOOL(FIELD, AcrossComments); \
CHECK_PARSE_NESTED_BOOL(FIELD, AlignCompound); \
CHECK_PARSE_NESTED_BOOL(FIELD, PadOperators); \
+ CHECK_PARSE_NESTED_BOOL(FIELD, AlignBlockComments); \
} while (false)
CHECK_ALIGN_CONSECUTIVE(AlignConsecutiveAssignments);
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 53aa93a7a4fb01..2f03149609878a 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -20018,6 +20018,52 @@ TEST_F(FormatTest, AlignConsecutiveDeclarations) {
BracedAlign);
}
+TEST_F(FormatTest, AlignConsecutiveDeclarationsBlockComments) {
+ FormatStyle Style = getLLVMStyleWithColumns(80);
+ Style.AlignConsecutiveDeclarations.Enabled = true;
+ Style.AlignConsecutiveDeclarations.AlignBlockComments = true;
+ Style.BinPackParameters = FormatStyle::BPPS_OnePerLine;
+ Style.BinPackArguments = false;
+
+ verifyFormat(
+ "bool SomeLongMethodName(int longParameterNameA,\n"
+ " bool /*longParameterNameB*/,\n"
+ " const std::string &longParameterNameC);",
+ "bool SomeLongMethodName(int longParameterNameA,\n"
+ " bool /*longParameterNameB*/,\n"
+ " const std::string &longParameterNameC);",
+ Style);
+
+ verifyFormat(
+ "const bool ret = SomeLongMethodName(4 /*parameterNameA*/,\n"
+ " true /*longParameterNameB*/,\n"
+ " str /*longestParameterNameC*/);",
+ "const bool ret = SomeLongMethodName(4 /*parameterNameA*/,\n"
+ " true /*longParameterNameB*/,\n"
+ " str /*longestParameterNameC*/);",
+ Style);
+
+ verifyNoChange("/*,\n"
+ "This is a multi-line block comment.\n"
+ "That is not part of a function declaration.\n"
+ "*/\n"
+ "static void SomeLongMethodName()",
+ Style);
+
+ verifyNoChange("static const unsigned int c_cMediaEntranceEntries = 31;\n"
+ "static const unsigned int c_cMediaEmphasisEntries = 4 /* "
+ "media effects */ + 12;\n"
+ "static const unsigned int c_cMediaExitEntries = 32;",
+ Style);
+
+ verifyNoChange(
+ "static bool SomeLongMethodName(int longParameterNameA,\n"
+ " bool longParameterNameB "
+ "/*=false*/,\n"
+ " std::string &longParameterNameC);",
+ Style);
+}
+
TEST_F(FormatTest, AlignConsecutiveShortCaseStatements) {
FormatStyle Alignment = getLLVMStyle();
Alignment.AllowShortCaseLabelsOnASingleLine = true;
@@ -20259,7 +20305,8 @@ TEST_F(FormatTest, AlignWithLineBreaks) {
FormatStyle::AlignConsecutiveStyle(
{/*Enabled=*/false, /*AcrossEmptyLines=*/false,
/*AcrossComments=*/false, /*AlignCompound=*/false,
- /*AlignFunctionPointers=*/false, /*PadOperators=*/true}));
+ /*AlignFunctionPointers=*/false, /*PadOperators=*/true,
+ /*AlignBlockComments=*/false}));
EXPECT_EQ(Style.AlignConsecutiveDeclarations,
FormatStyle::AlignConsecutiveStyle({}));
verifyFormat("void foo() {\n"
More information about the cfe-commits
mailing list