summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorEmilia Kond <emilia@rymiel.space>2023-05-18 05:50:10 +0300
committerEmilia Kond <emilia@rymiel.space>2023-05-18 05:50:26 +0300
commit06763ea5d8f96625545bb2c2445363aca9922bf1 (patch)
treed3281bb15e2209d534d0f34386d02249db7cfa58
parent2d5ef39184a4f17d75b0cd640f442cc43fe7edfc (diff)
downloadllvm-06763ea5d8f96625545bb2c2445363aca9922bf1.tar.gz
[clang-format] Ignore first token when finding MustBreak
When in ColumnLimit 0, the formatter looks for MustBreakBefore in the line in order to check if a line is allowed to be merged onto one line. However, since MustBreakBefore is really a property of the gap between the token and the one previously, I belive the check is erroneous in checking all the tokens in a line, since whether the previous line ended with a forced line break should have no effect on whether the current line is allowed to merge with the next one. This patch changes the check to skip the first token in `LineJoiner.containsMustBreak`. This patch also changes a test, which is not ideal, but I believe the test also suffered from this bug. The test case in question sets AllowShortFunctionsOnASingleLine to "Empty", but the empty function in said test case isn't merged to a single line, because of the very same bug this patch fixes. Fixes https://github.com/llvm/llvm-project/issues/62721 Reviewed By: HazardyKnusperkeks, owenpan, MyDeveloperDay Differential Revision: https://reviews.llvm.org/D150614
-rw-r--r--clang/lib/Format/UnwrappedLineFormatter.cpp5
-rw-r--r--clang/unittests/Format/FormatTest.cpp18
2 files changed, 19 insertions, 4 deletions
diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp
index ee0de2c8e20d..33be74dfe1b9 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -888,7 +888,10 @@ private:
}
bool containsMustBreak(const AnnotatedLine *Line) {
- for (const FormatToken *Tok = Line->First; Tok; Tok = Tok->Next)
+ assert(Line->First);
+ // Ignore the first token, because in this situation, it applies more to the
+ // last token of the previous line.
+ for (const FormatToken *Tok = Line->First->Next; Tok; Tok = Tok->Next)
if (Tok->MustBreakBefore)
return true;
return false;
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index dec0ea72b58a..942c6259015e 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -13832,6 +13832,20 @@ TEST_F(FormatTest, PullTrivialFunctionDefinitionsIntoSingleLine) {
"}",
format("A()\n:b(0)\n{\n}", NoColumnLimit));
+ FormatStyle NoColumnLimitWrapAfterFunction = NoColumnLimit;
+ NoColumnLimitWrapAfterFunction.BreakBeforeBraces = FormatStyle::BS_Custom;
+ NoColumnLimitWrapAfterFunction.BraceWrapping.AfterFunction = true;
+ verifyFormat("class C {\n"
+ "#pragma foo\n"
+ " int foo { return 0; }\n"
+ "};",
+ NoColumnLimitWrapAfterFunction);
+ verifyFormat("class C {\n"
+ "#pragma foo\n"
+ " void foo {}\n"
+ "};",
+ NoColumnLimitWrapAfterFunction);
+
FormatStyle DoNotMergeNoColumnLimit = NoColumnLimit;
DoNotMergeNoColumnLimit.AllowShortFunctionsOnASingleLine =
FormatStyle::SFS_None;
@@ -20120,9 +20134,7 @@ TEST_F(FormatTest, WhitesmithsBraceBreaking) {
" int i = 5;\n"
" }\n"
"#ifdef _DEBUG\n"
- "void bar()\n"
- " {\n"
- " }\n"
+ "void bar() {}\n"
"#else\n"
"void bar()\n"
" {\n"