ksyx created this revision.
ksyx added reviewers: MyDeveloperDay, curdeius, HazardyKnusperkeks.
ksyx requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
- Fixes https://github.com/llvm/llvm-project/issues/53227 that wrongly indents
multiline comments
- Fixes wrong detection of single-line opening bracket when used along with
those only opening scopes:
void foo()
{
{
int x;
}
}
- Fixes wrong recognition of first line of definition when the line starts with
block comment:
/*
Some descriptions about function
*/
/*inline*/ void bar() {
}
- Fixes wrong recognition of enum when used as a type name rather than starting
definition block:
void foobar(const enum EnumType e) {
}
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D117520
Files:
clang/lib/Format/DefinitionBlockSeparator.cpp
clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
Index: clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
===================================================================
--- clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
+++ clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
@@ -130,6 +130,41 @@
Style);
}
+TEST_F(DefinitionBlockSeparatorTest, CommentBlock) {
+ FormatStyle Style = getLLVMStyle();
+ Style.SeparateDefinitionBlocks = FormatStyle::SDS_Always;
+ std::string Prefix = "enum Foo { FOO, BAR };\n"
+ "\n"
+ "/*\n"
+ "test1\n"
+ "test2\n"
+ "*/\n"
+ "int foo(int i, int j) {\n"
+ " int r = i + j;\n"
+ " return r;\n"
+ "}\n";
+ std::string Suffix = "enum Bar { FOOBAR, BARFOO };\n"
+ "\n"
+ "/* Comment block in one line*/\n"
+ "int bar3(int j, int k) {\n"
+ " // A comment\n"
+ " int r = j % k;\n"
+ " return r;\n"
+ "}\n";
+ std::string CommentedCode = "/*\n"
+ "int bar2(int j, int k) {\n"
+ " int r = j / k;\n"
+ " return r;\n"
+ "}\n"
+ "*/\n";
+ verifyFormat(removeEmptyLines(Prefix) + "\n" + CommentedCode + "\n" +
+ removeEmptyLines(Suffix),
+ Style, Prefix + "\n" + CommentedCode + "\n" + Suffix);
+ verifyFormat(removeEmptyLines(Prefix) + "\n" + CommentedCode +
+ removeEmptyLines(Suffix),
+ Style, Prefix + "\n" + CommentedCode + Suffix);
+}
+
TEST_F(DefinitionBlockSeparatorTest, UntouchBlockStartStyle) {
// Returns a std::pair of two strings, with the first one for passing into
// Always test and the second one be the expected result of the first string.
@@ -172,13 +207,17 @@
FormatStyle NeverStyle = getLLVMStyle();
NeverStyle.SeparateDefinitionBlocks = FormatStyle::SDS_Never;
- auto TestKit = MakeUntouchTest("#ifdef FOO\n\n", "\n#elifndef BAR\n\n",
+ auto TestKit = MakeUntouchTest("/* FOOBAR */\n"
+ "#ifdef FOO\n\n",
+ "\n#elifndef BAR\n\n",
"\n#endif\n\n", false);
verifyFormat(TestKit.first, AlwaysStyle, TestKit.second);
verifyFormat(TestKit.second, NeverStyle, removeEmptyLines(TestKit.second));
- TestKit =
- MakeUntouchTest("#ifdef FOO\n", "#elifndef BAR\n", "#endif\n", false);
+ TestKit = MakeUntouchTest("/* FOOBAR */\n"
+ "#ifdef FOO\n",
+ "#elifndef BAR\n",
+ "#endif\n", false);
verifyFormat(TestKit.first, AlwaysStyle, TestKit.second);
verifyFormat(TestKit.second, NeverStyle, removeEmptyLines(TestKit.second));
@@ -210,7 +249,7 @@
"test1\n"
"test2\n"
"*/\n"
- "int foo(int i, int j) {\n"
+ "/*const*/ int foo(int i, int j) {\n"
" int r = i + j;\n"
" return r;\n"
"}\n"
@@ -222,8 +261,10 @@
"// Comment line 2\n"
"// Comment line 3\n"
"int bar(int j, int k) {\n"
- " int r = j * k;\n"
- " return r;\n"
+ " {\n"
+ " int r = j * k;\n"
+ " return r;\n"
+ " }\n"
"}\n"
"\n"
"int bar2(int j, int k) {\n"
@@ -234,7 +275,7 @@
"/* Comment block in one line*/\n"
"enum Bar { FOOBAR, BARFOO };\n"
"\n"
- "int bar3(int j, int k) {\n"
+ "int bar3(int j, int k, const enum Bar b) {\n"
" // A comment\n"
" int r = j % k;\n"
" return r;\n"
@@ -261,7 +302,7 @@
"test1\n"
"test2\n"
"*/\n"
- "int foo(int i, int j) {\n"
+ "/*const*/ int foo(int i, int j) {\n"
" int r = i + j;\n"
" return r;\n"
"}\n"
@@ -273,8 +314,10 @@
"// Comment line 2\n"
"// Comment line 3\n"
"int bar(int j, int k) {\n"
- " int r = j * k;\n"
- " return r;\n"
+ " {\n"
+ " int r = j * k;\n"
+ " return r;\n"
+ " }\n"
"}\n"
"\n"
"int bar2(int j, int k) {\n"
@@ -285,7 +328,7 @@
"/* Comment block in one line*/\n"
"enum Bar { FOOBAR, BARFOO };\n"
"\n"
- "int bar3(int j, int k) {\n"
+ "int bar3(int j, int k, const enum Bar b) {\n"
" // A comment\n"
" int r = j % k;\n"
" return r;\n"
@@ -313,7 +356,7 @@
"test1\n"
"test2\n"
"*/\n"
- "int foo(int i, int j)\n"
+ "/*const*/ int foo(int i, int j)\n"
"{\n"
" int r = i + j;\n"
" return r;\n"
@@ -327,8 +370,10 @@
"// Comment line 3\n"
"int bar(int j, int k)\n"
"{\n"
- " int r = j * k;\n"
- " return r;\n"
+ " {\n"
+ " int r = j * k;\n"
+ " return r;\n"
+ " }\n"
"}\n"
"\n"
"int bar2(int j, int k)\n"
@@ -343,7 +388,7 @@
" BARFOO\n"
"};\n"
"\n"
- "int bar3(int j, int k)\n"
+ "int bar3(int j, int k, const enum Bar b)\n"
"{\n"
" // A comment\n"
" int r = j % k;\n"
@@ -367,7 +412,7 @@
"test1\n"
"test2\n"
"*/\n"
- "int foo(int i, int j) {\n"
+ "/*const*/ int foo(int i, int j) {\n"
" int r = i + j;\n"
" return r;\n"
"}\n"
@@ -379,8 +424,10 @@
"// Comment line 2\n"
"// Comment line 3\n"
"int bar(int j, int k) {\n"
- " int r = j * k;\n"
- " return r;\n"
+ " {\n"
+ " int r = j * k;\n"
+ " return r;\n"
+ " }\n"
"}\n"
"\n"
"int bar2(int j, int k) {\n"
@@ -390,7 +437,7 @@
"\n"
"// Comment for inline enum\n"
"enum Bar { FOOBAR, BARFOO };\n"
- "int bar3(int j, int k) {\n"
+ "int bar3(int j, int k, const enum Bar b) {\n"
" // A comment\n"
" int r = j % k;\n"
" return r;\n"
Index: clang/lib/Format/DefinitionBlockSeparator.cpp
===================================================================
--- clang/lib/Format/DefinitionBlockSeparator.cpp
+++ clang/lib/Format/DefinitionBlockSeparator.cpp
@@ -33,15 +33,18 @@
SmallVectorImpl<AnnotatedLine *> &Lines, tooling::Replacements &Result) {
const bool IsNeverStyle =
Style.SeparateDefinitionBlocks == FormatStyle::SDS_Never;
- auto LikelyDefinition = [this](const AnnotatedLine *Line) {
+ auto LikelyDefinition = [this](const AnnotatedLine *Line,
+ const bool ExcludeEnum = false) {
if ((Line->MightBeFunctionDecl && Line->mightBeFunctionDefinition()) ||
Line->startsWithNamespace())
return true;
FormatToken *CurrentToken = Line->First;
while (CurrentToken) {
- if (CurrentToken->isOneOf(tok::kw_class, tok::kw_struct, tok::kw_enum) ||
+ if (CurrentToken->isOneOf(tok::kw_class, tok::kw_struct) ||
(Style.isJavaScript() && CurrentToken->TokenText == "function"))
return true;
+ if (!ExcludeEnum && CurrentToken->is(tok::kw_enum))
+ return true;
CurrentToken = CurrentToken->Next;
}
return false;
@@ -73,8 +76,8 @@
if (!TargetLine->Affected)
return;
Whitespaces.replaceWhitespace(*TargetToken, NewlineToInsert,
- TargetToken->SpacesRequiredBefore - 1,
- TargetToken->StartsColumn);
+ TargetToken->OriginalColumn,
+ TargetToken->OriginalColumn);
};
const auto IsPPConditional = [&](const size_t LineIndex) {
const auto &Line = Lines[LineIndex];
@@ -89,14 +92,18 @@
Lines[OpeningLineIndex - 1]->Last->opensScope() ||
IsPPConditional(OpeningLineIndex - 1);
};
- const auto HasEnumOnLine = [CurrentLine]() {
+ const auto HasEnumOnLine = [&]() {
FormatToken *CurrentToken = CurrentLine->First;
+ bool FoundEnumKeyword = false;
while (CurrentToken) {
if (CurrentToken->is(tok::kw_enum))
+ FoundEnumKeyword = true;
+ else if (FoundEnumKeyword && CurrentToken->is(tok::l_brace))
return true;
CurrentToken = CurrentToken->Next;
}
- return false;
+ return FoundEnumKeyword && I + 1 < Lines.size() &&
+ Lines[I + 1]->First->is(tok::l_brace);
};
bool IsDefBlock = false;
@@ -104,11 +111,14 @@
const size_t OperateIndex = OpeningLineIndex + Direction;
assert(OperateIndex < Lines.size());
const auto &OperateLine = Lines[OperateIndex];
- return (Style.isCSharp() && OperateLine->First->is(TT_AttributeSquare)) ||
- OperateLine->First->is(tok::comment);
+ return ((Style.isCSharp() &&
+ OperateLine->First->is(TT_AttributeSquare)) ||
+ OperateLine->First->is(tok::comment)) &&
+ !LikelyDefinition(OperateLine);
};
- if (HasEnumOnLine()) {
+ if (HasEnumOnLine() &&
+ !LikelyDefinition(CurrentLine, /*ExcludeEnum=*/true)) {
// We have no scope opening/closing information for enum.
IsDefBlock = true;
OpeningLineIndex = I;
@@ -132,8 +142,13 @@
} else if (CurrentLine->First->closesScope()) {
if (OpeningLineIndex > Lines.size())
continue;
- // Handling the case that opening bracket has its own line.
- OpeningLineIndex -= Lines[OpeningLineIndex]->First->is(tok::l_brace);
+ // Handling the case that opening bracket has its own line, with checking
+ // whether the last line already had an opening bracket to guard against
+ // misrecognition.
+ if (OpeningLineIndex > 0 &&
+ Lines[OpeningLineIndex]->First->is(tok::l_brace) &&
+ Lines[OpeningLineIndex - 1]->Last->isNot(tok::l_brace))
+ --OpeningLineIndex;
OpeningLine = Lines[OpeningLineIndex];
// Closing a function definition.
if (LikelyDefinition(OpeningLine)) {
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
