forked from OSchip/llvm-project
[clang-format] Fix crash while reflowing backslash in comments
Summary: The added test case was currently crashing with an assertion: ``` krasimir@krasimir> cat test.cc ~ // How to run: // bbbbb run \ // rrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrr \ // <log_file> -- --output_directory="<output_directory>" krasimir@krasimir> ~/work/llvm-build/bin/clang-format test.cc ~ clang-format: /usr/local/google/home/krasimir/work/llvm/tools/clang/lib/Format/WhitespaceManager.cpp:117: void clang::format::WhitespaceManager::calculateLineBreakInformation(): Assertion `PreviousOriginalWhitespaceEndOffset <= OriginalWhitespaceStartOffset' failed. ``` The root cause was that BreakableToken was not considering the case of a reflow between an unescaped newline in a line comment. Subscribers: cfe-commits Differential Revision: https://reviews.llvm.org/D48089 llvm-svn: 334527
This commit is contained in:
parent
4872750dd3
commit
4fc55a76b8
|
@ -789,16 +789,47 @@ BreakableComment::Split BreakableLineCommentSection::getReflowSplit(
|
||||||
|
|
||||||
void BreakableLineCommentSection::reflow(unsigned LineIndex,
|
void BreakableLineCommentSection::reflow(unsigned LineIndex,
|
||||||
WhitespaceManager &Whitespaces) const {
|
WhitespaceManager &Whitespaces) const {
|
||||||
|
if (LineIndex > 0 && Tokens[LineIndex] != Tokens[LineIndex - 1]) {
|
||||||
// Reflow happens between tokens. Replace the whitespace between the
|
// Reflow happens between tokens. Replace the whitespace between the
|
||||||
// tokens by the empty string.
|
// tokens by the empty string.
|
||||||
Whitespaces.replaceWhitespace(
|
Whitespaces.replaceWhitespace(
|
||||||
*Tokens[LineIndex], /*Newlines=*/0, /*Spaces=*/0,
|
*Tokens[LineIndex], /*Newlines=*/0, /*Spaces=*/0,
|
||||||
/*StartOfTokenColumn=*/StartColumn, /*InPPDirective=*/false);
|
/*StartOfTokenColumn=*/StartColumn, /*InPPDirective=*/false);
|
||||||
// Replace the indent and prefix of the token with the reflow prefix.
|
} else if (LineIndex > 0) {
|
||||||
|
// In case we're reflowing after the '\' in:
|
||||||
|
//
|
||||||
|
// // line comment \
|
||||||
|
// // line 2
|
||||||
|
//
|
||||||
|
// the reflow happens inside the single comment token (it is a single line
|
||||||
|
// comment with an unescaped newline).
|
||||||
|
// Replace the whitespace between the '\' and '//' with the empty string.
|
||||||
|
//
|
||||||
|
// Offset points to after the '\' relative to start of the token.
|
||||||
|
unsigned Offset = Lines[LineIndex - 1].data() +
|
||||||
|
Lines[LineIndex - 1].size() -
|
||||||
|
tokenAt(LineIndex - 1).TokenText.data();
|
||||||
|
// WhitespaceLength is the number of chars between the '\' and the '//' on
|
||||||
|
// the next line.
|
||||||
unsigned WhitespaceLength =
|
unsigned WhitespaceLength =
|
||||||
Content[LineIndex].data() - tokenAt(LineIndex).TokenText.data();
|
Lines[LineIndex].data() - tokenAt(LineIndex).TokenText.data() - Offset;
|
||||||
Whitespaces.replaceWhitespaceInToken(*Tokens[LineIndex],
|
Whitespaces.replaceWhitespaceInToken(*Tokens[LineIndex],
|
||||||
/*Offset=*/0,
|
Offset,
|
||||||
|
/*ReplaceChars=*/WhitespaceLength,
|
||||||
|
/*PreviousPostfix=*/"",
|
||||||
|
/*CurrentPrefix=*/"",
|
||||||
|
/*InPPDirective=*/false,
|
||||||
|
/*Newlines=*/0,
|
||||||
|
/*Spaces=*/0);
|
||||||
|
|
||||||
|
}
|
||||||
|
// Replace the indent and prefix of the token with the reflow prefix.
|
||||||
|
unsigned Offset =
|
||||||
|
Lines[LineIndex].data() - tokenAt(LineIndex).TokenText.data();
|
||||||
|
unsigned WhitespaceLength =
|
||||||
|
Content[LineIndex].data() - Lines[LineIndex].data();
|
||||||
|
Whitespaces.replaceWhitespaceInToken(*Tokens[LineIndex],
|
||||||
|
Offset,
|
||||||
/*ReplaceChars=*/WhitespaceLength,
|
/*ReplaceChars=*/WhitespaceLength,
|
||||||
/*PreviousPostfix=*/"",
|
/*PreviousPostfix=*/"",
|
||||||
/*CurrentPrefix=*/ReflowPrefix,
|
/*CurrentPrefix=*/ReflowPrefix,
|
||||||
|
|
|
@ -3090,6 +3090,21 @@ TEST_F(FormatTestComments, BreaksBeforeTrailingUnbreakableSequence) {
|
||||||
getLLVMStyleWithColumns(23));
|
getLLVMStyleWithColumns(23));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
TEST_F(FormatTestComments, ReflowBackslashCrash) {
|
||||||
|
// clang-format off
|
||||||
|
EXPECT_EQ(
|
||||||
|
"// How to run:\n"
|
||||||
|
"// bbbbb run \\\n"
|
||||||
|
"// rrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrr\n"
|
||||||
|
"// \\ <log_file> -- --output_directory=\"<output_directory>\"",
|
||||||
|
format(
|
||||||
|
"// How to run:\n"
|
||||||
|
"// bbbbb run \\\n"
|
||||||
|
"// rrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrr \\\n"
|
||||||
|
"// <log_file> -- --output_directory=\"<output_directory>\""));
|
||||||
|
// clang-format on
|
||||||
|
}
|
||||||
|
|
||||||
} // end namespace
|
} // end namespace
|
||||||
} // end namespace format
|
} // end namespace format
|
||||||
} // end namespace clang
|
} // end namespace clang
|
||||||
|
|
Loading…
Reference in New Issue