Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 11 additions & 0 deletions clang/docs/ClangFormatStyleOptions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -6343,6 +6343,17 @@ the configuration (without a prefix: ``Auto``).
case 1 : break; case 1: break;
} }

.. _SpaceBeforeClosingBlockComment:

**SpaceBeforeClosingBlockComment** (``Boolean``) :versionbadge:`clang-format 21` :ref:`¶ <SpaceBeforeClosingBlockComment>`
If ``true``, a space is inserted immediately before the closing ``*/`` in
block comments that contain content.

.. code-block:: c++

true: false:
/* comment */ vs. /* comment*/

.. _SpaceBeforeCpp11BracedList:

**SpaceBeforeCpp11BracedList** (``Boolean``) :versionbadge:`clang-format 7` :ref:`¶ <SpaceBeforeCpp11BracedList>`
Expand Down
10 changes: 10 additions & 0 deletions clang/include/clang/Format/Format.h
Original file line number Diff line number Diff line change
Expand Up @@ -4684,6 +4684,15 @@ struct FormatStyle {
/// \version 17
bool SpaceBeforeJsonColon;

/// If ``true``, a space is inserted immediately before the closing ``*/`` in
/// block comments that contain content.
/// \code
/// true: false:
/// /* comment */ vs. /* comment*/
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we want options to handle this differently.

foo(/*bar=*/true);

Copy link
Contributor

Choose a reason for hiding this comment

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

Needs to have at least Space, NoSpace, Leave because the default of false is going to now remove spaces everywhere

Copy link
Contributor

Choose a reason for hiding this comment

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

All options normally go through a phase boolean -> enum -> struct I think in this case we should short circuit straight to struct

struct SpaceInComments
{
AfterOpeningComment = Leave
BeforeClosingComment = Leave
AfterOpeningParamComment = Leave
BeforeCloseingParamComment = No // I think this is the current behavior
}

/// \endcode
/// \version 21
bool SpaceBeforeClosingBlockComment;

/// Different ways to put a space before opening parentheses.
enum SpaceBeforeParensStyle : int8_t {
/// This is **deprecated** and replaced by ``Custom`` below, with all
Expand Down Expand Up @@ -5611,6 +5620,7 @@ struct FormatStyle {
SpaceAroundPointerQualifiers == R.SpaceAroundPointerQualifiers &&
SpaceBeforeRangeBasedForLoopColon ==
R.SpaceBeforeRangeBasedForLoopColon &&
SpaceBeforeClosingBlockComment == R.SpaceBeforeClosingBlockComment &&
Copy link
Contributor

Choose a reason for hiding this comment

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

Alphabetic ordering please

SpaceBeforeSquareBrackets == R.SpaceBeforeSquareBrackets &&
SpaceInEmptyBraces == R.SpaceInEmptyBraces &&
SpacesBeforeTrailingComments == R.SpacesBeforeTrailingComments &&
Expand Down
51 changes: 50 additions & 1 deletion clang/lib/Format/BreakableToken.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -776,6 +776,30 @@ void BreakableBlockComment::reflow(unsigned LineIndex,
void BreakableBlockComment::adaptStartOfLine(
unsigned LineIndex, WhitespaceManager &Whitespaces) const {
if (LineIndex == 0) {
if (Style.SpaceBeforeClosingBlockComment) {
const StringRef Content = Tok.TokenText;
if (Content.size() >= 4 && Content.starts_with("/*") &&
Content.ends_with("*/")) {
const StringRef Body = Content.drop_front(2).drop_back(2);
const bool IsEffectivelyEmpty = Body.trim(" \t").empty();
const bool IsSingleLine = !Content.contains('\n');

if (IsEffectivelyEmpty && IsSingleLine) {
const unsigned WhitespaceLength = Body.size();
Whitespaces.replaceWhitespaceInToken(
Tok, /*Offset=*/2, WhitespaceLength, /*PreviousPostfix=*/"",
/*CurrentPrefix=*/"", InPPDirective, /*Newlines=*/1,
/*Spaces=*/0);
} else if (Tok.NeedsSpaceBeforeClosingBlockComment &&
Tok.SpaceBeforeClosingBlockCommentOffset <
Tok.TokenText.size()) {
Whitespaces.replaceWhitespaceInToken(
Tok, Tok.SpaceBeforeClosingBlockCommentOffset,
/*ReplaceChars=*/0, /*PreviousPostfix=*/"", /*CurrentPrefix=*/" ",
InPPDirective, /*Newlines=*/0, /*Spaces=*/0);
}
}
}
if (DelimitersOnNewline) {
// Since we're breaking at index 1 below, the break position and the
// break length are the same.
Expand Down Expand Up @@ -816,9 +840,34 @@ void BreakableBlockComment::adaptStartOfLine(
unsigned WhitespaceLength = Content[LineIndex].data() -
tokenAt(LineIndex).TokenText.data() -
WhitespaceOffsetInToken;
int Spaces = ContentColumn[LineIndex] - Prefix.size();
const bool IsTerminatorOnSeparateLine =
Content[LineIndex].ltrim(Blanks).empty();
const bool IsLastLineOfBlock = (LineIndex + 1 == Lines.size());

if (IsTerminatorOnSeparateLine && IsLastLineOfBlock) {
if (Style.SpaceBeforeClosingBlockComment &&
!Tok.NeedsSpaceBeforeClosingBlockComment) {
if (std::all_of(Content.begin(), Content.begin() + LineIndex,
[](StringRef Line) { return Line.trim().empty(); })) {
// This case handles empty or whitespace-only comments like `/*\n*/`.
// The user wants a space before `*/`, but the lexer correctly
// identifies no space is needed for the token itself. We must avoid
// indenting the `*/` on its new line, which would result in an
// unwanted leading space.
Spaces = 0;
}
} else if (Tok.NeedsSpaceBeforeClosingBlockComment && Spaces > 0) {
// The token text itself will contain the leading space (e.g., " */").
// The calculated `Spaces` is for alignment relative to the star column.
// Decrement to prevent adding a second, redundant space.
--Spaces;
}
}

Whitespaces.replaceWhitespaceInToken(
tokenAt(LineIndex), WhitespaceOffsetInToken, WhitespaceLength, "", Prefix,
InPPDirective, /*Newlines=*/1, ContentColumn[LineIndex] - Prefix.size());
InPPDirective, /*Newlines=*/1, Spaces);
}

BreakableToken::Split
Expand Down
3 changes: 3 additions & 0 deletions clang/lib/Format/Format.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1222,6 +1222,8 @@ template <> struct MappingTraits<FormatStyle> {
IO.mapOptional("SpaceBeforeInheritanceColon",
Style.SpaceBeforeInheritanceColon);
IO.mapOptional("SpaceBeforeJsonColon", Style.SpaceBeforeJsonColon);
IO.mapOptional("SpaceBeforeClosingBlockComment",
Style.SpaceBeforeClosingBlockComment);
IO.mapOptional("SpaceBeforeParens", Style.SpaceBeforeParens);
IO.mapOptional("SpaceBeforeParensOptions", Style.SpaceBeforeParensOptions);
IO.mapOptional("SpaceBeforeRangeBasedForLoopColon",
Expand Down Expand Up @@ -1717,6 +1719,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
LLVMStyle.SpaceBeforeCtorInitializerColon = true;
LLVMStyle.SpaceBeforeInheritanceColon = true;
LLVMStyle.SpaceBeforeJsonColon = false;
LLVMStyle.SpaceBeforeClosingBlockComment = false;
LLVMStyle.SpaceBeforeParens = FormatStyle::SBPO_ControlStatements;
LLVMStyle.SpaceBeforeParensOptions = {};
LLVMStyle.SpaceBeforeParensOptions.AfterControlStatements = true;
Expand Down
14 changes: 11 additions & 3 deletions clang/lib/Format/FormatToken.h
Original file line number Diff line number Diff line change
Expand Up @@ -324,9 +324,9 @@ struct FormatToken {
EndsBinaryExpression(false), PartOfMultiVariableDeclStmt(false),
ContinuesLineCommentSection(false), Finalized(false),
ClosesRequiresClause(false), EndsCppAttributeGroup(false),
BlockKind(BK_Unknown), Decision(FD_Unformatted),
PackingKind(PPK_Inconclusive), TypeIsFinalized(false),
Type(TT_Unknown) {}
NeedsSpaceBeforeClosingBlockComment(false), BlockKind(BK_Unknown),
Decision(FD_Unformatted), PackingKind(PPK_Inconclusive),
TypeIsFinalized(false), Type(TT_Unknown) {}

/// The \c Token.
Token Tok;
Expand Down Expand Up @@ -402,6 +402,9 @@ struct FormatToken {
/// \c true if this token ends a group of C++ attributes.
unsigned EndsCppAttributeGroup : 1;

/// \c true if clang-format should insert a space before the closing '*/'.
unsigned NeedsSpaceBeforeClosingBlockComment : 1;

private:
/// Contains the kind of block if this token is a brace.
unsigned BlockKind : 2;
Expand Down Expand Up @@ -505,6 +508,11 @@ struct FormatToken {
/// token.
unsigned LastLineColumnWidth = 0;

/// Offset (from the start of the token) where a space should be inserted
/// before the closing '*/' when \c NeedsSpaceBeforeClosingBlockComment is
/// set.
unsigned SpaceBeforeClosingBlockCommentOffset = 0;

/// The number of spaces that should be inserted before this token.
unsigned SpacesRequiredBefore = 0;

Expand Down
18 changes: 18 additions & 0 deletions clang/lib/Format/FormatTokenLexer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1386,6 +1386,24 @@ FormatToken *FormatTokenLexer::getNextToken() {
StringRef UntrimmedText = FormatTok->TokenText;
FormatTok->TokenText = FormatTok->TokenText.rtrim(" \t\v\f");
TrailingWhitespace = UntrimmedText.size() - FormatTok->TokenText.size();

bool NeedsSpace = true;
if (!Style.SpaceBeforeClosingBlockComment ||
!FormatTok->TokenText.starts_with("/*") ||
!FormatTok->TokenText.ends_with("*/") ||
FormatTok->TokenText.size() < 4) {
NeedsSpace = false;
}
const StringRef Content =
FormatTok->TokenText.drop_front(2).drop_back(2).rtrim("\r\n");
if (Content.empty())
NeedsSpace = false;
const unsigned char LastChar = static_cast<unsigned char>(Content.back());
if (NeedsSpace && !isHorizontalWhitespace(LastChar)) {
FormatTok->NeedsSpaceBeforeClosingBlockComment = true;
FormatTok->SpaceBeforeClosingBlockCommentOffset =
FormatTok->TokenText.size() - 2;
}
} else if (FormatTok->is(tok::raw_identifier)) {
IdentifierInfo &Info = IdentTable.get(FormatTok->TokenText);
FormatTok->Tok.setIdentifierInfo(&Info);
Expand Down
5 changes: 4 additions & 1 deletion clang/lib/Format/TokenAnnotator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4821,7 +4821,10 @@ bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line,
}
if (Left.is(TT_BlockComment)) {
// No whitespace in x(/*foo=*/1), except for JavaScript.
return Style.isJavaScript() || !Left.TokenText.ends_with("=*/");
bool EndsWithAssignmentComment = Left.TokenText.ends_with("=*/");
if (!EndsWithAssignmentComment && Style.SpaceBeforeClosingBlockComment)
EndsWithAssignmentComment = Left.TokenText.ends_with("= */");
return Style.isJavaScript() || !EndsWithAssignmentComment;
}

// Space between template and attribute.
Expand Down
47 changes: 47 additions & 0 deletions clang/unittests/Format/FormatTestComments.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,53 @@ TEST_F(FormatTestComments, UnderstandsSingleLineComments) {
verifyNoCrash(StringRef("/*\\\0\n/", 6));
}

TEST_F(FormatTestComments, InsertsSpaceBeforeClosingBlockComment) {
FormatStyle Style = getLLVMStyle();
Style.SpaceBeforeClosingBlockComment = true;

verifyFormat("foo(/* comment */);", "foo(/* comment*/);", Style);
verifyFormat("foo(/*Logger= */nullptr);", "foo(/*Logger=*/nullptr);", Style);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think I want to handle this different from comments

verifyFormat("/* comment */", Style);
verifyFormat("/* comment before code */\nint x;",
"/* comment before code*/\nint x;", Style);
verifyFormat("/* comment\n */", "/* comment\n*/", Style);
verifyFormat("/* comment\n */\nint x;", "/* comment\n*/\nint x;", Style);
verifyFormat("/*\ncomment line\n */", "/*\ncomment line\n*/", Style);
verifyFormat("/*\n * comment star\n */", "/*\n * comment star\n*/", Style);
verifyFormat("/* comment line\nnext */", "/* comment line\nnext*/", Style);
verifyFormat("/*\n*/", Style);
verifyFormat("/*\n\n*/", "/*\n \n*/", Style);
verifyFormat("/*This is a multi line comment\n"
"this is the next line\n"
"and this is the 3th line. */",
"/*This is a multi line comment\n"
"this is the next line\n"
"and this is the 3th line.*/",
Style);
verifyFormat(
"/*\n * Another multi line comment\n * this is the next line. */",
"/*\n * Another multi line comment\n * this is the next line.*/", Style);
}

TEST_F(FormatTestComments, BlockCommentDoesNotForceBreakBeforeFollowingToken) {
FormatStyle Style = getLLVMStyle();
Style.SpaceBeforeClosingBlockComment = true;

verifyFormat("switch (n) {\n"
"case 1:\n"
" foo();\n"
"/*FALLTHROUGH */ case 2:\n"
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to handle "/*" while we are at it..

" bar();\n"
"}\n",
"switch (n) {\n"
"case 1:\n"
" foo();\n"
" /*FALLTHROUGH*/ case 2:\n"
" bar();\n"
"}\n",
Style);
}

TEST_F(FormatTestComments, KeepsParameterWithTrailingCommentsOnTheirOwnLine) {
EXPECT_EQ("SomeFunction(a,\n"
" b, // comment\n"
Expand Down
Loading