ksyx updated this revision to Diff 402078.
ksyx marked an inline comment as done.
ksyx added a comment.
Apply review suggestions of renaming.
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/lib/Format/FormatToken.h
clang/lib/Format/TokenAnnotator.cpp
clang/lib/Format/UnwrappedLineParser.cpp
clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
Index: clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
===================================================================
--- clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
+++ clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
@@ -131,6 +131,73 @@
"\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"
+ " HRESULT\n"
+ " Foobar(int t, int p) {\n"
+ " int r = t * p;\n"
+ " return r;\n"
+ " }\n"
+ "}\n",
+ BreakAfterReturnTypeStyle);
+}
+
+TEST_F(DefinitionBlockSeparatorTest, FormatConflict) {
+ FormatStyle Style = getLLVMStyle();
+ Style.SeparateDefinitionBlocks = FormatStyle::SDS_Always;
+ llvm::StringRef Code = "class Test {\n"
+ "public:\n"
+ " static void foo() {\n"
+ " int t;\n"
+ " return 1;\n"
+ " }\n"
+ "};";
+ std::vector<tooling::Range> Ranges = {1, tooling::Range(0, Code.size())};
+ EXPECT_EQ(reformat(Style, Code, Ranges, "<stdin>").size(), 0u);
+}
+
+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) {
@@ -175,13 +242,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));
@@ -213,7 +282,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"
@@ -225,8 +294,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"
@@ -237,7 +308,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"
@@ -264,7 +335,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"
@@ -276,8 +347,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"
@@ -288,7 +361,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"
@@ -316,7 +389,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"
@@ -330,8 +403,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"
@@ -346,7 +421,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"
@@ -370,7 +445,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"
@@ -382,8 +457,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"
@@ -393,7 +470,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/UnwrappedLineParser.cpp
===================================================================
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1682,6 +1682,8 @@
// See if the following token should start a new unwrapped line.
StringRef Text = FormatTok->TokenText;
+
+ FormatToken *PreviousToken = FormatTok;
nextToken();
// JS doesn't have macros, and within classes colons indicate fields, not
@@ -1710,6 +1712,7 @@
if (FollowedByNewline && (Text.size() >= 5 || FunctionLike) &&
tokenCanStartNewLine(*FormatTok) && Text == Text.upper()) {
+ PreviousToken->setType(TT_FunctionLikeOrFreestandingMacro);
addUnwrappedLine();
return;
}
Index: clang/lib/Format/TokenAnnotator.cpp
===================================================================
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1423,7 +1423,7 @@
TT_LambdaArrow, TT_NamespaceMacro, TT_OverloadedOperator,
TT_RegexLiteral, TT_TemplateString, TT_ObjCStringLiteral,
TT_UntouchableMacroFunc, TT_ConstraintJunctions,
- TT_StatementAttributeLikeMacro))
+ TT_StatementAttributeLikeMacro, TT_FunctionLikeOrFreestandingMacro))
CurrentToken->setType(TT_Unknown);
CurrentToken->Role.reset();
CurrentToken->MatchingParen = nullptr;
Index: clang/lib/Format/FormatToken.h
===================================================================
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -51,6 +51,7 @@
TYPE(FunctionAnnotationRParen) \
TYPE(FunctionDeclarationName) \
TYPE(FunctionLBrace) \
+ TYPE(FunctionLikeOrFreestandingMacro) \
TYPE(FunctionTypeLParen) \
TYPE(IfMacro) \
TYPE(ImplicitStringLiteral) \
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,57 @@
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);
+ assert(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 &&
+ OperateLine->First->is(TT_FunctionLikeOrFreestandingMacro))
+ 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 +175,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 +216,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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
