Skip to content

[clang-format] Introduce "ReflowComments: IndentOnly" to re-indent comments without breaking internal structure (think Doxygen). #96804

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Oct 12, 2024
Merged

[clang-format] Introduce "ReflowComments: IndentOnly" to re-indent comments without breaking internal structure (think Doxygen). #96804

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
4c6b2fb
[clang-format] Introduce "ReflowComments: IndentOnly" to re-indent co…
ichaer Dec 13, 2023
48ed2cf
Fix docs.
ichaer Jun 27, 2024
1c816f5
Revert bad version number change in docs, break lines in test string …
ichaer Jun 27, 2024
3b3c762
Oops
ichaer Jun 27, 2024
159281a
Oh no, I regenerated the docs before I made the version change :(. So…
ichaer Jul 8, 2024
1d65304
Move `clang-format off` and `clang-format on` comments outside of the…
ichaer Aug 7, 2024
e11767e
Merge remote-tracking branch 'llvm-trunk/main' into clang-format-comm…
ichaer Aug 14, 2024
1b2c790
Addressing @owenca's feedback.
ichaer Aug 14, 2024
f72fe9f
verifyFormat() and verifyFormat.py
ichaer Aug 15, 2024
39ccdc0
Update release notes.
ichaer Oct 11, 2024
f92325a
Merge remote-tracking branch 'llvm-trunk/main' into clang-format-comm…
ichaer Oct 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 37 additions & 13 deletions clang/docs/ClangFormatStyleOptions.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5204,22 +5204,46 @@ the configuration (without a prefix: ``Auto``).

.. _ReflowComments:

**ReflowComments** (``Boolean``) :versionbadge:`clang-format 3.8` :ref:`¶ <ReflowComments>`
If ``true``, clang-format will attempt to re-flow comments. That is it
will touch a comment and *reflow* long comments into new lines, trying to
obey the ``ColumnLimit``.
**ReflowComments** (``ReflowCommentsStyle``) :versionbadge:`clang-format 3.8` :ref:`¶ <ReflowComments>`
Comment reformatting style.

.. code-block:: c++
Possible values:

* ``RCS_Never`` (in configuration: ``Never``)
Leave comments untouched.

.. code-block:: c++

// veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of information
/* second veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of information */
/* third veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of information
* and a misaligned second line */

* ``RCS_IndentOnly`` (in configuration: ``IndentOnly``)
Only apply indentation rules, moving comments left or right, without
changing formatting inside the comments.

.. code-block:: c++

// veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of information
/* second veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of information */
/* third veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of information
* and a misaligned second line */

* ``RCS_Always`` (in configuration: ``Always``)
Apply indentation rules and re-flow long comments into new lines, trying
Copy link
Contributor

@owenca owenca Aug 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to rerun dump_format_style.py after updating Format.h.

Copy link
Contributor Author

@ichaer ichaer Aug 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh man, I remembered it while making the changes and then forgot before committing =(

Have you considered adding that to the build system and perhaps also a CI test? It must be tricky for reviewers, having to remember to check this stuff every time, and the additional review roundtrips... I can propose a separate PR that introduces something like this to clang/docs/CMakeLists.txt:

add_custom_target(clang-format-style-options
    COMMAND "${Python3_EXECUTABLE}" dump_format_style.py
    WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}/tools"
    DEPENDS "${CMAKE_CURRENT_SOURCE_DIR}/../include/clang/Format/Format.h")
add_dependencies(clang-format clang-format-style-options)

And maybe add the clang-format-style-options target to .github/workflows/docs.yml, with a validation that git status --porcelain clang/docs/ClangFormatStyleOptions.rst returns an empty output. If this is something you think could help reduce the burden on reviewers.

Copy link
Contributor

@owenca owenca Aug 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be very helpful. Thank you!

Copy link
Contributor Author

@ichaer ichaer Aug 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought I'd manage some time for opening the new PR before leaving on holiday, but I'm always too optimistic about the number of things I have to wrap up before leaving =(, sorry about this, @owenca. I'm adding it to my calendar to make sure I don't forget, I should be back in late September.

to obey the ``ColumnLimit``.

.. code-block:: c++

// veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of
// information
/* second veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of
* information */
/* third veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of
* information and a misaligned second line */

false:
// veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of information
/* second veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of information */

true:
// veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of
// information
/* second veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of
* information */

.. _RemoveBracesLLVM:

Expand Down
51 changes: 35 additions & 16 deletions clang/include/clang/Format/Format.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3750,24 +3750,43 @@ struct FormatStyle {
ReferenceAlignmentStyle ReferenceAlignment;

// clang-format off
/// If ``true``, clang-format will attempt to re-flow comments. That is it
/// will touch a comment and *reflow* long comments into new lines, trying to
/// obey the ``ColumnLimit``.
/// \code
/// false:
/// // veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of information
/// /* second veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of information */
///
/// true:
/// // veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of
/// // information
/// /* second veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of
/// * information */
/// \endcode
/// \version 3.8
bool ReflowComments;
/// \brief Types of comment re-flow style.
enum ReflowCommentsStyle : int8_t {
/// Leave comments untouched.
/// \code
/// // veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of information
/// /* second veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of information */
/// /* third veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of information
/// * and a misaligned second line */
/// \endcode
RCS_Never,
/// Only apply indentation rules, moving comments left or right, without
/// changing formatting inside the comments.
/// \code
/// // veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of information
/// /* second veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of information */
/// /* third veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of information
/// * and a misaligned second line */
/// \endcode
RCS_IndentOnly,
/// Apply indentation rules and re-flow long comments into new lines, trying
/// to obey the ``ColumnLimit``.
/// \code
/// // veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of
/// // information
/// /* second veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of
/// * information */
/// /* third veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of
/// * information and a misaligned second line */
/// \endcode
RCS_Always
};
// clang-format on

/// \brief Comment reformatting style.
/// \version 3.8
ReflowCommentsStyle ReflowComments;

/// Remove optional braces of control statements (``if``, ``else``, ``for``,
/// and ``while``) in C++ according to the LLVM coding style.
/// \warning
Expand Down
14 changes: 10 additions & 4 deletions clang/lib/Format/BreakableToken.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,8 +420,10 @@ BreakableComment::getSplit(unsigned LineIndex, unsigned TailOffset,
unsigned ColumnLimit, unsigned ContentStartColumn,
const llvm::Regex &CommentPragmasRegex) const {
// Don't break lines matching the comment pragmas regex.
if (CommentPragmasRegex.match(Content[LineIndex]))
if (Style.ReflowComments != FormatStyle::RCS_Always ||
CommentPragmasRegex.match(Content[LineIndex])) {
return Split(StringRef::npos, 0);
}
return getCommentSplit(Content[LineIndex].substr(TailOffset),
ContentStartColumn, ColumnLimit, Style.TabWidth,
Encoding, Style);
Expand Down Expand Up @@ -608,8 +610,10 @@ BreakableToken::Split BreakableBlockComment::getSplit(
unsigned LineIndex, unsigned TailOffset, unsigned ColumnLimit,
unsigned ContentStartColumn, const llvm::Regex &CommentPragmasRegex) const {
// Don't break lines matching the comment pragmas regex.
if (CommentPragmasRegex.match(Content[LineIndex]))
if (Style.ReflowComments != FormatStyle::RCS_Always ||
CommentPragmasRegex.match(Content[LineIndex])) {
return Split(StringRef::npos, 0);
}
return getCommentSplit(Content[LineIndex].substr(TailOffset),
ContentStartColumn, ColumnLimit, Style.TabWidth,
Encoding, Style, Decoration.ends_with("*"));
Expand Down Expand Up @@ -855,7 +859,8 @@ bool BreakableBlockComment::mayReflow(
StringRef IndentContent = Content[LineIndex];
if (Lines[LineIndex].ltrim(Blanks).starts_with("*"))
IndentContent = Lines[LineIndex].ltrim(Blanks).substr(1);
return LineIndex > 0 && !CommentPragmasRegex.match(IndentContent) &&
return LineIndex > 0 && Style.ReflowComments == FormatStyle::RCS_Always &&
!CommentPragmasRegex.match(IndentContent) &&
mayReflowContent(Content[LineIndex]) && !Tok.Finalized &&
!switchesFormatting(tokenAt(LineIndex));
}
Expand Down Expand Up @@ -1160,7 +1165,8 @@ bool BreakableLineCommentSection::mayReflow(
// // text that protrudes
// // into text with different indent
// We do reflow in that case in block comments.
return LineIndex > 0 && !CommentPragmasRegex.match(IndentContent) &&
return LineIndex > 0 && Style.ReflowComments == FormatStyle::RCS_Always &&
!CommentPragmasRegex.match(IndentContent) &&
mayReflowContent(Content[LineIndex]) && !Tok.Finalized &&
!switchesFormatting(tokenAt(LineIndex)) &&
OriginalPrefix[LineIndex] == OriginalPrefix[LineIndex - 1];
Expand Down
4 changes: 2 additions & 2 deletions clang/lib/Format/ContinuationIndenter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2400,7 +2400,7 @@ ContinuationIndenter::createBreakableToken(const FormatToken &Current,
State.Line->InPPDirective, Encoding, Style);
}
} else if (Current.is(TT_BlockComment)) {
if (!Style.ReflowComments ||
if (Style.ReflowComments == FormatStyle::RCS_Never ||
// If a comment token switches formatting, like
// /* clang-format on */, we don't want to break it further,
// but we may still want to adjust its indentation.
Expand All @@ -2421,7 +2421,7 @@ ContinuationIndenter::createBreakableToken(const FormatToken &Current,
}
return true;
}();
if (!Style.ReflowComments ||
if (Style.ReflowComments == FormatStyle::RCS_Never ||
CommentPragmasRegex.match(Current.TokenText.substr(2)) ||
switchesFormatting(Current) || !RegularComments) {
return nullptr;
Expand Down
12 changes: 11 additions & 1 deletion clang/lib/Format/Format.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,16 @@ template <> struct MappingTraits<FormatStyle::RawStringFormat> {
}
};

template <> struct ScalarEnumerationTraits<FormatStyle::ReflowCommentsStyle> {
static void enumeration(IO &IO, FormatStyle::ReflowCommentsStyle &Value) {
IO.enumCase(Value, "Never", FormatStyle::RCS_Never);
IO.enumCase(Value, "IndentOnly", FormatStyle::RCS_IndentOnly);
IO.enumCase(Value, "Always", FormatStyle::RCS_Always);
IO.enumCase(Value, "false", FormatStyle::RCS_Never);
IO.enumCase(Value, "true", FormatStyle::RCS_Always);
}
};

template <>
struct ScalarEnumerationTraits<FormatStyle::ReferenceAlignmentStyle> {
static void enumeration(IO &IO, FormatStyle::ReferenceAlignmentStyle &Value) {
Expand Down Expand Up @@ -1534,7 +1544,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
LLVMStyle.PPIndentWidth = -1;
LLVMStyle.QualifierAlignment = FormatStyle::QAS_Leave;
LLVMStyle.ReferenceAlignment = FormatStyle::RAS_Pointer;
LLVMStyle.ReflowComments = true;
LLVMStyle.ReflowComments = FormatStyle::RCS_Always;
LLVMStyle.RemoveBracesLLVM = false;
LLVMStyle.RemoveParentheses = FormatStyle::RPS_Leave;
LLVMStyle.RemoveSemicolon = false;
Expand Down
18 changes: 9 additions & 9 deletions clang/lib/Format/UnwrappedLineParser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4588,11 +4588,11 @@ bool UnwrappedLineParser::isOnNewLine(const FormatToken &FormatTok) {

// Checks if \p FormatTok is a line comment that continues the line comment
// section on \p Line.
static bool
continuesLineCommentSection(const FormatToken &FormatTok,
const UnwrappedLine &Line,
const llvm::Regex &CommentPragmasRegex) {
if (Line.Tokens.empty())
static bool continuesLineCommentSection(
const FormatToken &FormatTok, const UnwrappedLine &Line,
const FormatStyle::ReflowCommentsStyle ReflowCommentsStyle,
const llvm::Regex &CommentPragmasRegex) {
if (Line.Tokens.empty() || ReflowCommentsStyle != FormatStyle::RCS_Always)
return false;

StringRef IndentContent = FormatTok.TokenText;
Expand Down Expand Up @@ -4704,8 +4704,8 @@ void UnwrappedLineParser::flushComments(bool NewlineBeforeNext) {
//
// FIXME: Consider putting separate line comment sections as children to the
// unwrapped line instead.
Tok->ContinuesLineCommentSection =
continuesLineCommentSection(*Tok, *Line, CommentPragmasRegex);
Tok->ContinuesLineCommentSection = continuesLineCommentSection(
*Tok, *Line, Style.ReflowComments, CommentPragmasRegex);
if (isOnNewLine(*Tok) && JustComments && !Tok->ContinuesLineCommentSection)
addUnwrappedLine();
pushToken(Tok);
Expand Down Expand Up @@ -4778,8 +4778,8 @@ void UnwrappedLineParser::distributeComments(
if (HasTrailAlignedWithNextToken && i == StartOfTrailAlignedWithNextToken) {
FormatTok->ContinuesLineCommentSection = false;
} else {
FormatTok->ContinuesLineCommentSection =
continuesLineCommentSection(*FormatTok, *Line, CommentPragmasRegex);
FormatTok->ContinuesLineCommentSection = continuesLineCommentSection(
*FormatTok, *Line, Style.ReflowComments, CommentPragmasRegex);
}
if (!FormatTok->ContinuesLineCommentSection &&
(isOnNewLine(*FormatTok) || FormatTok->IsFirst)) {
Expand Down
11 changes: 10 additions & 1 deletion clang/unittests/Format/ConfigParseTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,6 @@ TEST(ConfigParseTest, ParsesConfigurationBools) {
CHECK_PARSE_BOOL(ObjCSpaceAfterProperty);
CHECK_PARSE_BOOL(ObjCSpaceBeforeProtocolList);
CHECK_PARSE_BOOL(Cpp11BracedListStyle);
CHECK_PARSE_BOOL(ReflowComments);
CHECK_PARSE_BOOL(RemoveBracesLLVM);
CHECK_PARSE_BOOL(RemoveSemicolon);
CHECK_PARSE_BOOL(SkipMacroDefinitionBody);
Expand Down Expand Up @@ -372,6 +371,16 @@ TEST(ConfigParseTest, ParsesConfiguration) {
CHECK_PARSE("PointerBindsToType: Middle", PointerAlignment,
FormatStyle::PAS_Middle);

Style.ReflowComments = FormatStyle::RCS_Always;
CHECK_PARSE("ReflowComments: Never", ReflowComments, FormatStyle::RCS_Never);
CHECK_PARSE("ReflowComments: IndentOnly", ReflowComments,
FormatStyle::RCS_IndentOnly);
CHECK_PARSE("ReflowComments: Always", ReflowComments,
FormatStyle::RCS_Always);
// For backward compatibility:
CHECK_PARSE("ReflowComments: false", ReflowComments, FormatStyle::RCS_Never);
CHECK_PARSE("ReflowComments: true", ReflowComments, FormatStyle::RCS_Always);

Style.Standard = FormatStyle::LS_Auto;
CHECK_PARSE("Standard: c++03", Standard, FormatStyle::LS_Cpp03);
CHECK_PARSE("Standard: c++11", Standard, FormatStyle::LS_Cpp11);
Expand Down
6 changes: 3 additions & 3 deletions clang/unittests/Format/FormatTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17851,7 +17851,7 @@ TEST_F(FormatTest, AlignConsecutiveMacros) {

// Test across comments
Style.MaxEmptyLinesToKeep = 10;
Style.ReflowComments = false;
Style.ReflowComments = FormatStyle::RCS_Never;
Style.AlignConsecutiveMacros.AcrossComments = true;
verifyFormat("#define a 3\n"
"// line comment\n"
Expand Down Expand Up @@ -18598,7 +18598,7 @@ TEST_F(FormatTest, AlignConsecutiveAssignmentsAcrossEmptyLinesAndComments) {
"y = 1;",
Alignment);

Alignment.ReflowComments = true;
Alignment.ReflowComments = FormatStyle::RCS_Always;
Alignment.ColumnLimit = 50;
verifyFormat("int x = 0;\n"
"int yy = 1; /// specificlennospace\n"
Expand Down Expand Up @@ -18996,7 +18996,7 @@ TEST_F(FormatTest, AlignConsecutiveAssignments) {
"y = 1;",
Alignment);

EXPECT_EQ(Alignment.ReflowComments, true);
EXPECT_EQ(Alignment.ReflowComments, FormatStyle::RCS_Always);
Alignment.ColumnLimit = 50;
verifyFormat("int x = 0;\n"
"int yy = 1; /// specificlennospace\n"
Expand Down
33 changes: 30 additions & 3 deletions clang/unittests/Format/FormatTestComments.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -493,9 +493,36 @@ TEST_F(FormatTestComments, AlignsBlockComments) {

TEST_F(FormatTestComments, CommentReflowingCanBeTurnedOff) {
FormatStyle Style = getLLVMStyleWithColumns(20);
Style.ReflowComments = false;
verifyFormat("// aaaaaaaaa aaaaaaaaaa aaaaaaaaaa", Style);
verifyFormat("/* aaaaaaaaa aaaaaaaaaa aaaaaaaaaa */", Style);
Style.ReflowComments = FormatStyle::RCS_Never;
verifyNoChange("// aaaaaaaaa aaaaaaaaaa aaaaaaaaaa", Style);
verifyNoChange("/* aaaaaaaaa aaaaaaaaaa aaaaaaaaaa */", Style);
verifyNoChange("/* aaaaaaaaa aaaaaaaaaa aaaaaaaaaa\n"
"aaaaaaaaa*/",
Style);
verifyNoChange("/* aaaaaaaaa aaaaaaaaaa aaaaaaaaaa\n"
" aaaaaaaaa*/",
Style);
verifyNoChange("/* aaaaaaaaa aaaaaaaaaa aaaaaaaaaa\n"
" * aaaaaaaaa*/",
Style);
}

TEST_F(FormatTestComments, CommentReflowingCanApplyOnlyToIndents) {
FormatStyle Style = getLLVMStyleWithColumns(20);
Style.ReflowComments = FormatStyle::RCS_IndentOnly;
verifyNoChange("// aaaaaaaaa aaaaaaaaaa aaaaaaaaaa", Style);
verifyNoChange("/* aaaaaaaaa aaaaaaaaaa aaaaaaaaaa */", Style);
verifyNoChange("/* aaaaaaaaa aaaaaaaaaa aaaaaaaaaa\n"
"aaaaaaaaa*/",
Style);
verifyNoChange("/* aaaaaaaaa aaaaaaaaaa aaaaaaaaaa\n"
" aaaaaaaaa*/",
Style);
verifyFormat("/* aaaaaaaaa aaaaaaaaaa aaaaaaaaaa\n"
" * aaaaaaaaa*/",
"/* aaaaaaaaa aaaaaaaaaa aaaaaaaaaa\n"
" * aaaaaaaaa*/",
Style);
}

TEST_F(FormatTestComments, CorrectlyHandlesLengthOfBlockComments) {
Expand Down