Don't break line comments with escaped newlines.

Summary:
These can appear when comments contain command lines with quoted line
breaks. As the text (including escaped newlines and '//' from consecutive lines)
is a single line comment, we used to break it even when it didn't exceed column
limit. This is a temporary solution, in the future we may want to support this
case completely - at least adjust leading whitespace when changing indentation
of the first line.

Reviewers: djasper

Reviewed By: djasper

CC: cfe-commits, klimek

Differential Revision: http://llvm-reviews.chandlerc.com/D1146

llvm-svn: 186456
This commit is contained in:
Alexander Kornienko 2013-07-16 21:06:13 +00:00
parent 8369aa5e12
commit 657c67b164
3 changed files with 76 additions and 2 deletions

View File

@ -905,6 +905,11 @@ private:
// Only break up default narrow strings.
if (!Current.TokenText.startswith("\""))
return 0;
// Don't break string literals with escaped newlines. As clang-format must
// not change the string's content, it is unlikely that we'll end up with
// a better format.
if (Current.TokenText.find("\\\n") != StringRef::npos)
return 0;
// Exempts unterminated string literals from line breaking. The user will
// likely want to terminate the string before any line breaking is done.
if (Current.IsUnterminatedLiteral)
@ -919,6 +924,23 @@ private:
} else if (Current.Type == TT_LineComment &&
(Current.Previous == NULL ||
Current.Previous->Type != TT_ImplicitStringLiteral)) {
// Don't break line comments with escaped newlines. These look like
// separate line comments, but in fact contain a single line comment with
// multiple lines including leading whitespace and the '//' markers.
//
// FIXME: If we want to handle them correctly, we'll need to adjust
// leading whitespace in consecutive lines when changing indentation of
// the first line similar to what we do with block comments.
StringRef::size_type EscapedNewlinePos = Current.TokenText.find("\\\n");
if (EscapedNewlinePos != StringRef::npos) {
State.Column =
StartColumn +
encoding::getCodePointCount(
Current.TokenText.substr(0, EscapedNewlinePos), Encoding) +
1;
return 0;
}
Token.reset(new BreakableLineComment(Current, StartColumn,
Line.InPPDirective, Encoding));
} else {

View File

@ -967,8 +967,9 @@ void TokenAnnotator::calculateFormattingInformation(AnnotatedLine &Line) {
Current->is(tok::string_literal) &&
Current->Previous->isNot(tok::lessless) &&
Current->Previous->Type != TT_InlineASMColon &&
Current->getNextNonComment() &&
Current->getNextNonComment()->is(tok::string_literal)) {
((Current->getNextNonComment() &&
Current->getNextNonComment()->is(tok::string_literal)) ||
(Current->TokenText.find("\\\n") != StringRef::npos))) {
Current->MustBreakBefore = true;
}
Current->CanBreakBefore =

View File

@ -921,6 +921,32 @@ TEST_F(FormatTest, SplitsLongCxxComments) {
getLLVMStyleWithColumns(20)));
}
TEST_F(FormatTest, DontSplitLineCommentsWithEscapedNewlines) {
EXPECT_EQ("// aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\\n"
"// aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\\n"
"// aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
format("// aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\\n"
"// aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\\n"
"// aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"));
EXPECT_EQ("int a; // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\\\n"
" // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\\\n"
" // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA",
format("int a; // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\\\n"
" // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\\\n"
" // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA",
getLLVMStyleWithColumns(50)));
// FIXME: One day we might want to implement adjustment of leading whitespace
// of the consecutive lines in this kind of comment:
EXPECT_EQ("int\n"
"a; // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\\\n"
" // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\\\n"
" // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA",
format("int a; // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\\\n"
" // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\\\n"
" // AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA",
getLLVMStyleWithColumns(49)));
}
TEST_F(FormatTest, PriorityOfCommentBreaking) {
EXPECT_EQ("if (xxx == yyy && // aaaaaaaaaaaa\n"
" // bbbbbbbbb\n"
@ -2969,6 +2995,21 @@ TEST_F(FormatTest, AlwaysBreakBeforeMultilineStrings) {
" \"bbbb\"\n"
" \"cccc\");",
format("aaaa(qqq, \"bbbb\" \"cccc\");", Break));
EXPECT_EQ("x = \"a\\\n"
"b\\\n"
"c\";",
format("x = \"a\\\n"
"b\\\n"
"c\";",
NoBreak));
EXPECT_EQ("x =\n"
" \"a\\\n"
"b\\\n"
"c\";",
format("x = \"a\\\n"
"b\\\n"
"c\";",
Break));
}
TEST_F(FormatTest, AlignsPipes) {
@ -4992,6 +5033,16 @@ TEST_F(FormatTest, BreakStringLiterals) {
format("#define A \"some text other\";", AlignLeft));
}
TEST_F(FormatTest, DontSplitStringLiteralsWithEscapedNewlines) {
EXPECT_EQ("aaaaaaaaaaa =\n"
" \"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\\n"
" aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\\n"
" aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\";",
format("aaaaaaaaaaa = \"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\\n"
" aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\\n"
" aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\";"));
}
TEST_F(FormatTest, SkipsUnknownStringLiterals) {
EXPECT_EQ(
"u8\"unsupported literal\";",