ksyx updated this revision to Diff 401293.
ksyx edited the summary of this revision.
ksyx added a comment.
Recognize long (len>=5) uppercased name taking a single line as return type
and fix the problem of adding newline below it:
void
afunc(int x) {
return;
}
TYPENAME
func(int x, int y) {
// ...
}
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
@@ -128,6 +128,60 @@
"\n"
"enum Bar { FOOBAR, BARFOO };\n",
Style);
+
+ FormatStyle BreakAfterReturnTypeStyle = Style;
+ BreakAfterReturnTypeStyle.AlwaysBreakAfterReturnType = FormatStyle::RTBS_All;
+ // Test uppercased long typename
+ verifyFormat(
+ "class Foo {\n"
+ " void\n"
+ " Bar(int t, int p) {\n"
+ " int r = t + p;\n"
+ " return r;\n"
+ " }\n"
+ "\n"
+ " LONGTYPENAME\n"
+ " Foobar(int t, int p) {\n"
+ " int r = t * p;\n"
+ " return r;\n"
+ " }\n"
+ "}\n",
+ BreakAfterReturnTypeStyle);
+}
+
+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) {
@@ -172,13 +226,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 +266,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 +278,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 +292,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 +319,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 +331,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 +345,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 +373,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 +387,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 +405,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 +429,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 +441,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 +454,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;
}
@@ -63,18 +68,25 @@
AnnotatedLine *TargetLine;
auto OpeningLineIndex = CurrentLine->MatchingOpeningBlockLineIndex;
AnnotatedLine *OpeningLine = nullptr;
+ const auto IsAccessSpecifierToken = [](const FormatToken *Token) {
+ return Token->isAccessSpecifier() || Token->isObjCAccessSpecifier();
+ };
const auto InsertReplacement = [&](const int NewlineToInsert) {
assert(TargetLine);
assert(TargetToken);
// Do not handle EOF newlines.
- if (TargetToken->is(tok::eof) && NewlineToInsert > 0)
+ if (TargetToken->is(tok::eof))
+ return;
+ if (IsAccessSpecifierToken(TargetToken) ||
+ (OpeningLineIndex > 0 &&
+ IsAccessSpecifierToken(Lines[OpeningLineIndex - 1]->First)))
return;
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,26 +101,59 @@
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;
const auto MayPrecedeDefinition = [&](const int Direction = -1) {
+ assert(Direction >= -1 && Direction <= 1);
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);
+ if (LikelyDefinition(OperateLine))
+ return false;
+
+ if (OperateLine->First->is(tok::comment))
+ return true;
+
+ // A single line identifier that is not in the last line
+ if (OperateLine->First->is(tok::identifier) &&
+ OperateLine->First == OperateLine->Last &&
+ OperateIndex + 1 < Lines.size()) {
+ // UnwrappedLineParser's recognition of free-standing macro like
+ // Q_OBJECT may also recognize some uppercased type names that may be
+ // used as return type as that kind of macros, which is a bit hard to
+ // distinguish one from another purely from token patterns. Here, we
+ // try not to add new lines below those identifiers.
+ AnnotatedLine *NextLine = Lines[OperateIndex + 1];
+ if (NextLine->MightBeFunctionDecl &&
+ NextLine->mightBeFunctionDefinition()) {
+ if (NextLine->First->NewlinesBefore == 1) {
+ const llvm::StringRef &Text = OperateLine->First->TokenText;
+ if (Text.size() >= 5 && Text.upper() == Text)
+ return true;
+ }
+ }
+ }
+
+ if ((Style.isCSharp() && OperateLine->First->is(TT_AttributeSquare)))
+ return true;
+ return false;
};
- if (HasEnumOnLine()) {
+ if (HasEnumOnLine() &&
+ !LikelyDefinition(CurrentLine, /*ExcludeEnum=*/true)) {
// We have no scope opening/closing information for enum.
IsDefBlock = true;
OpeningLineIndex = I;
@@ -132,8 +177,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)) {
@@ -168,15 +218,13 @@
++OpeningLineIndex;
TargetLine = Lines[OpeningLineIndex];
if (!LikelyDefinition(TargetLine)) {
+ OpeningLineIndex = I + 1;
TargetLine = Lines[I + 1];
TargetToken = TargetLine->First;
InsertReplacement(NewlineCount);
}
- } else if (IsNeverStyle) {
- TargetLine = Lines[I + 1];
- TargetToken = TargetLine->First;
- InsertReplacement(OpeningLineIndex != 0);
- }
+ } else if (IsNeverStyle)
+ InsertReplacement(/*NewlineToInsert=*/1);
}
}
for (const auto &R : Whitespaces.generateReplacements())
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
