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
