[PATCH] Fix for corner cases in code handling leading "* " decorations in block comments
Alexander Kornienko
alexfh at google.com
Mon Jul 8 06:56:06 PDT 2013
Hi klimek,
Fixes problems that lead to incorrect formatting of these and similar snippets:
/*
**
*/
/*
**/
/*
* */
/*
*test
*/
Clang-format used to think that all the cases above use "* " decoration, and
failed to calculate insertion position properly. It also used to remove leading
"* " in the last line.
http://llvm-reviews.chandlerc.com/D1113
Files:
lib/Format/BreakableToken.cpp
lib/Format/BreakableToken.h
unittests/Format/FormatTest.cpp
Index: lib/Format/BreakableToken.cpp
===================================================================
--- lib/Format/BreakableToken.cpp
+++ lib/Format/BreakableToken.cpp
@@ -225,50 +225,56 @@
TokenText.substr(2, TokenText.size() - 4).split(Lines, "\n");
int IndentDelta = StartColumn - OriginalStartColumn;
- bool NeedsStar = true;
LeadingWhitespace.resize(Lines.size());
StartOfLineColumn.resize(Lines.size());
+ StartOfLineColumn[0] = StartColumn + 2;
+ for (size_t i = 1; i < Lines.size(); ++i)
+ adjustWhitespace(Style, i, IndentDelta);
+
+ Decoration = "* ";
if (Lines.size() == 1 && !FirstInLine) {
// Comments for which FirstInLine is false can start on arbitrary column,
// and available horizontal space can be too small to align consecutive
// lines with the first one.
// FIXME: We could, probably, align them to current indentation level, but
// now we just wrap them without stars.
- NeedsStar = false;
+ Decoration = "";
}
- StartOfLineColumn[0] = StartColumn + 2;
- for (size_t i = 1; i < Lines.size(); ++i) {
- adjustWhitespace(Style, i, IndentDelta);
- if (Lines[i].empty())
- // If the last line is empty, the closing "*/" will have a star.
- NeedsStar = NeedsStar && i + 1 == Lines.size();
- else
- NeedsStar = NeedsStar && Lines[i][0] == '*';
+ for (size_t i = 1, e = Lines.size(); i < e && !Decoration.empty(); ++i) {
+ // If the last line is empty, the closing "*/" will have a star.
+ if (i == e - 1 && Lines[i].empty())
+ continue;
+ while (!Lines[i].startswith(Decoration))
+ Decoration = Decoration.substr(0, Decoration.size() - 1);
}
- Decoration = NeedsStar ? "* " : "";
+
+ LastLineNeedsDecoration = true;
IndentAtLineBreak = StartOfLineColumn[0] + 1;
for (size_t i = 1; i < Lines.size(); ++i) {
if (Lines[i].empty()) {
- if (!NeedsStar && i + 1 != Lines.size())
- // For all but the last line (which always ends in */), set the
- // start column to 0 if they're empty, so we do not insert
- // trailing whitespace anywhere.
+ if (i + 1 == Lines.size()) {
+ // Empty last line means that we already have a star as a part of the
+ // trailing */. We also need to preserve whitespace, so that */ is
+ // correctly indented.
+ LastLineNeedsDecoration = false;
+ } else if (Decoration.empty()) {
+ // For all other lines, set the start column to 0 if they're empty, so
+ // we do not insert trailing whitespace anywhere.
StartOfLineColumn[i] = 0;
+ }
continue;
}
- if (NeedsStar) {
- // The first line already excludes the star.
- // For all other lines, adjust the line to exclude the star and
- // (optionally) the first whitespace.
- int Offset = Lines[i].startswith("* ") ? 2 : 1;
- StartOfLineColumn[i] += Offset;
- Lines[i] = Lines[i].substr(Offset);
- LeadingWhitespace[i] += Offset;
- }
+ // The first line already excludes the star.
+ // For all other lines, adjust the line to exclude the star and
+ // (optionally) the first whitespace.
+ StartOfLineColumn[i] += Decoration.size();
+ Lines[i] = Lines[i].substr(Decoration.size());
+ LeadingWhitespace[i] += Decoration.size();
IndentAtLineBreak = std::min<int>(IndentAtLineBreak, StartOfLineColumn[i]);
}
IndentAtLineBreak = std::max<unsigned>(IndentAtLineBreak, Decoration.size());
DEBUG({
+ llvm::dbgs() << "IndentAtLineBreak " << IndentAtLineBreak << "\n";
for (size_t i = 0; i < Lines.size(); ++i) {
llvm::dbgs() << i << " |" << Lines[i] << "| " << LeadingWhitespace[i]
<< "\n";
@@ -365,17 +371,19 @@
StringRef Prefix = Decoration;
if (Lines[LineIndex].empty()) {
if (LineIndex + 1 == Lines.size()) {
- // If the last line is empty, we don't need a prefix, as the */ will line
- // up with the decoration (if it exists).
- Prefix = "";
+ if (!LastLineNeedsDecoration) {
+ // If the last line is empty, we don't need a prefix, as the */ will
+ // line up with the decoration (if it exists).
+ Prefix = "";
+ }
} else if (!Decoration.empty()) {
// For other empty lines, if we do have a decoration, adapt it to not
// contain a trailing whitespace.
Prefix = Prefix.substr(0, 1);
}
} else {
if (StartOfLineColumn[LineIndex] == 1) {
- // This lines starts immediately after the decorating *.
+ // This line starts immediately after the decorating *.
Prefix = Prefix.substr(0, 1);
}
}
Index: lib/Format/BreakableToken.h
===================================================================
--- lib/Format/BreakableToken.h
+++ lib/Format/BreakableToken.h
@@ -208,6 +208,10 @@
// instead.
unsigned IndentAtLineBreak;
+ // This is to distinguish between the case when the last line was empty and
+ // the case when it started with a decoration ("*" or "* ").
+ bool LastLineNeedsDecoration;
+
// Either "* " if all lines begin with a "*", or empty.
StringRef Decoration;
};
Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -3967,7 +3967,7 @@
EXPECT_EQ("/*\n"
"*\n"
" * aaaaaa\n"
- "* aaaaaa\n"
+ "*aaaaaa\n"
"*/",
format("/*\n"
"*\n"
@@ -3977,7 +3977,7 @@
EXPECT_EQ("/*\n"
"**\n"
"* aaaaaa\n"
- "* aaaaaa\n"
+ "*aaaaaa\n"
"*/",
format("/*\n"
"**\n"
@@ -4017,6 +4017,33 @@
"int cccccccccccccccccccccccccccccc; /* comment */\n"));
verifyFormat("void f(int * /* unused */) {}");
+
+ EXPECT_EQ("/*\n"
+ " **\n"
+ " */",
+ format("/*\n"
+ " **\n"
+ " */"));
+ EXPECT_EQ("/*\n"
+ " *q\n"
+ " */",
+ format("/*\n"
+ " *q\n"
+ " */"));
+ EXPECT_EQ("/*\n"
+ " * q\n"
+ " */",
+ format("/*\n"
+ " * q\n"
+ " */"));
+ EXPECT_EQ("/*\n"
+ " **/",
+ format("/*\n"
+ " **/"));
+ EXPECT_EQ("/*\n"
+ " ***/",
+ format("/*\n"
+ " ***/"));
}
TEST_F(FormatTest, BlockCommentsInMacros) {
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D1113.1.patch
Type: text/x-patch
Size: 6566 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130708/294c6bfd/attachment.bin>
More information about the cfe-commits
mailing list