From 60bf5826cfd3629b5200a8ab743d701c90f66af0 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Albertas=20Vy=C5=A1niauskas?= Date: Mon, 25 Jan 2021 20:47:22 +0100 Subject: [PATCH] [clang-format] PR16518 Add flag to suppress empty line insertion before access modifier Add new option called InsertEmptyLineBeforeAccessModifier. Empty line before access modifier is inerted if this option is set to true (which is the default value, because clang-format always inserts empty lines before access modifiers), otherwise empty lines are removed. Fixes issue #16518. Differential Revision: https://reviews.llvm.org/D93846 --- clang/docs/ClangFormatStyleOptions.rst | 69 +++++++ clang/include/clang/Format/Format.h | 63 ++++++ clang/lib/Format/Format.cpp | 15 ++ clang/lib/Format/UnwrappedLineFormatter.cpp | 31 ++- clang/test/Format/access-modifiers.cpp | 63 ++++++ clang/unittests/Format/FormatTest.cpp | 302 ++++++++++++++++++++++++++++ 6 files changed, 539 insertions(+), 4 deletions(-) create mode 100644 clang/test/Format/access-modifiers.cpp diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst index d5ce1b7..f6ff571 100644 --- a/clang/docs/ClangFormatStyleOptions.rst +++ b/clang/docs/ClangFormatStyleOptions.rst @@ -2119,6 +2119,75 @@ the configuration (without a prefix: ``Auto``). **DisableFormat** (``bool``) Disables formatting completely. +**EmptyLineBeforeAccessModifier** (``EmptyLineBeforeAccessModifierStyle``) + Defines in which cases to put empty line before access modifiers. + + Possible values: + + * ``ELBAMS_Never`` (in configuration: ``Never``) + Remove all empty lines before access modifiers. + + .. code-block:: c++ + + struct foo { + private: + int i; + protected: + int j; + /* comment */ + public: + foo() {} + private: + protected: + }; + + * ``ELBAMS_Leave`` (in configuration: ``Leave``) + Keep existing empty lines before access modifiers. + + * ``ELBAMS_LogicalBlock`` (in configuration: ``LogicalBlock``) + Add empty line only when access modifier starts a new logical block. + Logical block is a group of one or more member fields or functions. + + .. code-block:: c++ + + struct foo { + private: + int i; + + protected: + int j; + /* comment */ + public: + foo() {} + + private: + protected: + }; + + * ``ELBAMS_Always`` (in configuration: ``Always``) + Always add empty line before access modifiers unless access modifier + is at the start of struct or class definition. + + .. code-block:: c++ + + struct foo { + private: + int i; + + protected: + int j; + /* comment */ + + public: + foo() {} + + private: + + protected: + }; + + + **ExperimentalAutoDetectBinPacking** (``bool``) If ``true``, clang-format detects whether function calls and definitions are formatted with one parameter per line. diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h index fcc38e25..96c2a74 100644 --- a/clang/include/clang/Format/Format.h +++ b/clang/include/clang/Format/Format.h @@ -1885,6 +1885,68 @@ struct FormatStyle { /// Disables formatting completely. bool DisableFormat; + /// Different styles for empty line before access modifiers. + enum EmptyLineBeforeAccessModifierStyle : unsigned char { + /// Remove all empty lines before access modifiers. + /// \code + /// struct foo { + /// private: + /// int i; + /// protected: + /// int j; + /// /* comment */ + /// public: + /// foo() {} + /// private: + /// protected: + /// }; + /// \endcode + ELBAMS_Never, + /// Keep existing empty lines before access modifiers. + ELBAMS_Leave, + /// Add empty line only when access modifier starts a new logical block. + /// Logical block is a group of one or more member fields or functions. + /// \code + /// struct foo { + /// private: + /// int i; + /// + /// protected: + /// int j; + /// /* comment */ + /// public: + /// foo() {} + /// + /// private: + /// protected: + /// }; + /// \endcode + ELBAMS_LogicalBlock, + /// Always add empty line before access modifiers unless access modifier + /// is at the start of struct or class definition. + /// \code + /// struct foo { + /// private: + /// int i; + /// + /// protected: + /// int j; + /// /* comment */ + /// + /// public: + /// foo() {} + /// + /// private: + /// + /// protected: + /// }; + /// \endcode + ELBAMS_Always, + }; + + /// Defines in which cases to put empty line before access modifiers. + EmptyLineBeforeAccessModifierStyle EmptyLineBeforeAccessModifier; + /// If ``true``, clang-format detects whether function calls and /// definitions are formatted with one parameter per line. /// @@ -3015,6 +3077,7 @@ struct FormatStyle { DeriveLineEnding == R.DeriveLineEnding && DerivePointerAlignment == R.DerivePointerAlignment && DisableFormat == R.DisableFormat && + EmptyLineBeforeAccessModifier == R.EmptyLineBeforeAccessModifier && ExperimentalAutoDetectBinPacking == R.ExperimentalAutoDetectBinPacking && FixNamespaceComments == R.FixNamespaceComments && diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index 0986ef8..cb41019 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -242,6 +242,18 @@ struct ScalarEnumerationTraits { }; template <> +struct ScalarEnumerationTraits< + FormatStyle::EmptyLineBeforeAccessModifierStyle> { + static void + enumeration(IO &IO, FormatStyle::EmptyLineBeforeAccessModifierStyle &Value) { + IO.enumCase(Value, "Never", FormatStyle::ELBAMS_Never); + IO.enumCase(Value, "Leave", FormatStyle::ELBAMS_Leave); + IO.enumCase(Value, "LogicalBlock", FormatStyle::ELBAMS_LogicalBlock); + IO.enumCase(Value, "Always", FormatStyle::ELBAMS_Always); + } +}; + +template <> struct ScalarEnumerationTraits { static void enumeration(IO &IO, FormatStyle::PPDirectiveIndentStyle &Value) { IO.enumCase(Value, "None", FormatStyle::PPDIS_None); @@ -560,6 +572,8 @@ template <> struct MappingTraits { IO.mapOptional("DeriveLineEnding", Style.DeriveLineEnding); IO.mapOptional("DerivePointerAlignment", Style.DerivePointerAlignment); IO.mapOptional("DisableFormat", Style.DisableFormat); + IO.mapOptional("EmptyLineBeforeAccessModifier", + Style.EmptyLineBeforeAccessModifier); IO.mapOptional("ExperimentalAutoDetectBinPacking", Style.ExperimentalAutoDetectBinPacking); IO.mapOptional("FixNamespaceComments", Style.FixNamespaceComments); @@ -931,6 +945,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) { LLVMStyle.Cpp11BracedListStyle = true; LLVMStyle.DeriveLineEnding = true; LLVMStyle.DerivePointerAlignment = false; + LLVMStyle.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_LogicalBlock; LLVMStyle.ExperimentalAutoDetectBinPacking = false; LLVMStyle.FixNamespaceComments = true; LLVMStyle.ForEachMacros.push_back("foreach"); diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp index 13c5fdc..d1138bb 100644 --- a/clang/lib/Format/UnwrappedLineFormatter.cpp +++ b/clang/lib/Format/UnwrappedLineFormatter.cpp @@ -1245,10 +1245,33 @@ void UnwrappedLineFormatter::formatFirstToken( !startsExternCBlock(*PreviousLine)) Newlines = 1; - // Insert extra new line before access specifiers. - if (PreviousLine && PreviousLine->Last->isOneOf(tok::semi, tok::r_brace) && - RootToken.isAccessSpecifier() && RootToken.NewlinesBefore == 1) - ++Newlines; + // Insert or remove empty line before access specifiers. + if (PreviousLine && RootToken.isAccessSpecifier()) { + switch (Style.EmptyLineBeforeAccessModifier) { + case FormatStyle::ELBAMS_Never: + if (RootToken.NewlinesBefore > 1) + Newlines = 1; + break; + case FormatStyle::ELBAMS_Leave: + Newlines = std::max(RootToken.NewlinesBefore, 1u); + break; + case FormatStyle::ELBAMS_LogicalBlock: + if (PreviousLine->Last->isOneOf(tok::semi, tok::r_brace) && + RootToken.NewlinesBefore <= 1) + Newlines = 2; + break; + case FormatStyle::ELBAMS_Always: { + const FormatToken *previousToken; + if (PreviousLine->Last->is(tok::comment)) + previousToken = PreviousLine->Last->getPreviousNonComment(); + else + previousToken = PreviousLine->Last; + if ((!previousToken || !previousToken->is(tok::l_brace)) && + RootToken.NewlinesBefore <= 1) + Newlines = 2; + } break; + } + } // Remove empty lines after access specifiers. if (PreviousLine && PreviousLine->First->isAccessSpecifier() && diff --git a/clang/test/Format/access-modifiers.cpp b/clang/test/Format/access-modifiers.cpp new file mode 100644 index 0000000..8bd9840 --- /dev/null +++ b/clang/test/Format/access-modifiers.cpp @@ -0,0 +1,63 @@ +// RUN: grep -Ev "// *[A-Z-]+:" %s \ +// RUN: | clang-format -style="{BasedOnStyle: LLVM, EmptyLineBeforeAccessModifier: LogicalBlock}" -lines=1:14 \ +// RUN: | clang-format -style="{BasedOnStyle: LLVM, EmptyLineBeforeAccessModifier: Never}" -lines=14:40 \ +// RUN: | FileCheck -strict-whitespace %s + +// CHECK: int i +// CHECK-NEXT: {{^$}} +// CHECK-NEXT: {{^private:$}} +// CHECK: } +struct foo1 { + int i; + +private: + int j; +}; + +// CHECK: struct bar1 +// CHECK-NEXT: {{^private:$}} +// CHECK: } +struct bar1 { +private: + int i; + int j; +}; + +// CHECK: int i +// CHECK-NEXT: {{^private:$}} +// CHECK: } +struct foo2 { + int i; + +private: + int j; +}; + +// CHECK: struct bar2 +// CHECK-NEXT: {{^private:$}} +// CHECK: } +struct bar2 { +private: + int i; + int j; +}; + +// CHECK: int j +// CHECK-NEXT: {{^private:$}} +// CHECK: } +struct foo3 { + int i; + int j; + +private: +}; + +// CHECK: struct bar3 +// CHECK-NEXT: {{^private:$}} +// CHECK: } +struct bar3 { + +private: + int i; + int j; +}; diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index cbb368d..c7dd203 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -8887,6 +8887,298 @@ TEST_F(FormatTest, BreaksLongDeclarations) { getLLVMStyle()); } +TEST_F(FormatTest, FormatsAccessModifiers) { + FormatStyle Style = getLLVMStyle(); + EXPECT_EQ(Style.EmptyLineBeforeAccessModifier, + FormatStyle::ELBAMS_LogicalBlock); + verifyFormat("struct foo {\n" + "private:\n" + " void f() {}\n" + "\n" + "private:\n" + " int i;\n" + "\n" + "protected:\n" + " int j;\n" + "};\n", + Style); + verifyFormat("struct foo {\n" + "private:\n" + " void f() {}\n" + "\n" + "private:\n" + " int i;\n" + "\n" + "protected:\n" + " int j;\n" + "};\n", + "struct foo {\n" + "private:\n" + " void f() {}\n" + "private:\n" + " int i;\n" + "protected:\n" + " int j;\n" + "};\n", + Style); + verifyFormat("struct foo { /* comment */\n" + "private:\n" + " int i;\n" + " // comment\n" + "private:\n" + " int j;\n" + "};\n", + Style); + verifyFormat("struct foo {\n" + "#ifdef FOO\n" + "#endif\n" + "private:\n" + " int i;\n" + "#ifdef FOO\n" + "private:\n" + "#endif\n" + " int j;\n" + "};\n", + Style); + Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_Never; + verifyFormat("struct foo {\n" + "private:\n" + " void f() {}\n" + "private:\n" + " int i;\n" + "protected:\n" + " int j;\n" + "};\n", + Style); + verifyFormat("struct foo {\n" + "private:\n" + " void f() {}\n" + "private:\n" + " int i;\n" + "protected:\n" + " int j;\n" + "};\n", + "struct foo {\n" + "\n" + "private:\n" + " void f() {}\n" + "\n" + "private:\n" + " int i;\n" + "\n" + "protected:\n" + " int j;\n" + "};\n", + Style); + verifyFormat("struct foo { /* comment */\n" + "private:\n" + " int i;\n" + " // comment\n" + "private:\n" + " int j;\n" + "};\n", + "struct foo { /* comment */\n" + "\n" + "private:\n" + " int i;\n" + " // comment\n" + "\n" + "private:\n" + " int j;\n" + "};\n", + Style); + verifyFormat("struct foo {\n" + "#ifdef FOO\n" + "#endif\n" + "private:\n" + " int i;\n" + "#ifdef FOO\n" + "private:\n" + "#endif\n" + " int j;\n" + "};\n", + "struct foo {\n" + "#ifdef FOO\n" + "#endif\n" + "\n" + "private:\n" + " int i;\n" + "#ifdef FOO\n" + "\n" + "private:\n" + "#endif\n" + " int j;\n" + "};\n", + Style); + Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_Always; + verifyFormat("struct foo {\n" + "private:\n" + " void f() {}\n" + "\n" + "private:\n" + " int i;\n" + "\n" + "protected:\n" + " int j;\n" + "};\n", + Style); + verifyFormat("struct foo {\n" + "private:\n" + " void f() {}\n" + "\n" + "private:\n" + " int i;\n" + "\n" + "protected:\n" + " int j;\n" + "};\n", + "struct foo {\n" + "private:\n" + " void f() {}\n" + "private:\n" + " int i;\n" + "protected:\n" + " int j;\n" + "};\n", + Style); + verifyFormat("struct foo { /* comment */\n" + "private:\n" + " int i;\n" + " // comment\n" + "\n" + "private:\n" + " int j;\n" + "};\n", + "struct foo { /* comment */\n" + "private:\n" + " int i;\n" + " // comment\n" + "\n" + "private:\n" + " int j;\n" + "};\n", + Style); + verifyFormat("struct foo {\n" + "#ifdef FOO\n" + "#endif\n" + "\n" + "private:\n" + " int i;\n" + "#ifdef FOO\n" + "\n" + "private:\n" + "#endif\n" + " int j;\n" + "};\n", + "struct foo {\n" + "#ifdef FOO\n" + "#endif\n" + "private:\n" + " int i;\n" + "#ifdef FOO\n" + "private:\n" + "#endif\n" + " int j;\n" + "};\n", + Style); + Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_Leave; + EXPECT_EQ("struct foo {\n" + "\n" + "private:\n" + " void f() {}\n" + "\n" + "private:\n" + " int i;\n" + "\n" + "protected:\n" + " int j;\n" + "};\n", + format("struct foo {\n" + "\n" + "private:\n" + " void f() {}\n" + "\n" + "private:\n" + " int i;\n" + "\n" + "protected:\n" + " int j;\n" + "};\n", + Style)); + verifyFormat("struct foo {\n" + "private:\n" + " void f() {}\n" + "private:\n" + " int i;\n" + "protected:\n" + " int j;\n" + "};\n", + Style); + EXPECT_EQ("struct foo { /* comment */\n" + "\n" + "private:\n" + " int i;\n" + " // comment\n" + "\n" + "private:\n" + " int j;\n" + "};\n", + format("struct foo { /* comment */\n" + "\n" + "private:\n" + " int i;\n" + " // comment\n" + "\n" + "private:\n" + " int j;\n" + "};\n", + Style)); + verifyFormat("struct foo { /* comment */\n" + "private:\n" + " int i;\n" + " // comment\n" + "private:\n" + " int j;\n" + "};\n", + Style); + EXPECT_EQ("struct foo {\n" + "#ifdef FOO\n" + "#endif\n" + "\n" + "private:\n" + " int i;\n" + "#ifdef FOO\n" + "\n" + "private:\n" + "#endif\n" + " int j;\n" + "};\n", + format("struct foo {\n" + "#ifdef FOO\n" + "#endif\n" + "\n" + "private:\n" + " int i;\n" + "#ifdef FOO\n" + "\n" + "private:\n" + "#endif\n" + " int j;\n" + "};\n", + Style)); + verifyFormat("struct foo {\n" + "#ifdef FOO\n" + "#endif\n" + "private:\n" + " int i;\n" + "#ifdef FOO\n" + "private:\n" + "#endif\n" + " int j;\n" + "};\n", + Style); +} + TEST_F(FormatTest, FormatsArrays) { verifyFormat("aaaaaaaaaaaaaaaaaaaaaaaaa[aaaaaaaaaaaaaaaaaaaaaaaaa]\n" " [bbbbbbbbbbbbbbbbbbbbbbbbb] = c;"); @@ -15352,6 +15644,16 @@ TEST_F(FormatTest, ParsesConfiguration) { CHECK_PARSE("BreakBeforeInheritanceComma: true", BreakInheritanceList, FormatStyle::BILS_BeforeComma); + Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_LogicalBlock; + CHECK_PARSE("EmptyLineBeforeAccessModifier: Never", + EmptyLineBeforeAccessModifier, FormatStyle::ELBAMS_Never); + CHECK_PARSE("EmptyLineBeforeAccessModifier: Leave", + EmptyLineBeforeAccessModifier, FormatStyle::ELBAMS_Leave); + CHECK_PARSE("EmptyLineBeforeAccessModifier: LogicalBlock", + EmptyLineBeforeAccessModifier, FormatStyle::ELBAMS_LogicalBlock); + CHECK_PARSE("EmptyLineBeforeAccessModifier: Always", + EmptyLineBeforeAccessModifier, FormatStyle::ELBAMS_Always); + Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak; CHECK_PARSE("AlignAfterOpenBracket: Align", AlignAfterOpenBracket, FormatStyle::BAS_Align); -- 2.7.4