HazardyKnusperkeks created this revision.
HazardyKnusperkeks added reviewers: MyDeveloperDay, curdeius.
HazardyKnusperkeks added a project: clang-format.
HazardyKnusperkeks requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Before line comments were not touched at all with `ColumnLimit == 0`, so this
is a real change. But at least for me it was unexpected that the comments were
not fixed with `ColumnLimit == 0`.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D96896
Files:
clang/lib/Format/ContinuationIndenter.cpp
clang/unittests/Format/FormatTestComments.cpp
Index: clang/unittests/Format/FormatTestComments.cpp
===================================================================
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -3793,6 +3793,189 @@
"int i;// A Comment to be moved\n"
" // with indent\n",
Style));
+
+ Style = getLLVMStyleWithColumns(0);
+ EXPECT_EQ("// Free comment without space\n"
+ "\n"
+ "// Free comment with 3 spaces\n"
+ "\n"
+ "/// Free Doxygen without space\n"
+ "\n"
+ "/// Free Doxygen with 3 spaces\n"
+ "\n"
+ "/// A Doxygen Comment with a nested list:\n"
+ "/// - Foo\n"
+ "/// - Bar\n"
+ "/// - Baz\n"
+ "/// - End\n"
+ "/// of the inner list\n"
+ "/// .\n"
+ "/// .\n"
+ "\n"
+ "namespace Foo {\n"
+ "bool bar(bool b) {\n"
+ " bool ret1 = true; ///< Doxygenstyle without space\n"
+ " bool ret2 = true; ///< Doxygenstyle with 3 spaces\n"
+ " if (b) {\n"
+ " // Foo\n"
+ "\n"
+ " // In function comment\n"
+ " ret2 = false;\n"
+ " } // End of if\n"
+ "\n"
+ " // if (ret1) {\n"
+ " // return ret2;\n"
+ " // }\n"
+ "\n"
+ " // if (ret1) {\n"
+ " // return ret2;\n"
+ " // }\n"
+ "\n"
+ " return ret1 && ret2;\n"
+ "}\n"
+ "} // namespace Foo\n"
+ "\n"
+ "namespace Bar {\n"
+ "int foo();\n"
+ "} // namespace Bar\n"
+ "//@Nothing added because of the non ascii char\n"
+ "\n"
+ "//@ Nothing removed because of the non ascii char\n"
+ "\n"
+ "// Comment to move to the left\n"
+ "// But not this?\n"
+ "// @but this\n"
+ "\n"
+ "// Comment to move to the right\n"
+ "//@ this stays\n"
+ "\n"
+ "//} will not move\n"
+ "\n"
+ "// vv will only move\n"
+ "// } if the line above does\n",
+ format(Code, Style));
+
+ Style.SpacesInLineCommentPrefix = {0, 0};
+ EXPECT_EQ("//Free comment without space\n"
+ "\n"
+ "//Free comment with 3 spaces\n"
+ "\n"
+ "///Free Doxygen without space\n"
+ "\n"
+ "///Free Doxygen with 3 spaces\n"
+ "\n"
+ "///A Doxygen Comment with a nested list:\n"
+ "///- Foo\n"
+ "///- Bar\n"
+ "/// - Baz\n" // Here we keep the relative indentation
+ "/// - End\n"
+ "/// of the inner list\n"
+ "/// .\n"
+ "///.\n"
+ "\n"
+ "namespace Foo {\n"
+ "bool bar(bool b) {\n"
+ " bool ret1 = true; ///<Doxygenstyle without space\n"
+ " bool ret2 = true; ///<Doxygenstyle with 3 spaces\n"
+ " if (b) {\n"
+ " //Foo\n"
+ "\n"
+ " //In function comment\n"
+ " ret2 = false;\n"
+ " } //End of if\n"
+ "\n"
+ " //if (ret1) {\n"
+ " // return ret2;\n"
+ " //}\n"
+ "\n"
+ " //if (ret1) {\n"
+ " // return ret2;\n"
+ " //}\n"
+ "\n"
+ " return ret1 && ret2;\n"
+ "}\n"
+ "} //namespace Foo\n"
+ "\n"
+ "namespace Bar {\n"
+ "int foo();\n"
+ "} //namespace Bar\n"
+ "//@Nothing added because of the non ascii char\n"
+ "\n"
+ "//@ Nothing removed because of the non ascii char\n"
+ "\n"
+ "//Comment to move to the left\n"
+ "//But not this?\n"
+ "//@but this\n"
+ "\n"
+ "//Comment to move to the right\n"
+ "//@ this stays\n"
+ "\n"
+ "//} will not move\n"
+ "\n"
+ "//vv will only move\n"
+ "//} if the line above does\n",
+ format(Code, Style));
+
+ Style.SpacesInLineCommentPrefix = {2, -1u};
+ EXPECT_EQ("// Free comment without space\n"
+ "\n"
+ "// Free comment with 3 spaces\n"
+ "\n"
+ "/// Free Doxygen without space\n"
+ "\n"
+ "/// Free Doxygen with 3 spaces\n"
+ "\n"
+ "/// A Doxygen Comment with a nested list:\n"
+ "/// - Foo\n"
+ "/// - Bar\n"
+ "/// - Baz\n"
+ "/// - End\n"
+ "/// of the inner list\n"
+ "/// .\n"
+ "/// .\n"
+ "\n"
+ "namespace Foo {\n"
+ "bool bar(bool b) {\n"
+ " bool ret1 = true; ///< Doxygenstyle without space\n"
+ " bool ret2 = true; ///< Doxygenstyle with 3 spaces\n"
+ " if (b) {\n"
+ " // Foo\n"
+ "\n"
+ " // In function comment\n"
+ " ret2 = false;\n"
+ " } // End of if\n"
+ "\n"
+ " // if (ret1) {\n"
+ " // return ret2;\n"
+ " // }\n"
+ "\n"
+ " // if (ret1) {\n"
+ " // return ret2;\n"
+ " // }\n"
+ "\n"
+ " return ret1 && ret2;\n"
+ "}\n"
+ "} // namespace Foo\n"
+ "\n"
+ "namespace Bar {\n"
+ "int foo();\n"
+ "} // namespace Bar\n"
+ "//@Nothing added because of the non ascii char\n"
+ "\n"
+ "//@ Nothing removed because of the non ascii char\n"
+ "\n"
+ "// Comment to move to the left\n"
+ "// But not this?\n"
+ "// @but this\n"
+ "\n"
+ "// Comment to move to the right\n"
+ "//@ this stays\n"
+ "\n"
+ "//} will not move\n"
+ "\n"
+ "// vv will only move\n"
+ "// } if the line above does\n",
+ format(Code, Style));
}
} // end namespace
Index: clang/lib/Format/ContinuationIndenter.cpp
===================================================================
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1993,7 +1993,7 @@
// We don't insert backslashes when breaking line comments.
ColumnLimit = Style.ColumnLimit;
}
- if (Current.UnbreakableTailLength >= ColumnLimit)
+ if (ColumnLimit != 0 && Current.UnbreakableTailLength >= ColumnLimit)
return {0, false};
// ColumnWidth was already accounted into State.Column before calling
// breakProtrudingToken.
@@ -2040,158 +2040,165 @@
// compressed.
bool TryReflow = Reflow;
// Break the current token until we can fit the rest of the line.
- while (ContentStartColumn + RemainingTokenColumns > ColumnLimit) {
- LLVM_DEBUG(llvm::dbgs() << " Over limit, need: "
- << (ContentStartColumn + RemainingTokenColumns)
- << ", space: " << ColumnLimit
- << ", reflown prefix: " << ContentStartColumn
- << ", offset in line: " << TailOffset << "\n");
- // If the current token doesn't fit, find the latest possible split in the
- // current line so that breaking at it will be under the column limit.
- // FIXME: Use the earliest possible split while reflowing to correctly
- // compress whitespace within a line.
- BreakableToken::Split Split =
- Token->getSplit(LineIndex, TailOffset, ColumnLimit,
- ContentStartColumn, CommentPragmasRegex);
- if (Split.first == StringRef::npos) {
- // No break opportunity - update the penalty and continue with the next
- // logical line.
- if (LineIndex < EndIndex - 1)
- // The last line's penalty is handled in addNextStateToQueue() or when
- // calling replaceWhitespaceAfterLastLine below.
- Penalty += Style.PenaltyExcessCharacter *
- (ContentStartColumn + RemainingTokenColumns - ColumnLimit);
- LLVM_DEBUG(llvm::dbgs() << " No break opportunity.\n");
- break;
- }
- assert(Split.first != 0);
-
- if (Token->supportsReflow()) {
- // Check whether the next natural split point after the current one can
- // still fit the line, either because we can compress away whitespace,
- // or because the penalty the excess characters introduce is lower than
- // the break penalty.
- // We only do this for tokens that support reflowing, and thus allow us
- // to change the whitespace arbitrarily (e.g. comments).
- // Other tokens, like string literals, can be broken on arbitrary
- // positions.
-
- // First, compute the columns from TailOffset to the next possible split
- // position.
- // For example:
- // ColumnLimit: |
- // // Some text that breaks
- // ^ tail offset
- // ^-- split
- // ^-------- to split columns
- // ^--- next split
- // ^--------------- to next split columns
- unsigned ToSplitColumns = Token->getRangeLength(
- LineIndex, TailOffset, Split.first, ContentStartColumn);
- LLVM_DEBUG(llvm::dbgs() << " ToSplit: " << ToSplitColumns << "\n");
-
- BreakableToken::Split NextSplit = Token->getSplit(
- LineIndex, TailOffset + Split.first + Split.second, ColumnLimit,
- ContentStartColumn + ToSplitColumns + 1, CommentPragmasRegex);
- // Compute the columns necessary to fit the next non-breakable sequence
- // into the current line.
- unsigned ToNextSplitColumns = 0;
- if (NextSplit.first == StringRef::npos) {
- ToNextSplitColumns = Token->getRemainingLength(LineIndex, TailOffset,
- ContentStartColumn);
- } else {
- ToNextSplitColumns = Token->getRangeLength(
- LineIndex, TailOffset,
- Split.first + Split.second + NextSplit.first, ContentStartColumn);
+ if (ColumnLimit != 0) {
+ while (ContentStartColumn + RemainingTokenColumns > ColumnLimit) {
+ LLVM_DEBUG(llvm::dbgs() << " Over limit, need: "
+ << (ContentStartColumn + RemainingTokenColumns)
+ << ", space: " << ColumnLimit
+ << ", reflown prefix: " << ContentStartColumn
+ << ", offset in line: " << TailOffset << "\n");
+ // If the current token doesn't fit, find the latest possible split in
+ // the current line so that breaking at it will be under the column
+ // limit.
+ // FIXME: Use the earliest possible split while reflowing to correctly
+ // compress whitespace within a line.
+ BreakableToken::Split Split =
+ Token->getSplit(LineIndex, TailOffset, ColumnLimit,
+ ContentStartColumn, CommentPragmasRegex);
+ if (Split.first == StringRef::npos) {
+ // No break opportunity - update the penalty and continue with the
+ // next logical line.
+ if (LineIndex < EndIndex - 1)
+ // The last line's penalty is handled in addNextStateToQueue() or
+ // when calling replaceWhitespaceAfterLastLine below.
+ Penalty +=
+ Style.PenaltyExcessCharacter *
+ (ContentStartColumn + RemainingTokenColumns - ColumnLimit);
+ LLVM_DEBUG(llvm::dbgs() << " No break opportunity.\n");
+ break;
}
- // Compress the whitespace between the break and the start of the next
- // unbreakable sequence.
- ToNextSplitColumns =
- Token->getLengthAfterCompression(ToNextSplitColumns, Split);
- LLVM_DEBUG(llvm::dbgs()
- << " ContentStartColumn: " << ContentStartColumn << "\n");
- LLVM_DEBUG(llvm::dbgs()
- << " ToNextSplit: " << ToNextSplitColumns << "\n");
- // If the whitespace compression makes us fit, continue on the current
- // line.
- bool ContinueOnLine =
- ContentStartColumn + ToNextSplitColumns <= ColumnLimit;
- unsigned ExcessCharactersPenalty = 0;
- if (!ContinueOnLine && !Strict) {
- // Similarly, if the excess characters' penalty is lower than the
- // penalty of introducing a new break, continue on the current line.
- ExcessCharactersPenalty =
- (ContentStartColumn + ToNextSplitColumns - ColumnLimit) *
- Style.PenaltyExcessCharacter;
+ assert(Split.first != 0);
+
+ if (Token->supportsReflow()) {
+ // Check whether the next natural split point after the current one
+ // can still fit the line, either because we can compress away
+ // whitespace, or because the penalty the excess characters introduce
+ // is lower than the break penalty.
+ // We only do this for tokens that support reflowing, and thus allow
+ // us to change the whitespace arbitrarily (e.g. comments). Other
+ // tokens, like string literals, can be broken on arbitrary positions.
+
+ // First, compute the columns from TailOffset to the next possible
+ // split position.
+ // For example:
+ // ColumnLimit: |
+ // // Some text that breaks
+ // ^ tail offset
+ // ^-- split
+ // ^-------- to split columns
+ // ^--- next split
+ // ^--------------- to next split columns
+ unsigned ToSplitColumns = Token->getRangeLength(
+ LineIndex, TailOffset, Split.first, ContentStartColumn);
+ LLVM_DEBUG(llvm::dbgs() << " ToSplit: " << ToSplitColumns << "\n");
+
+ BreakableToken::Split NextSplit = Token->getSplit(
+ LineIndex, TailOffset + Split.first + Split.second, ColumnLimit,
+ ContentStartColumn + ToSplitColumns + 1, CommentPragmasRegex);
+ // Compute the columns necessary to fit the next non-breakable
+ // sequence into the current line.
+ unsigned ToNextSplitColumns = 0;
+ if (NextSplit.first == StringRef::npos) {
+ ToNextSplitColumns = Token->getRemainingLength(
+ LineIndex, TailOffset, ContentStartColumn);
+ } else {
+ ToNextSplitColumns = Token->getRangeLength(
+ LineIndex, TailOffset,
+ Split.first + Split.second + NextSplit.first,
+ ContentStartColumn);
+ }
+ // Compress the whitespace between the break and the start of the next
+ // unbreakable sequence.
+ ToNextSplitColumns =
+ Token->getLengthAfterCompression(ToNextSplitColumns, Split);
+ LLVM_DEBUG(llvm::dbgs() << " ContentStartColumn: "
+ << ContentStartColumn << "\n");
LLVM_DEBUG(llvm::dbgs()
- << " Penalty excess: " << ExcessCharactersPenalty
- << "\n break : " << NewBreakPenalty << "\n");
- if (ExcessCharactersPenalty < NewBreakPenalty) {
- Exceeded = true;
- ContinueOnLine = true;
+ << " ToNextSplit: " << ToNextSplitColumns << "\n");
+ // If the whitespace compression makes us fit, continue on the current
+ // line.
+ bool ContinueOnLine =
+ ContentStartColumn + ToNextSplitColumns <= ColumnLimit;
+ unsigned ExcessCharactersPenalty = 0;
+ if (!ContinueOnLine && !Strict) {
+ // Similarly, if the excess characters' penalty is lower than the
+ // penalty of introducing a new break, continue on the current line.
+ ExcessCharactersPenalty =
+ (ContentStartColumn + ToNextSplitColumns - ColumnLimit) *
+ Style.PenaltyExcessCharacter;
+ LLVM_DEBUG(llvm::dbgs()
+ << " Penalty excess: " << ExcessCharactersPenalty
+ << "\n break : " << NewBreakPenalty << "\n");
+ if (ExcessCharactersPenalty < NewBreakPenalty) {
+ Exceeded = true;
+ ContinueOnLine = true;
+ }
+ }
+ if (ContinueOnLine) {
+ LLVM_DEBUG(llvm::dbgs() << " Continuing on line...\n");
+ // The current line fits after compressing the whitespace - reflow
+ // the next line into it if possible.
+ TryReflow = true;
+ if (!DryRun)
+ Token->compressWhitespace(LineIndex, TailOffset, Split,
+ Whitespaces);
+ // When we continue on the same line, leave one space between
+ // content.
+ ContentStartColumn += ToSplitColumns + 1;
+ Penalty += ExcessCharactersPenalty;
+ TailOffset += Split.first + Split.second;
+ RemainingTokenColumns = Token->getRemainingLength(
+ LineIndex, TailOffset, ContentStartColumn);
+ continue;
}
}
- if (ContinueOnLine) {
- LLVM_DEBUG(llvm::dbgs() << " Continuing on line...\n");
- // The current line fits after compressing the whitespace - reflow
- // the next line into it if possible.
- TryReflow = true;
- if (!DryRun)
- Token->compressWhitespace(LineIndex, TailOffset, Split,
- Whitespaces);
- // When we continue on the same line, leave one space between content.
- ContentStartColumn += ToSplitColumns + 1;
- Penalty += ExcessCharactersPenalty;
- TailOffset += Split.first + Split.second;
- RemainingTokenColumns = Token->getRemainingLength(
- LineIndex, TailOffset, ContentStartColumn);
- continue;
- }
- }
- LLVM_DEBUG(llvm::dbgs() << " Breaking...\n");
- // Update the ContentIndent only if the current line was not reflown with
- // the previous line, since in that case the previous line should still
- // determine the ContentIndent. Also never intent the last line.
- if (!Reflow)
- ContentIndent = Token->getContentIndent(LineIndex);
- LLVM_DEBUG(llvm::dbgs()
- << " ContentIndent: " << ContentIndent << "\n");
- ContentStartColumn = ContentIndent + Token->getContentStartColumn(
- LineIndex, /*Break=*/true);
-
- unsigned NewRemainingTokenColumns = Token->getRemainingLength(
- LineIndex, TailOffset + Split.first + Split.second,
- ContentStartColumn);
- if (NewRemainingTokenColumns == 0) {
- // No content to indent.
- ContentIndent = 0;
- ContentStartColumn =
- Token->getContentStartColumn(LineIndex, /*Break=*/true);
- NewRemainingTokenColumns = Token->getRemainingLength(
+
+ LLVM_DEBUG(llvm::dbgs() << " Breaking...\n");
+ // Update the ContentIndent only if the current line was not reflown
+ // with the previous line, since in that case the previous line should
+ // still determine the ContentIndent. Also never intent the last line.
+ if (!Reflow)
+ ContentIndent = Token->getContentIndent(LineIndex);
+ LLVM_DEBUG(llvm::dbgs()
+ << " ContentIndent: " << ContentIndent << "\n");
+ ContentStartColumn = ContentIndent + Token->getContentStartColumn(
+ LineIndex, /*Break=*/true);
+
+ unsigned NewRemainingTokenColumns = Token->getRemainingLength(
LineIndex, TailOffset + Split.first + Split.second,
ContentStartColumn);
- }
+ if (NewRemainingTokenColumns == 0) {
+ // No content to indent.
+ ContentIndent = 0;
+ ContentStartColumn =
+ Token->getContentStartColumn(LineIndex, /*Break=*/true);
+ NewRemainingTokenColumns = Token->getRemainingLength(
+ LineIndex, TailOffset + Split.first + Split.second,
+ ContentStartColumn);
+ }
- // When breaking before a tab character, it may be moved by a few columns,
- // but will still be expanded to the next tab stop, so we don't save any
- // columns.
- if (NewRemainingTokenColumns == RemainingTokenColumns) {
- // FIXME: Do we need to adjust the penalty?
- break;
+ // When breaking before a tab character, it may be moved by a few
+ // columns, but will still be expanded to the next tab stop, so we don't
+ // save any columns.
+ if (NewRemainingTokenColumns == RemainingTokenColumns) {
+ // FIXME: Do we need to adjust the penalty?
+ break;
+ }
+ assert(NewRemainingTokenColumns < RemainingTokenColumns);
+
+ LLVM_DEBUG(llvm::dbgs()
+ << " Breaking at: " << TailOffset + Split.first << ", "
+ << Split.second << "\n");
+ if (!DryRun)
+ Token->insertBreak(LineIndex, TailOffset, Split, ContentIndent,
+ Whitespaces);
+
+ Penalty += NewBreakPenalty;
+ TailOffset += Split.first + Split.second;
+ RemainingTokenColumns = NewRemainingTokenColumns;
+ BreakInserted = true;
+ NewBreakBefore = true;
}
- assert(NewRemainingTokenColumns < RemainingTokenColumns);
-
- LLVM_DEBUG(llvm::dbgs() << " Breaking at: " << TailOffset + Split.first
- << ", " << Split.second << "\n");
- if (!DryRun)
- Token->insertBreak(LineIndex, TailOffset, Split, ContentIndent,
- Whitespaces);
-
- Penalty += NewBreakPenalty;
- TailOffset += Split.first + Split.second;
- RemainingTokenColumns = NewRemainingTokenColumns;
- BreakInserted = true;
- NewBreakBefore = true;
}
// In case there's another line, prepare the state for the start of the next
// line.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
