ksyx updated this revision to Diff 400821.
ksyx marked 3 inline comments as done.
ksyx edited the summary of this revision.
ksyx added a comment.
- Use function keyword for JavaScript instead of comparing strings
- Apply review suggestions
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D117520/new/
https://reviews.llvm.org/D117520
Files:
clang/lib/Format/DefinitionBlockSeparator.cpp
clang/lib/Format/DefinitionBlockSeparator.h
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,15 @@
FormatStyle NeverStyle = getLLVMStyle();
NeverStyle.SeparateDefinitionBlocks = FormatStyle::SDS_Never;
- auto TestKit = MakeUntouchTest("#ifdef FOO\n\n", "\n#elifndef BAR\n\n",
- "\n#endif\n\n", false);
+ 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 +247,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 +259,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 +273,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 +300,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 +312,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 +326,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 +354,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 +368,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 +386,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 +410,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 +422,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 +435,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.h
===================================================================
--- clang/lib/Format/DefinitionBlockSeparator.h
+++ clang/lib/Format/DefinitionBlockSeparator.h
@@ -33,7 +33,7 @@
private:
void separateBlocks(SmallVectorImpl<AnnotatedLine *> &Lines,
- tooling::Replacements &Result);
+ tooling::Replacements &Result, FormatTokenLexer &Tokens);
};
} // namespace format
} // namespace clang
Index: clang/lib/Format/DefinitionBlockSeparator.cpp
===================================================================
--- clang/lib/Format/DefinitionBlockSeparator.cpp
+++ clang/lib/Format/DefinitionBlockSeparator.cpp
@@ -25,22 +25,27 @@
assert(Style.SeparateDefinitionBlocks != FormatStyle::SDS_Leave);
AffectedRangeMgr.computeAffectedLines(AnnotatedLines);
tooling::Replacements Result;
- separateBlocks(AnnotatedLines, Result);
+ separateBlocks(AnnotatedLines, Result, Tokens);
return {Result, 0};
}
void DefinitionBlockSeparator::separateBlocks(
- SmallVectorImpl<AnnotatedLine *> &Lines, tooling::Replacements &Result) {
+ SmallVectorImpl<AnnotatedLine *> &Lines, tooling::Replacements &Result,
+ FormatTokenLexer &Tokens) {
const bool IsNeverStyle =
Style.SeparateDefinitionBlocks == FormatStyle::SDS_Never;
- auto LikelyDefinition = [this](const AnnotatedLine *Line) {
+ const AdditionalKeywords &ExtraKeywords = Tokens.getKeywords();
+ auto LikelyDefinition = [this, ExtraKeywords](const AnnotatedLine *Line,
+ 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) ||
- (Style.isJavaScript() && CurrentToken->TokenText == "function"))
+ if (CurrentToken->isOneOf(tok::kw_class, tok::kw_struct) ||
+ (Style.isJavaScript() && CurrentToken->is(ExtraKeywords.kw_function)))
+ return true;
+ if (!ExcludeEnum && CurrentToken->is(tok::kw_enum))
return true;
CurrentToken = CurrentToken->Next;
}
@@ -73,8 +78,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 +94,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 +113,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 +144,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 brace has its own line, with checking
+ // whether the last line already had an opening brace 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
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
