r185818 - Fix for corner cases in code handling leading "* " decorations in block comments
Alexander Kornienko
alexfh at google.com
Mon Jul 8 07:12:07 PDT 2013
Author: alexfh
Date: Mon Jul 8 09:12:07 2013
New Revision: 185818
URL: http://llvm.org/viewvc/llvm-project?rev=185818&view=rev
Log:
Fix for corner cases in code handling leading "* " decorations in block comments
Summary:
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.
Reviewers: klimek
Reviewed By: klimek
CC: cfe-commits
Differential Revision: http://llvm-reviews.chandlerc.com/D1113
Modified:
cfe/trunk/lib/Format/BreakableToken.cpp
cfe/trunk/lib/Format/BreakableToken.h
cfe/trunk/unittests/Format/FormatTest.cpp
Modified: cfe/trunk/lib/Format/BreakableToken.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/BreakableToken.cpp?rev=185818&r1=185817&r2=185818&view=diff
==============================================================================
--- cfe/trunk/lib/Format/BreakableToken.cpp (original)
+++ cfe/trunk/lib/Format/BreakableToken.cpp Mon Jul 8 09:12:07 2013
@@ -225,50 +225,56 @@ BreakableBlockComment::BreakableBlockCom
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 + 1 == e && Lines[i].empty())
+ break;
+ 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,9 +371,11 @@ BreakableBlockComment::replaceWhitespace
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 was 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.
@@ -375,7 +383,7 @@ BreakableBlockComment::replaceWhitespace
}
} else {
if (StartOfLineColumn[LineIndex] == 1) {
- // This lines starts immediately after the decorating *.
+ // This line starts immediately after the decorating *.
Prefix = Prefix.substr(0, 1);
}
}
Modified: cfe/trunk/lib/Format/BreakableToken.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/BreakableToken.h?rev=185818&r1=185817&r2=185818&view=diff
==============================================================================
--- cfe/trunk/lib/Format/BreakableToken.h (original)
+++ cfe/trunk/lib/Format/BreakableToken.h Mon Jul 8 09:12:07 2013
@@ -208,6 +208,10 @@ private:
// 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;
};
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=185818&r1=185817&r2=185818&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Jul 8 09:12:07 2013
@@ -3967,7 +3967,7 @@ TEST_F(FormatTest, BlockComments) {
EXPECT_EQ("/*\n"
"*\n"
" * aaaaaa\n"
- "* aaaaaa\n"
+ "*aaaaaa\n"
"*/",
format("/*\n"
"*\n"
@@ -3977,7 +3977,7 @@ TEST_F(FormatTest, BlockComments) {
EXPECT_EQ("/*\n"
"**\n"
"* aaaaaa\n"
- "* aaaaaa\n"
+ "*aaaaaa\n"
"*/",
format("/*\n"
"**\n"
@@ -4017,6 +4017,33 @@ TEST_F(FormatTest, BlockComments) {
"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) {
More information about the cfe-commits
mailing list