[clang] fd4e08a - [clang-format] Inconsistent behavior regarding line break before access modifier

[Marek Kurdej via cfe-commits]

Author: Max Sagebaum
Date: 2021-04-16T10:39:13+02:00
New Revision: fd4e08aa8f7edfe877d022859ce162a1b4fa684c

URL: https://github.com/llvm/llvm-project/commit/fd4e08aa8f7edfe877d022859ce162a1b4fa684c
DIFF: https://github.com/llvm/llvm-project/commit/fd4e08aa8f7edfe877d022859ce162a1b4fa684c.diff

LOG: [clang-format] Inconsistent behavior regarding line break before access modifier

Fixes https://llvm.org/PR41870.

Checks for newlines in option Style.EmptyLineBeforeAccessModifier are now based on the formatted new lines and not on the new lines in the file.

Reviewed By: HazardyKnusperkeks, curdeius

Differential Revision: https://reviews.llvm.org/D99503

Added: 
    

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/lib/Format/UnwrappedLineFormatter.cpp
    clang/unittests/Format/FormatTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ddf048c2dd33a..ad7ff71802ddc 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -225,6 +225,10 @@ clang-format
 - Option ``EmptyLineAfterAccessModifier`` has been added to remove, force or keep
   new lines after access modifiers.
 
+- Checks for newlines in option ``EmptyLineBeforeAccessModifier`` are now based
+  on the formatted new lines and not on the new lines in the file. (Fixes
+  https://llvm.org/PR41870.)
+
 libclang
 --------
 

diff  --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp
index 991f4fafe5ac2..e38d90d58362e 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -1254,15 +1254,14 @@ void UnwrappedLineFormatter::formatFirstToken(
   if (PreviousLine && RootToken.isAccessSpecifier()) {
     switch (Style.EmptyLineBeforeAccessModifier) {
     case FormatStyle::ELBAMS_Never:
-      if (RootToken.NewlinesBefore > 1)
+      if (Newlines > 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)
+      if (PreviousLine->Last->isOneOf(tok::semi, tok::r_brace) && Newlines <= 1)
         Newlines = 2;
       if (PreviousLine->First->isAccessSpecifier())
         Newlines = 1; // Previous is an access modifier remove all new lines.
@@ -1273,8 +1272,7 @@ void UnwrappedLineFormatter::formatFirstToken(
         previousToken = PreviousLine->Last->getPreviousNonComment();
       else
         previousToken = PreviousLine->Last;
-      if ((!previousToken || !previousToken->is(tok::l_brace)) &&
-          RootToken.NewlinesBefore <= 1)
+      if ((!previousToken || !previousToken->is(tok::l_brace)) && Newlines <= 1)
         Newlines = 2;
     } break;
     }

diff  --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 36d560469119a..5ae67d91323df 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -9177,6 +9177,48 @@ TEST_F(FormatTest, FormatsAccessModifiers) {
                "  int j;\n"
                "};\n",
                Style);
+
+  FormatStyle NoEmptyLines = getLLVMStyle();
+  NoEmptyLines.MaxEmptyLinesToKeep = 0;
+  verifyFormat("struct foo {\n"
+               "private:\n"
+               "  void f() {}\n"
+               "\n"
+               "private:\n"
+               "  int i;\n"
+               "\n"
+               "public:\n"
+               "protected:\n"
+               "  int j;\n"
+               "};\n",
+               NoEmptyLines);
+
+  NoEmptyLines.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_Never;
+  verifyFormat("struct foo {\n"
+               "private:\n"
+               "  void f() {}\n"
+               "private:\n"
+               "  int i;\n"
+               "public:\n"
+               "protected:\n"
+               "  int j;\n"
+               "};\n",
+               NoEmptyLines);
+
+  NoEmptyLines.EmptyLineBeforeAccessModifier = FormatStyle::ELBAMS_Always;
+  verifyFormat("struct foo {\n"
+               "private:\n"
+               "  void f() {}\n"
+               "\n"
+               "private:\n"
+               "  int i;\n"
+               "\n"
+               "public:\n"
+               "\n"
+               "protected:\n"
+               "  int j;\n"
+               "};\n",
+               NoEmptyLines);
 }
 
 TEST_F(FormatTest, FormatsAfterAccessModifiers) {
@@ -9266,32 +9308,26 @@ TEST_F(FormatTest, FormatsAfterAccessModifiers) {
   // Leave tests rely on the code layout, test::messUp can not be used.
   Style.EmptyLineAfterAccessModifier = FormatStyle::ELAAMS_Leave;
   Style.MaxEmptyLinesToKeep = 0u;
-  EXPECT_EQ("struct foo {\n"
-            "private:\n"
-            "  void f() {}\n"
-            "private:\n"
-            "  int i;\n"
-            "protected:\n"
-            "  int j;\n"
-            "};\n",
-            format("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",
+               Style);
 
   // Check if MaxEmptyLinesToKeep is respected.
   EXPECT_EQ("struct foo {\n"
             "private:\n"
             "  void f() {}\n"
+            "\n"
             "private:\n"
             "  int i;\n"
+            "\n"
             "protected:\n"
             "  int j;\n"
             "};\n",