Index: clang/lib/Format/DefinitionBlockSeparator.h =================================================================== --- clang/lib/Format/DefinitionBlockSeparator.h +++ clang/lib/Format/DefinitionBlockSeparator.h @@ -33,7 +33,7 @@ private: void separateBlocks(SmallVectorImpl &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 &Lines, tooling::Replacements &Result) { + SmallVectorImpl &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,60 @@ 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) { + 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 +178,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 +219,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()) Index: clang/unittests/Format/DefinitionBlockSeparatorTest.cpp =================================================================== --- clang/unittests/Format/DefinitionBlockSeparatorTest.cpp +++ clang/unittests/Format/DefinitionBlockSeparatorTest.cpp @@ -128,6 +128,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" + " LONGTYPENAME\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 Ranges = {1, tooling::Range(0, Code.size())}; + EXPECT_EQ(reformat(Style, Code, Ranges, "").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) { @@ -172,13 +239,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 +279,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 +291,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 +305,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 +332,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 +344,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 +358,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 +386,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 +400,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 +418,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 +442,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 +454,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 +467,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"