[clang] Add support for aligning BlockComments in declarations (PR #109497)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 20 16:45:42 PDT 2024
https://github.com/JessehMSFT created https://github.com/llvm/llvm-project/pull/109497
There are two primary scenarios where our team uses block comments in function declarations.
1. To comment out unused parameters
2. To provide additional context when passing an unnamed parameter
Clang-format alignment currently breaks when it encounters a block comment.
This PR adds a new AlignBlockComments option which will allow these block comments to be aligned.
>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