ksyx updated this revision to Diff 396421. ksyx marked 4 inline comments as done. ksyx added a comment. Herald added a subscriber: mgorny.
- Change version from 15 to 14 - Add option `SDS_Never`, rename `SDS_Between` => `SDS_Always` - Add unit tests - Separate class into new file CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116314/new/ https://reviews.llvm.org/D116314 Files: clang/docs/ClangFormatStyleOptions.rst clang/include/clang/Format/Format.h clang/lib/Format/CMakeLists.txt clang/lib/Format/DefinitionBlockSeparator.cpp clang/lib/Format/DefinitionBlockSeparator.h clang/lib/Format/Format.cpp clang/lib/Format/WhitespaceManager.cpp clang/lib/Format/WhitespaceManager.h clang/unittests/Format/CMakeLists.txt clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
Index: clang/unittests/Format/DefinitionBlockSeparatorTest.cpp =================================================================== --- /dev/null +++ clang/unittests/Format/DefinitionBlockSeparatorTest.cpp @@ -0,0 +1,273 @@ +//===- DefinitionBlockSeparatorTest.cpp - Formatting unit tests -----------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "clang/Format/Format.h" + +#include "llvm/Support/Debug.h" +#include "gtest/gtest.h" + +#define DEBUG_TYPE "definition-block-separator-test" + +namespace clang { +namespace format { +namespace { + +class DefinitionBlockSeparatorTest : public ::testing::Test { +protected: + std::string + separateDefinitionBlocks(llvm::StringRef Code, + const std::vector<tooling::Range> &Ranges, + const FormatStyle &Style = getLLVMStyle()) { + LLVM_DEBUG(llvm::errs() << "---\n"); + LLVM_DEBUG(llvm::errs() << Code << "\n\n"); + tooling::Replacements Replaces = + clang::format::separateDefinitionBlocks(Style, Code, Ranges, "<stdin>"); + auto Result = applyAllReplacements(Code, Replaces); + EXPECT_TRUE(static_cast<bool>(Result)); + LLVM_DEBUG(llvm::errs() << "\n" << *Result << "\n\n"); + return *Result; + } + + std::string + separateDefinitionBlocks(llvm::StringRef Code, + const FormatStyle &Style = getLLVMStyle()) { + return separateDefinitionBlocks( + Code, + /*Ranges=*/{1, tooling::Range(0, Code.size())}, Style); + } +}; + +TEST_F(DefinitionBlockSeparatorTest, Basic) { + FormatStyle Style = getLLVMStyle(); + Style.SeparateDefinitionBlocks = FormatStyle::SDS_Always; + EXPECT_EQ("int foo() {\n" + " int i, j;\n" + "}\n" + "\n" + "int bar() {\n" + " int j, k;\n" + "}", + separateDefinitionBlocks("int foo() {\n" + " int i, j;\n" + "}\n" + "int bar() {\n" + " int j, k;\n" + "}", Style)); + + EXPECT_EQ("struct foo {\n" + " int i, j;\n" + "};\n" + "\n" + "struct bar {\n" + " int j, k;\n" + "};", + separateDefinitionBlocks("struct foo {\n" + " int i, j;\n" + "};\n" + "struct bar {\n" + " int j, k;\n" + "};", Style)); + + EXPECT_EQ("class foo {\n" + " int i, j;\n" + "};\n" + "\n" + "class bar {\n" + " int j, k;\n" + "};", + separateDefinitionBlocks("class foo {\n" + " int i, j;\n" + "};\n" + "class bar {\n" + " int j, k;\n" + "};", Style)); + + EXPECT_EQ("namespace foo {\n" + " int i, j;\n" + "}\n" + "\n" + "namespace bar {\n" + " int j, k;\n" + "}", + separateDefinitionBlocks("namespace foo {\n" + " int i, j;\n" + "}\n" + "namespace bar {\n" + " int j, k;\n" + "}", Style)); + + EXPECT_EQ("enum Foo {\n" + " FOO, BAR\n" + "};\n" + "\n" + "enum Bar {\n" + " FOOBAR, BARFOO\n" + "};", + separateDefinitionBlocks("enum Foo {\n" + " FOO, BAR\n" + "};\n" + "enum Bar {\n" + " FOOBAR, BARFOO\n" + "};", Style)); +} + +TEST_F(DefinitionBlockSeparatorTest, Always) { + FormatStyle Style = getLLVMStyle(); + Style.SeparateDefinitionBlocks = FormatStyle::SDS_Always; + EXPECT_EQ("enum Foo {\n" + " FOO, BAR\n" + "};\n" + "\n" + "int foo() {\n" + " int i, j;\n" + "}\n" + "\n" + "int i, j, k;\n" + "\n" + "int bar() {\n" + " int j, k;\n" + "}\n" + "\n" + "enum Bar {\n" + " FOOBAR, BARFOO\n" + "};", + separateDefinitionBlocks("enum Foo {\n" + " FOO, BAR\n" + "};\n" + "int foo() {\n" + " int i, j;\n" + "}\n" + "int i, j, k;\n" + "int bar() {\n" + " int j, k;\n" + "}\n" + "enum Bar {\n" + " FOOBAR, BARFOO\n" + "};", Style)); +} + +TEST_F(DefinitionBlockSeparatorTest, Never) { + FormatStyle Style = getLLVMStyle(); + Style.SeparateDefinitionBlocks = FormatStyle::SDS_Never; + EXPECT_EQ("enum Foo {\n" + " FOO, BAR\n" + "};\n" + "int foo() {\n" + " int i, j;\n" + "}\n" + "int i, j, k;\n" + "int bar() {\n" + " int j, k;\n" + "}\n" + "enum Bar {\n" + " FOOBAR, BARFOO\n" + "};", + separateDefinitionBlocks("enum Foo {\n" + " FOO, BAR\n" + "};\n" + "\n" + "int foo() {\n" + " int i, j;\n" + "}\n" + "\n" + "int i, j, k;\n" + "\n" + "int bar() {\n" + " int j, k;\n" + "}\n" + "\n" + "enum Bar {\n" + " FOOBAR, BARFOO\n" + "};", + Style)); +} + +TEST_F(DefinitionBlockSeparatorTest, OpeningBracketOwnsLine) { + FormatStyle Style = getLLVMStyle(); + Style.SeparateDefinitionBlocks = FormatStyle::SDS_Always; + EXPECT_EQ("enum Foo\n" + "{\n" + " FOO, BAR\n" + "};\n" + "\n" + "int foo()\n" + "{\n" + " int i, j;\n" + "}\n" + "\n" + "int i, j, k;\n" + "\n" + "int bar()\n" + "{\n" + " int j, k;\n" + "}\n" + "\n" + "enum Bar\n" + "{\n" + " FOOBAR, BARFOO\n" + "};", + separateDefinitionBlocks("enum Foo\n" + "{\n" + " FOO, BAR\n" + "};\n" + "int foo()\n" + "{\n" + " int i, j;\n" + "}\n" + "int i, j, k;\n" + "int bar()\n" + "{\n" + " int j, k;\n" + "}\n" + "enum Bar\n" + "{\n" + " FOOBAR, BARFOO\n" + "};", Style)); +} + +TEST_F(DefinitionBlockSeparatorTest, Leave) { + FormatStyle Style = getLLVMStyle(); + Style.SeparateDefinitionBlocks = FormatStyle::SDS_Leave; + EXPECT_EQ("enum Foo\n" + "{\n" + " FOO, BAR\n" + "};\n\n\n\n" + "int foo()\n" + "{\n" + " int i, j;\n" + "}\n" + "int i, j, k;\n" + "int bar()\n" + "{\n" + " int j, k;\n" + "}\n" + "enum Bar\n" + "{\n" + " FOOBAR, BARFOO\n" + "};", + separateDefinitionBlocks("enum Foo\n" + "{\n" + " FOO, BAR\n" + "};\n\n\n\n" + "int foo()\n" + "{\n" + " int i, j;\n" + "}\n" + "int i, j, k;\n" + "int bar()\n" + "{\n" + " int j, k;\n" + "}\n" + "enum Bar\n" + "{\n" + " FOOBAR, BARFOO\n" + "};", Style)); +} +} // end namespace +} // end namespace format +} // end namespace clang Index: clang/unittests/Format/CMakeLists.txt =================================================================== --- clang/unittests/Format/CMakeLists.txt +++ clang/unittests/Format/CMakeLists.txt @@ -4,6 +4,7 @@ add_clang_unittest(FormatTests CleanupTest.cpp + DefinitionBlockSeparatorTest.cpp FormatTest.cpp FormatTestComments.cpp FormatTestCSharp.cpp Index: clang/lib/Format/WhitespaceManager.h =================================================================== --- clang/lib/Format/WhitespaceManager.h +++ clang/lib/Format/WhitespaceManager.h @@ -45,6 +45,9 @@ bool useCRLF() const { return UseCRLF; } + /// Infers whether the input is using CRLF + static bool inputUsesCRLF(StringRef Text, bool DefaultToCRLF); + /// Replaces the whitespace in front of \p Tok. Only call once for /// each \c AnnotatedToken. /// Index: clang/lib/Format/WhitespaceManager.cpp =================================================================== --- clang/lib/Format/WhitespaceManager.cpp +++ clang/lib/Format/WhitespaceManager.cpp @@ -74,6 +74,12 @@ return Replaces.add(Replacement); } +bool WhitespaceManager::inputUsesCRLF(StringRef Text, bool DefaultToCRLF) { + size_t LF = Text.count('\n'); + size_t CR = Text.count('\r') * 2; + return LF == CR ? DefaultToCRLF : CR > LF; +} + void WhitespaceManager::replaceWhitespaceInToken( const FormatToken &Tok, unsigned Offset, unsigned ReplaceChars, StringRef PreviousPostfix, StringRef CurrentPrefix, bool InPPDirective, Index: clang/lib/Format/Format.cpp =================================================================== --- clang/lib/Format/Format.cpp +++ clang/lib/Format/Format.cpp @@ -16,6 +16,7 @@ #include "AffectedRangeManager.h" #include "BreakableToken.h" #include "ContinuationIndenter.h" +#include "DefinitionBlockSeparator.h" #include "FormatInternal.h" #include "FormatTokenLexer.h" #include "NamespaceEndCommentsFixer.h" @@ -450,6 +451,15 @@ } }; +template <> +struct ScalarEnumerationTraits<FormatStyle::SeparateDefinitionStyle> { + static void enumeration(IO &IO, FormatStyle::SeparateDefinitionStyle &Value) { + IO.enumCase(Value, "Leave", FormatStyle::SDS_Leave); + IO.enumCase(Value, "Always", FormatStyle::SDS_Always); + IO.enumCase(Value, "Never", FormatStyle::SDS_Never); + } +}; + template <> struct ScalarEnumerationTraits<FormatStyle::SpaceBeforeParensStyle> { static void enumeration(IO &IO, FormatStyle::SpaceBeforeParensStyle &Value) { @@ -770,6 +780,7 @@ IO.mapOptional("ReferenceAlignment", Style.ReferenceAlignment); IO.mapOptional("ReflowComments", Style.ReflowComments); IO.mapOptional("ShortNamespaceLines", Style.ShortNamespaceLines); + IO.mapOptional("SeparateDefinitionBlocks", Style.SeparateDefinitionBlocks); IO.mapOptional("SortIncludes", Style.SortIncludes); IO.mapOptional("SortJavaStaticImport", Style.SortJavaStaticImport); IO.mapOptional("SortUsingDeclarations", Style.SortUsingDeclarations); @@ -1193,6 +1204,7 @@ LLVMStyle.ObjCSpaceBeforeProtocolList = true; LLVMStyle.PointerAlignment = FormatStyle::PAS_Right; LLVMStyle.ReferenceAlignment = FormatStyle::RAS_Pointer; + LLVMStyle.SeparateDefinitionBlocks = FormatStyle::SDS_Leave; LLVMStyle.ShortNamespaceLines = 1; LLVMStyle.SpacesBeforeTrailingComments = 1; LLVMStyle.Standard = FormatStyle::LS_Latest; @@ -1840,7 +1852,7 @@ WhitespaceManager Whitespaces( Env.getSourceManager(), Style, Style.DeriveLineEnding - ? inputUsesCRLF( + ? WhitespaceManager::inputUsesCRLF( Env.getSourceManager().getBufferData(Env.getFileID()), Style.UseCRLF) : Style.UseCRLF); @@ -1864,12 +1876,6 @@ } private: - static bool inputUsesCRLF(StringRef Text, bool DefaultToCRLF) { - size_t LF = Text.count('\n'); - size_t CR = Text.count('\r') * 2; - return LF == CR ? DefaultToCRLF : CR > LF; - } - bool hasCpp03IncompatibleFormat(const SmallVectorImpl<AnnotatedLine *> &Lines) { for (const AnnotatedLine *Line : Lines) { @@ -3048,6 +3054,11 @@ Passes.emplace_back([&](const Environment &Env) { return UsingDeclarationsSorter(Env, Expanded).process(); }); + + if (Style.SeparateDefinitionBlocks) + Passes.emplace_back([&](const Environment &Env) { + return DefinitionBlockSeparator(Env, Expanded).process(); + }); } if (Style.isJavaScript() && Style.JavaScriptQuotes != FormatStyle::JSQS_Leave) @@ -3138,6 +3149,16 @@ return NamespaceEndCommentsFixer(*Env, Style).process().first; } +tooling::Replacements separateDefinitionBlocks(const FormatStyle &Style, + StringRef Code, + ArrayRef<tooling::Range> Ranges, + StringRef FileName) { + auto Env = Environment::make(Code, FileName, Ranges); + if (!Env) + return {}; + return DefinitionBlockSeparator(*Env, Style).process().first; +} + tooling::Replacements sortUsingDeclarations(const FormatStyle &Style, StringRef Code, ArrayRef<tooling::Range> Ranges, Index: clang/lib/Format/DefinitionBlockSeparator.h =================================================================== --- /dev/null +++ clang/lib/Format/DefinitionBlockSeparator.h @@ -0,0 +1,41 @@ +//===--- DefinitionBlockSeparator.h -----------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +/// +/// \file +/// This file declares DefinitionBlockSeparator, a TokenAnalyzer that separates +/// definition blocks like classes, structs, functions, and namespaces by +/// inserting an empty line in between for C++ language. +/// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_LIB_FORMAT_DEFINITIONBLOCKSEPARATOR_H +#define LLVM_CLANG_LIB_FORMAT_DEFINITIONBLOCKSEPARATOR_H + +#include "TokenAnalyzer.h" +#include "WhitespaceManager.h" + +namespace clang { +namespace format { +class DefinitionBlockSeparator : public TokenAnalyzer { +public: + DefinitionBlockSeparator(const Environment &Env, const FormatStyle &Style) + : TokenAnalyzer(Env, Style) {} + + std::pair<tooling::Replacements, unsigned> + analyze(TokenAnnotator &Annotator, + SmallVectorImpl<AnnotatedLine *> &AnnotatedLines, + FormatTokenLexer &Tokens) override; + +private: + void separateBlocks(SmallVectorImpl<AnnotatedLine *> &Lines, + tooling::Replacements &Result); +}; +} // end namespace format +} // end namespace clang + +#endif Index: clang/lib/Format/DefinitionBlockSeparator.cpp =================================================================== --- /dev/null +++ clang/lib/Format/DefinitionBlockSeparator.cpp @@ -0,0 +1,114 @@ +//===--- DefinitionBlockSeparator.cpp ---------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +/// +/// \file +/// This file implements DefinitionBlockSeparator, a TokenAnalyzer that +/// separates definition blocks like classes, structs, functions, and namespaces +/// by inserting an empty line in between for C++ language. +/// +//===----------------------------------------------------------------------===// + +#include "DefinitionBlockSeparator.h" +#include "llvm/Support/Debug.h" +#define DEBUG_TYPE "definition-block-separator" + +namespace clang { +namespace format { + std::pair<tooling::Replacements, unsigned> + DefinitionBlockSeparator::analyze(TokenAnnotator &Annotator, + SmallVectorImpl<AnnotatedLine *> &AnnotatedLines, + FormatTokenLexer &Tokens) { + AffectedRangeMgr.computeAffectedLines(AnnotatedLines); + tooling::Replacements Result; + separateBlocks(AnnotatedLines, Result); + return {Result, 0}; + } + + void + DefinitionBlockSeparator::separateBlocks(SmallVectorImpl<AnnotatedLine *> &Lines, + tooling::Replacements &Result) { + if (Style.SeparateDefinitionBlocks == FormatStyle::SDS_Leave) return; + auto likelyDefinition = [](AnnotatedLine *Line) { + return (Line->MightBeFunctionDecl && Line->mightBeFunctionDefinition()) || + Line->First->isOneOf(tok::kw_class, tok::kw_struct, + tok::kw_namespace, tok::kw_enum); + }; + unsigned NewlineCount = + (Style.SeparateDefinitionBlocks == FormatStyle::SDS_Always ? 1 : 0) + 1; + WhitespaceManager Whitespaces( + Env.getSourceManager(), Style, + Style.DeriveLineEnding + ? WhitespaceManager::inputUsesCRLF( + Env.getSourceManager().getBufferData(Env.getFileID()), + Style.UseCRLF) + : Style.UseCRLF); + for (unsigned I = 0; I < Lines.size(); I++) { + auto Line = Lines[I]; + FormatToken *TargetToken = nullptr; + auto OpeningLineIndex = Line->MatchingOpeningBlockLineIndex; + auto insertReplacement = [&]() { + // Do not handle EOF newlines + assert(TargetToken); + if (TargetToken->is(tok::eof)) return; + Whitespaces.replaceWhitespace(*TargetToken, NewlineCount, + TargetToken->SpacesRequiredBefore - 1, + TargetToken->StartsColumn); + }; + auto followingOtherOpening = [&]() { + return OpeningLineIndex == 0 || + Lines[OpeningLineIndex - 1]->Last->opensScope(); + }; + + if (Line->First->closesScope()) { + // Case: Opening bracket has its own line + if (OpeningLineIndex > 0 && + Lines[OpeningLineIndex]->First->TokenText == "{") { + OpeningLineIndex--; + } + AnnotatedLine *OpeningLine = Lines[OpeningLineIndex]; + // Closing a function definition + if (likelyDefinition(OpeningLine)) { + AnnotatedLine *TargetLine; + // Not the first token + if (!followingOtherOpening()) { + TargetLine = OpeningLine; + TargetToken = TargetLine->First; + + // Avoid duplicated replacement + if (TargetToken && !TargetToken->opensScope()) + insertReplacement(); + } + + // Not the last token + if (I + 1 < Lines.size()) { + TargetLine = Lines[I + 1]; + TargetToken = TargetLine->First; + + // Not continuously closing scopes (e.g. function + class + + // namespace); The token will be handled in another case if + // it is a definition line + if (TargetLine->Affected && !TargetToken->closesScope() && + !likelyDefinition(TargetLine)) { + insertReplacement(); + } + } + } + } + else if (Line->First->is(tok::kw_enum)) { + OpeningLineIndex = I; + TargetToken = Line->First; + if (!followingOtherOpening()) + insertReplacement(); + } + } + for (const auto &R : Whitespaces.generateReplacements()) + if (Result.add(R)) + return; + } +} // namespace format +} // namespace clang Index: clang/lib/Format/CMakeLists.txt =================================================================== --- clang/lib/Format/CMakeLists.txt +++ clang/lib/Format/CMakeLists.txt @@ -4,6 +4,7 @@ AffectedRangeManager.cpp BreakableToken.cpp ContinuationIndenter.cpp + DefinitionBlockSeparator.cpp Format.cpp FormatToken.cpp FormatTokenLexer.cpp Index: clang/include/clang/Format/Format.h =================================================================== --- clang/include/clang/Format/Format.h +++ clang/include/clang/Format/Format.h @@ -3050,6 +3050,54 @@ bool ReflowComments; // clang-format on + enum SeparateDefinitionStyle { + /// Leave definition blocks separated as they are without changing anything + SDS_Leave, + /// Insert empty lines between definition blocks + SDS_Always, + /// Remove any empty line between definition blocks + SDS_Never + }; + + /// Specifies the use of empty lines to separate definition blocks, including + /// classes, structs, enum, and functions, for C++ language. + /// \code + /// SeparateDefinitions + /// Never v.s. Always + /// #include <cstring> #include <cstring> + /// struct Foo{ + /// int a,b,c; struct Foo { + /// }; int a, b, c; + /// namespace Ns { }; + /// class Bar { + /// public: namespace Ns { + /// struct Foobar { class Bar { + /// int a; public: + /// int b; struct Foobar { + /// }; int a; + /// private: int b; + /// int t; }; + /// int method1() { + /// // ... private: + /// } int t; + /// template<typename T> + /// int method2(T x) { int method1() { + /// // ... // ... + /// } } + /// int method3(int par) {} + /// }; template <typename T> int method2(T x) { + /// class C { // ... + /// }; } + /// } + /// int method3(int par) {} + /// }; + /// + /// class C {}; + /// } // namespace Ns + /// \endcode + /// \version 14 + SeparateDefinitionStyle SeparateDefinitionBlocks; + /// The maximal number of unwrapped lines that a short namespace spans. /// Defaults to 1. /// @@ -4028,6 +4076,16 @@ ArrayRef<tooling::Range> Ranges, StringRef FileName = "<stdin>"); +/// Use empty line to separate definition blocks including classes, structs, +/// functions, namespaces, and enum in the given \p Ranges in \p Code. +/// +/// Returns the ``Replacements`` that inserts empty lines to separate definition +/// blocks in all \p Ranges in \p Code. +tooling::Replacements separateDefinitionBlocks(const FormatStyle &Style, + StringRef Code, + ArrayRef<tooling::Range> Ranges, + StringRef FileName = "<stdin>"); + /// Sort consecutive using declarations in the given \p Ranges in /// \p Code. /// Index: clang/docs/ClangFormatStyleOptions.rst =================================================================== --- clang/docs/ClangFormatStyleOptions.rst +++ clang/docs/ClangFormatStyleOptions.rst @@ -3395,6 +3395,56 @@ /* second veryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongComment with plenty of * information */ +**SeparateDefinitionBlocks** (``SeparateDefinitionStyle``) :versionbadge:`clang-format 14` + Specifies the use of empty lines to separate definition blocks, including classes, + structs, enum, and functions, for C++ language. + + Possible values: + + * ``SDS_Leave`` (in configuration: ``Leave``) + Leave definition blocks separated as they are, without changing anything. + + * ``SDS_Always`` (in configuration: ``Always``) + Insert empty lines between definition blocks. + + * ``SDS_Never`` (in configuration: ``Never``) + Remove any empty line between definition blocks. + + .. code-block:: c++ + + SeparateDefinitions + Never v.s. Always + #include <cstring> #include <cstring> + struct Foo{ + int a,b,c; struct Foo { + }; int a, b, c; + namespace Ns { }; + class Bar { + public: namespace Ns { + struct Foobar { class Bar { + int a; public: + int b; struct Foobar { + }; int a; + private: int b; + int t; }; + int method1() { + // ... private: + } int t; + template<typename T> + int method2(T x) { int method1() { + // ... // ... + } } + int method3(int par) {} + }; template <typename T> int method2(T x) { + class C { // ... + }; } + } + int method3(int par) {} + }; + + class C {}; + } // namespace Ns + **ShortNamespaceLines** (``Unsigned``) :versionbadge:`clang-format 14` The maximal number of unwrapped lines that a short namespace spans. Defaults to 1.
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
