From: Alexander Kornienko Date: Mon, 8 Jul 2013 14:12:07 +0000 (+0000) Subject: Fix for corner cases in code handling leading "* " decorations in block comments X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=1659dedac63858de50ee60175a88c42ff974e61b;p=clang 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 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@185818 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Format/BreakableToken.cpp b/lib/Format/BreakableToken.cpp index ace3baa660..f01b481b84 100644 --- a/lib/Format/BreakableToken.cpp +++ b/lib/Format/BreakableToken.cpp @@ -225,50 +225,56 @@ BreakableBlockComment::BreakableBlockComment( 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(IndentAtLineBreak, StartOfLineColumn[i]); } IndentAtLineBreak = std::max(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::replaceWhitespaceBefore(unsigned LineIndex, 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::replaceWhitespaceBefore(unsigned LineIndex, } } else { if (StartOfLineColumn[LineIndex] == 1) { - // This lines starts immediately after the decorating *. + // This line starts immediately after the decorating *. Prefix = Prefix.substr(0, 1); } } diff --git a/lib/Format/BreakableToken.h b/lib/Format/BreakableToken.h index 9dab473d8a..afcc8b83a5 100644 --- a/lib/Format/BreakableToken.h +++ b/lib/Format/BreakableToken.h @@ -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; }; diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp index 96ea9de9c6..cff178eeb9 100644 --- a/unittests/Format/FormatTest.cpp +++ b/unittests/Format/FormatTest.cpp @@ -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) {