Author: Krasimir Georgiev Date: 2020-11-09T15:29:09+01:00 New Revision: 7b7170fa5791a8fcf28418e6daa1ab7dd2126a25
URL: https://github.com/llvm/llvm-project/commit/7b7170fa5791a8fcf28418e6daa1ab7dd2126a25 DIFF: https://github.com/llvm/llvm-project/commit/7b7170fa5791a8fcf28418e6daa1ab7dd2126a25.diff LOG: [clang-format] avoid introducing multiline comments In C++ with -Werror=comment, multiline comments are not allowed. clang-format could accidentally introduce multiline comments when reflowing. This adapts clang-format to not introduce multiline comments by not allowing a break after `\`. Note that this does not apply to comment lines that already are multiline comments, such as comments in macros. Reviewed By: sammccall Differential Revision: https://reviews.llvm.org/D90949 Added: Modified: clang/lib/Format/BreakableToken.cpp clang/unittests/Format/FormatTestComments.cpp Removed: ################################################################################ diff --git a/clang/lib/Format/BreakableToken.cpp b/clang/lib/Format/BreakableToken.cpp index 2ef1540a7f82..c3853687c228 100644 --- a/clang/lib/Format/BreakableToken.cpp +++ b/clang/lib/Format/BreakableToken.cpp @@ -101,17 +101,37 @@ getCommentSplit(StringRef Text, unsigned ContentStartColumn, StringRef::size_type SpaceOffset = Text.find_last_of(Blanks, MaxSplitBytes); static const auto kNumberedListRegexp = llvm::Regex("^[1-9][0-9]?\\."); + // Some spaces are unacceptable to break on, rewind past them. while (SpaceOffset != StringRef::npos) { + // If a line-comment ends with `\`, the next line continues the comment, + // whether or not it starts with `//`. This is confusing and triggers + // -Wcomment. + // Avoid introducing multiline comments by not allowing a break right + // after '\'. + if (Style.isCpp()) { + StringRef::size_type LastNonBlank = + Text.find_last_not_of(Blanks, SpaceOffset); + if (LastNonBlank != StringRef::npos && Text[LastNonBlank] == '\\') { + SpaceOffset = Text.find_last_of(Blanks, LastNonBlank); + continue; + } + } + // Do not split before a number followed by a dot: this would be interpreted // as a numbered list, which would prevent re-flowing in subsequent passes. - if (kNumberedListRegexp.match(Text.substr(SpaceOffset).ltrim(Blanks))) + if (kNumberedListRegexp.match(Text.substr(SpaceOffset).ltrim(Blanks))) { SpaceOffset = Text.find_last_of(Blanks, SpaceOffset); + continue; + } + // Avoid ever breaking before a { in JavaScript. - else if (Style.Language == FormatStyle::LK_JavaScript && - SpaceOffset + 1 < Text.size() && Text[SpaceOffset + 1] == '{') + if (Style.Language == FormatStyle::LK_JavaScript && + SpaceOffset + 1 < Text.size() && Text[SpaceOffset + 1] == '{') { SpaceOffset = Text.find_last_of(Blanks, SpaceOffset); - else - break; + continue; + } + + break; } if (SpaceOffset == StringRef::npos || diff --git a/clang/unittests/Format/FormatTestComments.cpp b/clang/unittests/Format/FormatTestComments.cpp index 95afb17114a2..27dfe71367b3 100644 --- a/clang/unittests/Format/FormatTestComments.cpp +++ b/clang/unittests/Format/FormatTestComments.cpp @@ -745,6 +745,24 @@ TEST_F(FormatTestComments, DontSplitLineCommentsWithEscapedNewlines) { getLLVMStyleWithColumns(49))); } +TEST_F(FormatTestComments, DontIntroduceMultilineComments) { + // Avoid introducing a multiline comment by breaking after `\`. + for (int ColumnLimit = 15; ColumnLimit <= 17; ++ColumnLimit) { + EXPECT_EQ( + "// aaaaaaaaaa\n" + "// \\ bb", + format("// aaaaaaaaaa \\ bb", getLLVMStyleWithColumns(ColumnLimit))); + EXPECT_EQ( + "// aaaaaaaaa\n" + "// \\ bb", + format("// aaaaaaaaa \\ bb", getLLVMStyleWithColumns(ColumnLimit))); + EXPECT_EQ( + "// aaaaaaaaa\n" + "// \\ \\ bb", + format("// aaaaaaaaa \\ \\ bb", getLLVMStyleWithColumns(ColumnLimit))); + } +} + TEST_F(FormatTestComments, DontSplitLineCommentsWithPragmas) { FormatStyle Pragmas = getLLVMStyleWithColumns(30); Pragmas.CommentPragmas = "^ IWYU pragma:"; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
