Max_S updated this revision to Diff 331818. Max_S added a comment. I added more tests for the interaction between MaxEmptyLinesToKeep, EmptyLineBeforeAccessModifier and EmptyLineAfterAccessModifier. During these tests I recognized, that EmptyLineBeforeAccessModifier = Always,Leave will (should adhere) to MaxEmptyLinesToKeep. So the option does not force the new line it only applies it if it is missing. I changed the logic for EmptyLineAfterAccessModifier accordingly.
Four of the new tests fail, two of them are probably bugs in the implementation of EmptyLineBeforeAccessModifier. The test in line 9354 is set to: Style.MaxEmptyLinesToKeep = 0u; // Both force one new line Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_Always; Style.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Always; EXPECT_EQ(NL_B_1_A_1_I_1, format(NL_B_3_A_3_I_3, Style)); So both options should force here the newline, that are removed by MaxEmptyLinesToKeep. EmptyLineBeforeAccessModifier removes all lines, so it seems that MaxEmptyLinesToKeep has the higher rank here. Is that correct behavior? The test in line 9368 is set to: Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_Leave; Style.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Leave; Style.MaxEmptyLinesToKeep = 1u; EXPECT_EQ(NL_B_1_A_1_I_1, format(NL_B_3_A_3_I_3, Style)); So both options should leave the new lines in place but should adhere to MaxEmptyLinesToKeep. EmptyLineBeforeAccessModifier keeps all three lines, so it seems that MaxEmptyLinesToKeep has the lower rank here. Is that correct behavior? The tests in line 9462 and 9466 require some additional implementation. EmptyLineAfterAccessModifier is adding the three lines since it is configured to do so, but EmptyLineBeforeAccessModifier would remove these lines. Since EmptyLineBeforeAccessModifier can only access the old file and not the new one. It does not know that three lines have been written and probably could not remove them. I would like to implement it in such a way, that EmptyLineAfterAccessModifier looks ahead and skips its logic if the next token is a also an access modifier such that the logic of EmptyLineBeforeAccessModifier takes precedence. I could not find a way to get the next line. Is this possible somehow? Does there exist some helper functionality? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98237/new/ https://reviews.llvm.org/D98237 Files: clang/docs/ClangFormatStyleOptions.rst clang/docs/ReleaseNotes.rst clang/include/clang/Format/Format.h clang/lib/Format/Format.cpp clang/lib/Format/UnwrappedLineFormatter.cpp clang/unittests/Format/FormatTest.cpp
Index: clang/unittests/Format/FormatTest.cpp =================================================================== --- clang/unittests/Format/FormatTest.cpp +++ clang/unittests/Format/FormatTest.cpp @@ -9179,6 +9179,297 @@ Style); } +TEST_F(FormatTest, FormatsAfterAccessModifiers) { + char const *const NL_After_Never = "struct foo {\n" + "private:\n" + " void f() {}\n" + "\n" + "private:\n" + " int i;\n" + "\n" + "protected:\n" + " int j;\n" + "};\n"; + char const *const NL_After_Always = "struct foo {\n" + "private:\n" + "\n" + " void f() {}\n" + "\n" + "private:\n" + "\n" + " int i;\n" + "\n" + "protected:\n" + "\n" + " int j;\n" + "};\n"; + + char const *const NL_After_Always3Times = "struct foo {\n" + "private:\n" + "\n\n\n" + " void f() {}\n" + "\n" + "private:\n" + "\n\n\n" + " int i;\n" + "\n" + "protected:\n" + "\n\n\n" + " int j;\n" + "};\n"; + + char const *const NL_After_Never_Before_Never = "struct foo {\n" + "private:\n" + " void f() {}\n" + "private:\n" + " int i;\n" + "protected:\n" + " int j;\n" + "};\n"; + FormatStyle Style = getLLVMStyle(); + EXPECT_EQ(Style.EmptyLineAfterAccessModifier, FormatStyle::ELAAMS_Never); + EXPECT_EQ(NL_After_Never, format(NL_After_Never, Style)); + EXPECT_EQ(NL_After_Never, format(NL_After_Always, Style)); + + Style.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Always; + EXPECT_EQ(NL_After_Always, format(NL_After_Never, Style)); + EXPECT_EQ(NL_After_Always, format(NL_After_Always, Style)); + + Style.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Leave; + Style.MaxEmptyLinesToKeep = 0u; + EXPECT_EQ(NL_After_Never_Before_Never, format(NL_After_Never, Style)); + EXPECT_EQ(NL_After_Never_Before_Never, format(NL_After_Always, Style)); + EXPECT_EQ(NL_After_Never_Before_Never, format(NL_After_Always3Times, Style)); + + Style.MaxEmptyLinesToKeep = 1u; + EXPECT_EQ(NL_After_Never, format(NL_After_Never, Style)); + EXPECT_EQ(NL_After_Always, format(NL_After_Always, Style)); + EXPECT_EQ(NL_After_Always, format(NL_After_Always3Times, Style)); + + Style.MaxEmptyLinesToKeep = 10u; + EXPECT_EQ(NL_After_Never, format(NL_After_Never, Style)); + EXPECT_EQ(NL_After_Always, format(NL_After_Always, Style)); + EXPECT_EQ(NL_After_Always3Times, format(NL_After_Always3Times, Style)); + + // Test with comments + char const *const NL_Com_After_Never = "struct foo {\n" + "private:\n" + " // comment\n" + " void f() {}\n" + "\n" + "private: /* comment */\n" + " int i;\n" + "};\n"; + char const *const NL_Com_After_Always = "struct foo {\n" + "private:\n" + "\n" + " // comment\n" + " void f() {}\n" + "\n" + "private: /* comment */\n" + "\n" + " int i;\n" + "};\n"; + Style = getLLVMStyle(); + EXPECT_EQ(NL_Com_After_Never, format(NL_Com_After_Never, Style)); + EXPECT_EQ(NL_Com_After_Never, format(NL_Com_After_Always, Style)); + + Style.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Always; + EXPECT_EQ(NL_Com_After_Always, format(NL_Com_After_Never, Style)); + EXPECT_EQ(NL_Com_After_Always, format(NL_Com_After_Always, Style)); + + // Test with preprocessor defines + char const *const NL_PPD_After_Never = "struct foo {\n" + "private:\n" + "#ifdef FOO\n" + "#endif\n" + " void f() {}\n" + "};\n"; + char const *const NL_PPD_After_Always = "struct foo {\n" + "private:\n" + "\n" + "#ifdef FOO\n" + "#endif\n" + " void f() {}\n" + "};\n"; + Style = getLLVMStyle(); + EXPECT_EQ(NL_PPD_After_Never, format(NL_PPD_After_Never, Style)); + EXPECT_EQ(NL_PPD_After_Never, format(NL_PPD_After_Always, Style)); + + Style.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Always; + EXPECT_EQ(NL_PPD_After_Always, format(NL_PPD_After_Never, Style)); + EXPECT_EQ(NL_PPD_After_Always, format(NL_PPD_After_Always, Style)); + + // Combined tests with EmptyLineAfterAccessModifier + char const *const NL_B_0_A_0_I_0 = "struct foo {\n" + "private:\n" + " void f() {}\n" + "private:\n" + " int i;\n" + "private:\n" + "protected:\n" + " int j;\n" + "};\n"; + char const *const NL_B_1_A_1_I_1 = "struct foo {\n" + "private:\n" + "\n" + " void f() {}\n" + "\n" + "private:\n" + "\n" + " int i;\n" + "\n" + "private:\n" + "\n" + "protected:\n" + "\n" + " int j;\n" + "};\n"; + + char const *const NL_B_3_A_3_I_3 = "struct foo {\n" + "private:\n" + "\n\n\n" + " void f() {}\n" + "\n\n\n" + "private:\n" + "\n\n\n" + " int i;\n" + "\n\n\n" + "private:\n" + "\n\n\n" + "protected:\n" + "\n\n\n" + " int j;\n" + "};\n"; + + Style = getLLVMStyle(); + Style.MaxEmptyLinesToKeep = 10u; // Both adhere to MaxEmptyLinesToKeep + Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_Always; + Style.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Always; + EXPECT_EQ(NL_B_3_A_3_I_3, format(NL_B_3_A_3_I_3, Style)); + + Style.MaxEmptyLinesToKeep = 0u; // Both force one new line + Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_Always; + Style.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Always; + EXPECT_EQ(NL_B_1_A_1_I_1, format(NL_B_3_A_3_I_3, Style)); + + Style.MaxEmptyLinesToKeep = 10u; // Both remove all new lines + Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_Never; + Style.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Never; + EXPECT_EQ(NL_B_0_A_0_I_0, format(NL_B_3_A_3_I_3, Style)); + + Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_Leave; + Style.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Leave; + Style.MaxEmptyLinesToKeep = 10u; + EXPECT_EQ(NL_B_3_A_3_I_3, format(NL_B_3_A_3_I_3, Style)); + Style.MaxEmptyLinesToKeep = 3u; + EXPECT_EQ(NL_B_3_A_3_I_3, format(NL_B_3_A_3_I_3, Style)); + Style.MaxEmptyLinesToKeep = 1u; + EXPECT_EQ(NL_B_1_A_1_I_1, format(NL_B_3_A_3_I_3, Style)); + + Style.MaxEmptyLinesToKeep = 10u; + Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_Always; + Style.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Leave; + EXPECT_EQ(NL_B_3_A_3_I_3, format(NL_B_3_A_3_I_3, Style)); + + Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_Leave; + Style.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Always; + char const *const NL_B_3_A_1_I_3 = "struct foo {\n" + "private:\n" + "\n\n\n" + " void f() {}\n" + "\n\n\n" + "private:\n" + "\n\n\n" + " int i;\n" + "\n\n\n" + "private:\n" + "\n\n\n" + "protected:\n" + "\n\n\n" + " int j;\n" + "};\n"; + EXPECT_EQ(NL_B_3_A_1_I_3, format(NL_B_3_A_3_I_3, Style)); + + Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_Leave; + Style.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Never; + char const *const NL_B_3_A_0_I_0 = "struct foo {\n" + "private:\n" + " void f() {}\n" + "\n\n\n" + "private:\n" + " int i;\n" + "\n\n\n" + "private:\n" + "protected:\n" + " int j;\n" + "};\n"; + EXPECT_EQ(NL_B_3_A_0_I_0, format(NL_B_3_A_3_I_3, Style)); + + Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_Never; + Style.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Leave; + char const *const NL_B_0_A_3_I_0 = "struct foo {\n" + "private:\n" + "\n\n\n" + " void f() {}\n" + "private:\n" + "\n\n\n" + " int i;\n" + "private:\n" + "protected:\n" + "\n\n\n" + " int j;\n" + "};\n"; + EXPECT_EQ(NL_B_0_A_3_I_0, format(NL_B_3_A_3_I_3, Style)); + + Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_Always; + Style.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Never; + EXPECT_EQ(NL_B_3_A_0_I_0, format(NL_B_3_A_3_I_3, Style)); + + Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_Never; + Style.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Always; + char const *const NL_B_0_A_3_I_1 = "struct foo {\n" + "private:\n" + "\n\n\n" + " void f() {}\n" + "private:\n" + "\n\n\n" + " int i;\n" + "private:\n" + "\n" + "protected:\n" + "\n\n\n" + " int j;\n" + "};\n"; + EXPECT_EQ(NL_B_0_A_3_I_1, format(NL_B_3_A_3_I_3, Style)); + + Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_LogicalBlock; + Style.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Always; + char const *const NL_B_3_A_3_I_0 = "struct foo {\n" + "private:\n" + "\n\n\n" + " void f() {}\n" + "\n\n\n" + "private:\n" + "\n\n\n" + " int i;\n" + "\n\n\n" + "private:\n" + "protected:\n" + "\n\n\n" + " int j;\n" + "};\n"; + EXPECT_EQ(NL_B_3_A_3_I_0, format(NL_B_3_A_3_I_3, Style)); + + Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_LogicalBlock; + Style.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Leave; + EXPECT_EQ(NL_B_3_A_3_I_0, format(NL_B_3_A_3_I_3, Style)); + + Style.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_LogicalBlock; + Style.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Never; + EXPECT_EQ(NL_B_3_A_0_I_0, format(NL_B_3_A_3_I_3, Style)); +} + TEST_F(FormatTest, FormatsArrays) { verifyFormat("aaaaaaaaaaaaaaaaaaaaaaaaa[aaaaaaaaaaaaaaaaaaaaaaaaa]\n" " [bbbbbbbbbbbbbbbbbbbbbbbbb] = c;"); Index: clang/lib/Format/UnwrappedLineFormatter.cpp =================================================================== --- clang/lib/Format/UnwrappedLineFormatter.cpp +++ clang/lib/Format/UnwrappedLineFormatter.cpp @@ -1278,10 +1278,21 @@ } } - // Remove empty lines after access specifiers. + // Insert or remove empty line after access specifiers. if (PreviousLine && PreviousLine->First->isAccessSpecifier() && - (!PreviousLine->InPPDirective || !RootToken.HasUnescapedNewline)) - Newlines = std::min(1u, Newlines); + (!PreviousLine->InPPDirective || !RootToken.HasUnescapedNewline)) { + switch (Style.EmptyLineAfterAccessModifier) { + case FormatStyle::ELAAMS_Never: + Newlines = 1; + break; + case FormatStyle::ELAAMS_Leave: + Newlines = std::max(Newlines, 1u); + break; + case FormatStyle::ELAAMS_Always: + Newlines = std::max(Newlines, 2u); + break; + } + } if (Newlines) Indent = NewlineIndent; Index: clang/lib/Format/Format.cpp =================================================================== --- clang/lib/Format/Format.cpp +++ clang/lib/Format/Format.cpp @@ -241,6 +241,17 @@ } }; +template <> +struct ScalarEnumerationTraits< + FormatStyle::EmptyLineAfterAccessModifierStyle> { + static void + enumeration(IO &IO, FormatStyle::EmptyLineAfterAccessModifierStyle &Value) { + IO.enumCase(Value, "Never", FormatStyle::ELAAMS_Never); + IO.enumCase(Value, "Leave", FormatStyle::ELAAMS_Leave); + IO.enumCase(Value, "Always", FormatStyle::ELAAMS_Always); + } +}; + template <> struct ScalarEnumerationTraits< FormatStyle::EmptyLineBeforeAccessModifierStyle> { @@ -584,6 +595,8 @@ IO.mapOptional("DeriveLineEnding", Style.DeriveLineEnding); IO.mapOptional("DerivePointerAlignment", Style.DerivePointerAlignment); IO.mapOptional("DisableFormat", Style.DisableFormat); + IO.mapOptional("EmptyLineAfterAccessModifier", + Style.EmptyLineAfterAccessModifier); IO.mapOptional("EmptyLineBeforeAccessModifier", Style.EmptyLineBeforeAccessModifier); IO.mapOptional("ExperimentalAutoDetectBinPacking", @@ -974,6 +987,7 @@ LLVMStyle.Cpp11BracedListStyle = true; LLVMStyle.DeriveLineEnding = true; LLVMStyle.DerivePointerAlignment = false; + LLVMStyle.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Never; LLVMStyle.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_LogicalBlock; LLVMStyle.ExperimentalAutoDetectBinPacking = false; LLVMStyle.FixNamespaceComments = true; Index: clang/include/clang/Format/Format.h =================================================================== --- clang/include/clang/Format/Format.h +++ clang/include/clang/Format/Format.h @@ -1891,6 +1891,52 @@ /// Disables formatting completely. bool DisableFormat; + /// Different styles for empty line after access modifiers. + enum EmptyLineAfterAccessModifierStyle : unsigned char { + /// Remove all empty lines after access modifiers. + /// \code + /// struct foo { + /// private: + /// int i; + /// protected: + /// int j; + /// /* comment */ + /// public: + /// foo() {} + /// private: + /// protected: + /// }; + /// \endcode + ELAAMS_Never, + /// Keep existing empty lines after access modifiers. + /// MaxEmptyLinesToKeep is applied instead. + ELAAMS_Leave, + /// Always add empty line after access modifiers if there are none. + /// MaxEmptyLinesToKeep is applied also. + /// \code + /// struct foo { + /// private: + // + /// int i; + /// protected: + // + /// int j; + /// /* comment */ + /// public: + // + /// foo() {} + /// private: + /// + /// protected: + // + /// }; + /// \endcode + ELAAMS_Always, + }; + + /// Defines in which cases to put empty line after access modifiers. + EmptyLineAfterAccessModifierStyle EmptyLineAfterAccessModifier; + /// Different styles for empty line before access modifiers. enum EmptyLineBeforeAccessModifierStyle : unsigned char { /// Remove all empty lines before access modifiers. @@ -3200,6 +3246,7 @@ DeriveLineEnding == R.DeriveLineEnding && DerivePointerAlignment == R.DerivePointerAlignment && DisableFormat == R.DisableFormat && + EmptyLineAfterAccessModifier == R.EmptyLineAfterAccessModifier && EmptyLineBeforeAccessModifier == R.EmptyLineBeforeAccessModifier && ExperimentalAutoDetectBinPacking == R.ExperimentalAutoDetectBinPacking && Index: clang/docs/ReleaseNotes.rst =================================================================== --- clang/docs/ReleaseNotes.rst +++ clang/docs/ReleaseNotes.rst @@ -209,6 +209,9 @@ - Support for Whitesmiths has been improved, with fixes for ``namespace`` blocks and ``case`` blocks and labels. +- Option ``EmptyLineAfterAccessModifier`` has been added to remove, force or keep + new lines after access modifiers. + libclang -------- Index: clang/docs/ClangFormatStyleOptions.rst =================================================================== --- clang/docs/ClangFormatStyleOptions.rst +++ clang/docs/ClangFormatStyleOptions.rst @@ -2128,6 +2128,55 @@ **DisableFormat** (``bool``) Disables formatting completely. +**EmptyLineAfterAccessModifier** (``EmptyLineAfterAccessModifierStyle``) + Defines in which cases to put empty line after access modifiers. + + Possible values: + + * ``ELAAMS_Never`` (in configuration: ``Never``) + Remove all empty lines after access modifiers. + + .. code-block:: c++ + + struct foo { + private: + int i; + protected: + int j; + /* comment */ + public: + foo() {} + private: + protected: + }; + + * ``ELAAMS_Leave`` (in configuration: ``Leave``) + Keep existing empty lines after access modifiers. + MaxEmptyLinesToKeep is applied instead. + + * ``ELAAMS_Always`` (in configuration: ``Always``) + Always add empty line after access modifiers if there are none. + MaxEmptyLinesToKeep is applied also. + + .. code-block:: c++ + + struct foo { + private: + + int i; + protected: + + int j; + /* comment */ + public: + + foo() {} + private: + + protected: + + }; + **EmptyLineBeforeAccessModifier** (``EmptyLineBeforeAccessModifierStyle``) Defines in which cases to put empty line before access modifiers. @@ -2195,8 +2244,6 @@ protected: }; - - **ExperimentalAutoDetectBinPacking** (``bool``) If ``true``, clang-format detects whether function calls and definitions are formatted with one parameter per line.
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits