poelmanc updated this revision to Diff 234976.
poelmanc added a comment.

Address most of the feedback, I'll comment individually.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68682/new/

https://reviews.llvm.org/D68682

Files:
  clang-tools-extra/clang-tidy/readability/RedundantControlFlowCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize-redundant-void-arg.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-control-flow.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-declaration.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-member-init.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp
  clang-tools-extra/unittests/clang-change-namespace/ChangeNamespaceTests.cpp
  clang-tools-extra/unittests/clang-move/ClangMoveTests.cpp
  clang/include/clang/Basic/CharInfo.h
  clang/include/clang/Format/Format.h
  clang/lib/AST/CommentLexer.cpp
  clang/lib/AST/CommentParser.cpp
  clang/lib/Format/Format.cpp
  clang/unittests/Format/CleanupTest.cpp

Index: clang/unittests/Format/CleanupTest.cpp
===================================================================
--- clang/unittests/Format/CleanupTest.cpp
+++ clang/unittests/Format/CleanupTest.cpp
@@ -349,11 +349,13 @@
                      "namespace C {\n"
                      "namespace D { int i; }\n"
                      "inline namespace E { namespace { int y; } }\n"
-                     "int x=     0;"
+                     "\n"
+                     "int x=     0;\n"
                      "}";
-  std::string Expected = "\n\nnamespace C {\n"
-                         "namespace D { int i; }\n\n"
-                         "int x=     0;"
+  std::string Expected = "\nnamespace C {\n"
+                         "namespace D { int i; }\n"
+                         "\n"
+                         "int x=     0;\n"
                          "}";
   tooling::Replacements Replaces =
       toReplacements({createReplacement(getOffset(Code, 3, 3), 6, ""),
@@ -362,6 +364,87 @@
   EXPECT_EQ(Expected, formatAndApply(Code, Replaces));
 }
 
+TEST_F(CleanUpReplacementsTest, RemoveLineWhenAllNonWhitespaceRemoved) {
+  std::string Code = "namespace A { // Useless comment\n"
+                     "  int x\t = 0;\t\n"
+                     "  int y\t = 0;\t\n"
+                     "} // namespace A\n";
+  std::string Expected = "namespace A {\n"
+                         "  int y\t = 0;\n"
+                         "} // namespace A\n";
+  tooling::Replacements Replaces =
+      toReplacements({createReplacement(getOffset(Code, 1, 14), 19, ""),
+                      createReplacement(getOffset(Code, 2, 3), 3, ""),
+                      createReplacement(getOffset(Code, 2, 7), 1, ""),
+                      createReplacement(getOffset(Code, 2, 10), 1, ""),
+                      createReplacement(getOffset(Code, 2, 12), 2, ""),
+                      createReplacement(getOffset(Code, 3, 14), 1, "")});
+
+  EXPECT_EQ(Expected, apply(Code, Replaces));
+}
+
+TEST_F(CleanUpReplacementsTest, RemoveLinesWhenAllNonWhitespaceRemoved) {
+  std::string Code = "struct A {\n"
+                     "  A()\n"
+                     "  : f(),\n"
+                     "    g()\n"
+                     "  {}\n"
+                     "  int f = 0;\n"
+                     "  int g = 0;\n"
+                     "};";
+  std::string Expected = "struct A {\n"
+                         "  A()\n"
+                         "  {}\n"
+                         "  int f = 0;\n"
+                         "  int g = 0;\n"
+                         "};";
+  tooling::Replacements Replaces =
+      toReplacements({createReplacement(getOffset(Code, 3, 5), 3, ""),
+                      createReplacement(getOffset(Code, 3, 8), 1, ""),
+                      createReplacement(getOffset(Code, 3, 3), 1, ""),
+                      createReplacement(getOffset(Code, 4, 5), 3, "")});
+
+  EXPECT_EQ(Expected, apply(Code, Replaces));
+}
+
+TEST_F(CleanUpReplacementsTest, KeepLinesWithInsertsOrReplacesEvenIfBlank) {
+  std::string Code = "struct A {\n"
+                     "  A() {}\n"
+                     "  int f;\n"
+                     "  \n"
+                     "  \n"
+                     "};";
+  std::string Expected = "struct A {\n"
+                         "  A() {}\n"
+                         "\n"
+                         "        \n"
+                         "  \n"
+                         "\t\n"
+                         "};";
+  tooling::Replacements Replaces =
+      toReplacements({createReplacement(getOffset(Code, 3, 3), 3, "   "),
+                      createReplacement(getOffset(Code, 3, 7), 2, "  "),
+                      createReplacement(getOffset(Code, 3, 1), 0, "\n"),
+                      createReplacement(getOffset(Code, 5, 1), 2, "\t")});
+
+  EXPECT_EQ(Expected, apply(Code, Replaces));
+}
+
+TEST_F(CleanUpReplacementsTest, KeepPreviouslyBlankLinesAfterPartialRemoval) {
+  std::string Code = "    \n"
+                     "\t\t\n"
+                     "\n";
+  std::string Expected = "  \n"
+                         "\t\n"
+                         "";
+  tooling::Replacements Replaces =
+      toReplacements({createReplacement(getOffset(Code, 1, 2), 2, ""),
+                      createReplacement(getOffset(Code, 2, 2), 1, ""),
+                      createReplacement(getOffset(Code, 3, 1), 1, "")});
+
+  EXPECT_EQ(Expected, apply(Code, Replaces));
+}
+
 TEST_F(CleanUpReplacementsTest, InsertMultipleIncludesLLVMStyle) {
   std::string Code = "#include \"x/fix.h\"\n"
                      "#include \"a.h\"\n"
Index: clang/lib/Format/Format.cpp
===================================================================
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -25,6 +25,7 @@
 #include "UnwrappedLineParser.h"
 #include "UsingDeclarationsSorter.h"
 #include "WhitespaceManager.h"
+#include "clang/Basic/CharInfo.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/SourceManager.h"
@@ -2374,6 +2375,87 @@
 
 } // anonymous namespace
 
+llvm::Expected<tooling::Replacements>
+removeNewlyBlankLines(StringRef Code, const tooling::Replacements &Replaces) {
+  tooling::Replacements NewReplaces(Replaces);
+  // pair< LineStartPos, CheckedThroughPos > of lines that have been checked
+  // and confirmed that the replacement result so far will be entirely blank.
+  std::vector<std::pair<int, int>> PotentialWholeLineCuts;
+  int LineStartPos = -1;
+  int LineCheckedThroughPos = -1;
+  bool LineBlankSoFar = true;
+  const char *FileText = Code.data();
+  StringRef FilePath; // Must be the same for every Replacement
+  for (const auto &R : Replaces) {
+    assert(FilePath.empty() || FilePath == R.getFilePath());
+    FilePath = R.getFilePath();
+    const int RStartPos = R.getOffset();
+
+    int CurrentRLineStartPos = RStartPos;
+    while (CurrentRLineStartPos > 0 &&
+           !isVerticalWhitespace(FileText[CurrentRLineStartPos - 1])) {
+      --CurrentRLineStartPos;
+    }
+
+    assert(CurrentRLineStartPos >= LineStartPos);
+    if (CurrentRLineStartPos != LineStartPos) {
+      // We've moved on to a new line. Wrap up the old one before moving on.
+      if (LineBlankSoFar) {
+        PotentialWholeLineCuts.push_back(
+            std::make_pair(LineStartPos, LineCheckedThroughPos));
+      }
+      LineCheckedThroughPos = CurrentRLineStartPos;
+      LineStartPos = CurrentRLineStartPos;
+      LineBlankSoFar = true;
+    }
+
+    // Check to see if line from LineCheckedThroughPos to here is blank.
+    assert(RStartPos >= LineCheckedThroughPos);
+    StringRef PriorTextToCheck(FileText + LineCheckedThroughPos,
+                               RStartPos - LineCheckedThroughPos);
+    StringRef ReplacementText = R.getReplacementText();
+    LineBlankSoFar = LineBlankSoFar &&
+                     isAllWhitespace(PriorTextToCheck) &&
+                     ReplacementText.empty();
+    LineCheckedThroughPos = R.getOffset() + R.getLength();
+  }
+
+  if (LineBlankSoFar) {
+    PotentialWholeLineCuts.push_back(
+        std::make_pair(LineStartPos, LineCheckedThroughPos));
+  }
+
+  // Now remove whole line if and only if (a) rest of line is blank, and
+  // (b) the original line was *not* blank.
+  for (const auto &LineCheckedThrough : PotentialWholeLineCuts) {
+    const int LineStartPos = LineCheckedThrough.first;
+    const int CheckedThroughPos = LineCheckedThrough.second;
+
+    int LineEndPos = CheckedThroughPos;
+    while (LineEndPos < Code.size() &&
+           !isVerticalWhitespace(FileText[LineEndPos])) {
+      ++LineEndPos;
+    }
+
+    assert(LineEndPos >= CheckedThroughPos);
+    StringRef TrailingText(FileText + CheckedThroughPos,
+                           LineEndPos - CheckedThroughPos);
+    assert(LineEndPos >= LineStartPos);
+    StringRef OriginalLine(FileText + LineStartPos, LineEndPos - LineStartPos);
+    if (isAllWhitespace(TrailingText) &&
+        !isAllWhitespace(OriginalLine)) {
+      const char *CutTo = skipNewlineChars(FileText + LineEndPos, Code.end());
+      int CutCount = CutTo - FileText - LineStartPos;
+      llvm::Error Err = NewReplaces.add(
+          tooling::Replacement(FilePath, LineStartPos, CutCount, ""));
+      if (Err) {
+        return llvm::Expected<tooling::Replacements>(std::move(Err));
+      }
+    }
+  }
+  return NewReplaces;
+}
+
 llvm::Expected<tooling::Replacements>
 cleanupAroundReplacements(StringRef Code, const tooling::Replacements &Replaces,
                           const FormatStyle &Style) {
@@ -2387,7 +2469,12 @@
   // Make header insertion replacements insert new headers into correct blocks.
   tooling::Replacements NewReplaces =
       fixCppIncludeInsertions(Code, Replaces, Style);
-  return processReplacements(Cleanup, Code, NewReplaces, Style);
+  llvm::Expected<tooling::Replacements> ProcessedReplaces =
+      processReplacements(Cleanup, Code, NewReplaces, Style);
+  // If success, also remove lines made blank by removals.
+  return (ProcessedReplaces
+              ? format::removeNewlyBlankLines(Code, *ProcessedReplaces)
+              : std::move(ProcessedReplaces));
 }
 
 namespace internal {
Index: clang/lib/AST/CommentParser.cpp
===================================================================
--- clang/lib/AST/CommentParser.cpp
+++ clang/lib/AST/CommentParser.cpp
@@ -16,14 +16,6 @@
 
 namespace clang {
 
-static inline bool isWhitespace(llvm::StringRef S) {
-  for (StringRef::const_iterator I = S.begin(), E = S.end(); I != E; ++I) {
-    if (!isWhitespace(*I))
-      return false;
-  }
-  return true;
-}
-
 namespace comments {
 
 /// Re-lexes a sequence of tok::text tokens.
@@ -609,7 +601,7 @@
       }
       // Also allow [tok::newline, tok::text, tok::newline] if the middle
       // tok::text is just whitespace.
-      if (Tok.is(tok::text) && isWhitespace(Tok.getText())) {
+      if (Tok.is(tok::text) && isAllWhitespace(Tok.getText())) {
         Token WhitespaceTok = Tok;
         consumeToken();
         if (Tok.is(tok::newline) || Tok.is(tok::eof)) {
Index: clang/lib/AST/CommentLexer.cpp
===================================================================
--- clang/lib/AST/CommentLexer.cpp
+++ clang/lib/AST/CommentLexer.cpp
@@ -131,21 +131,6 @@
   return BufferEnd;
 }
 
-const char *skipNewline(const char *BufferPtr, const char *BufferEnd) {
-  if (BufferPtr == BufferEnd)
-    return BufferPtr;
-
-  if (*BufferPtr == '\n')
-    BufferPtr++;
-  else {
-    assert(*BufferPtr == '\r');
-    BufferPtr++;
-    if (BufferPtr != BufferEnd && *BufferPtr == '\n')
-      BufferPtr++;
-  }
-  return BufferPtr;
-}
-
 const char *skipNamedCharacterReference(const char *BufferPtr,
                                         const char *BufferEnd) {
   for ( ; BufferPtr != BufferEnd; ++BufferPtr) {
@@ -254,7 +239,7 @@
         (EscapePtr - 2 >= BufferPtr && EscapePtr[0] == '/' &&
          EscapePtr[-1] == '?' && EscapePtr[-2] == '?')) {
       // We found an escaped newline.
-      CurPtr = skipNewline(CurPtr, BufferEnd);
+      CurPtr = skipNewlineChars(CurPtr, BufferEnd);
     } else
       return CurPtr; // Not an escaped newline.
   }
@@ -302,7 +287,7 @@
     switch (*TokenPtr) {
       case '\n':
       case '\r':
-          TokenPtr = skipNewline(TokenPtr, CommentEnd);
+          TokenPtr = skipNewlineChars(TokenPtr, CommentEnd);
           formTokenWithChars(T, TokenPtr, tok::newline);
 
           if (CommentState == LCS_InsideCComment)
@@ -477,7 +462,7 @@
   // text content.
   if (BufferPtr != CommentEnd &&
       isVerticalWhitespace(*BufferPtr)) {
-    BufferPtr = skipNewline(BufferPtr, CommentEnd);
+    BufferPtr = skipNewlineChars(BufferPtr, CommentEnd);
     State = LS_VerbatimBlockBody;
     return;
   }
@@ -503,7 +488,7 @@
   if (Pos == StringRef::npos) {
     // Current line is completely verbatim.
     TextEnd = Newline;
-    NextLine = skipNewline(Newline, CommentEnd);
+    NextLine = skipNewlineChars(Newline, CommentEnd);
   } else if (Pos == 0) {
     // Current line contains just an end command.
     const char *End = BufferPtr + VerbatimBlockEndCommandName.size();
Index: clang/include/clang/Format/Format.h
===================================================================
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -2284,6 +2284,13 @@
 formatReplacements(StringRef Code, const tooling::Replacements &Replaces,
                    const FormatStyle &Style);
 
+/// Examines Replaces for consecutive sequences of one or more deletions on
+/// the same line that would leave a previously non-blank line blank. Returns
+/// extended Replacements that fully remove each such newly blank line,
+/// including trailing newline character(s), to avoid introducing blank lines.
+llvm::Expected<tooling::Replacements>
+removeNewlyBlankLines(StringRef Code, const tooling::Replacements &Replaces);
+
 /// Returns the replacements corresponding to applying \p Replaces and
 /// cleaning up the code after that on success; otherwise, return an llvm::Error
 /// carrying llvm::StringError.
Index: clang/include/clang/Basic/CharInfo.h
===================================================================
--- clang/include/clang/Basic/CharInfo.h
+++ clang/include/clang/Basic/CharInfo.h
@@ -89,6 +89,11 @@
   return (InfoTable[c] & (CHAR_HORZ_WS|CHAR_VERT_WS|CHAR_SPACE)) != 0;
 }
 
+/// Returns true if and only if S consists entirely of whitespace.
+LLVM_READONLY inline bool isAllWhitespace(llvm::StringRef S) {
+  return llvm::all_of(S, isWhitespace);
+}
+
 /// Return true if this character is an ASCII digit: [0-9]
 LLVM_READONLY inline bool isDigit(unsigned char c) {
   using namespace charinfo;
@@ -193,6 +198,25 @@
   return true;
 }
 
+/// Requires that BufferPtr point to a newline character ("\n" or "\r").
+/// Returns a pointer past the end of any platform newline, i.e. past
+/// "\n", "\r", or "\r\n", up to BufferEnd.
+LLVM_READONLY inline const char *skipNewlineChars(const char *BufferPtr,
+                                                  const char *BufferEnd) {
+  if (BufferPtr == BufferEnd)
+    return BufferPtr;
+
+  if (*BufferPtr == '\n')
+    BufferPtr++;
+  else {
+    assert(*BufferPtr == '\r');
+    BufferPtr++;
+    if (BufferPtr != BufferEnd && *BufferPtr == '\n')
+      BufferPtr++;
+  }
+  return BufferPtr;
+}
+
 } // end namespace clang
 
 #endif
Index: clang-tools-extra/unittests/clang-move/ClangMoveTests.cpp
===================================================================
--- clang-tools-extra/unittests/clang-move/ClangMoveTests.cpp
+++ clang-tools-extra/unittests/clang-move/ClangMoveTests.cpp
@@ -94,7 +94,6 @@
                                   "class C1; // test\n"
                                   "template <typename T> class C2;\n"
                                   "namespace b {\n"
-                                  "\n"
                                   "class Foo2 {\n"
                                   "public:\n"
                                   "  int f();\n"
Index: clang-tools-extra/unittests/clang-change-namespace/ChangeNamespaceTests.cpp
===================================================================
--- clang-tools-extra/unittests/clang-change-namespace/ChangeNamespaceTests.cpp
+++ clang-tools-extra/unittests/clang-change-namespace/ChangeNamespaceTests.cpp
@@ -89,8 +89,7 @@
                      "class A {};\n"
                      "} // namespace nb\n"
                      "} // namespace na\n";
-  std::string Expected = "\n\n"
-                         "namespace x {\n"
+  std::string Expected = "namespace x {\n"
                          "namespace y {\n"
                          "class A {};\n"
                          "} // namespace y\n"
@@ -145,8 +144,7 @@
                      "\n"
                      "} // namespace nb\n"
                      "} // namespace na\n";
-  std::string Expected = "\n\n"
-                         "namespace nx {\n"
+  std::string Expected = "namespace nx {\n"
                          "namespace ny {\n"
                          "\n"
                          "class A {};\n"
@@ -187,7 +185,6 @@
                      "} // namespace nb\n"
                      "} // namespace na\n";
   std::string Expected = "namespace na {\n"
-                         "\n"
                          "namespace nc {\n"
                          "class A {};\n"
                          "} // namespace nc\n"
@@ -205,7 +202,6 @@
                      "} // namespace na\n";
   std::string Expected = "namespace na {\n"
                          "class A {};\n"
-                         "\n"
                          "namespace nc {\n"
                          "class X { A a; };\n"
                          "} // namespace nc\n"
@@ -237,7 +233,7 @@
                          "} // namespace y\n"
                          "} // namespace x\n"
                          "namespace na {\n"
-                         "class A {};\n\n"
+                         "class A {};\n"
                          "} // namespace na\n"
                          "namespace x {\n"
                          "namespace y {\n"
@@ -257,7 +253,6 @@
                      "} // namespace na\n";
   std::string Expected = "namespace na {\n"
                          "class A {};\n"
-                         "\n"
                          "namespace x {\n"
                          "namespace y {\n"
                          "class B {};\n"
@@ -289,7 +284,6 @@
                          "namespace nc {\n"
                          "class C_C {};"
                          "} // namespace nc\n"
-                         "\n"
                          "} // namespace na\n"
                          "namespace x {\n"
                          "namespace y {\n"
@@ -443,8 +437,7 @@
                      "};\n"
                      "} // namespace nb\n"
                      "} // namespace na\n";
-  std::string Expected = "\n\n"
-                         "namespace x {\n"
+  std::string Expected = "namespace x {\n"
                          "namespace y {\n"
                          "class A {\n"
                          "  class FWD;\n"
@@ -475,7 +468,6 @@
                          "namespace nc {\n"
                          "class C_C {};"
                          "} // namespace nc\n"
-                         "\n"
                          "} // namespace na\n"
                          "namespace x {\n"
                          "namespace y {\n"
@@ -515,7 +507,6 @@
                          "namespace nd {\n"
                          "class SAME {};\n"
                          "}\n"
-                         "\n"
                          "} // namespace na\n"
                          "namespace x {\n"
                          "namespace y {\n"
@@ -544,7 +535,6 @@
   std::string Expected = "namespace na {\n"
                          "class A {};\n"
                          "class Base { public: Base() {} void m() {} };\n"
-                         "\n"
                          "} // namespace na\n"
                          "namespace x {\n"
                          "namespace y {\n"
@@ -590,7 +580,6 @@
       "      static void nestedFunc() {}\n"
       "  };\n"
       "};\n"
-      "\n"
       "}  // namespace na\n"
       "namespace x {\n"
       "namespace y {\n"
@@ -633,7 +622,6 @@
       "void A::g() {}"
       "void a_f() {}\n"
       "static void static_f() {}\n"
-      "\n"
       "}  // namespace na\n"
       "namespace x {\n"
       "namespace y {\n"
@@ -669,7 +657,6 @@
       "  bool operator==(const A &RHS) const { return x == RHS.x; }\n"
       "};\n"
       "bool operator<(const A &LHS, const A &RHS) { return LHS.x == RHS.x; }\n"
-      "\n"
       "}  // namespace na\n"
       "namespace x {\n"
       "namespace y {\n"
@@ -709,7 +696,6 @@
       "};\n"
       "void a_f() {}\n"
       "static void s_f() {}\n"
-      "\n"
       "}  // namespace na\n"
       "namespace x {\n"
       "namespace y {\n"
@@ -742,7 +728,6 @@
                          "int GlobA;\n"
                          "static int GlobAStatic = 0;\n"
                          "namespace nc { int GlobC; }\n"
-                         "\n"
                          "} // namespace na\n"
                          "namespace x {\n"
                          "namespace y {\n"
@@ -780,7 +765,6 @@
                          "static int A2;\n"
                          "};\n"
                          "int A::A1 = 0;\n"
-                         "\n"
                          "} // namespace na\n"
                          "namespace x {\n"
                          "namespace y {\n"
@@ -828,9 +812,7 @@
                      "};\n"
                      "}\n"
                      "}";
-  std::string Expected = "\n"
-                         "\n"
-                         "namespace x {\n"
+  std::string Expected = "namespace x {\n"
                          "namespace y {\n"
                          "\n\n"
                          "// Wild comments.\n"
@@ -865,7 +847,6 @@
                          "}\n"
                          "using glob::Glob;\n"
                          "using glob::GFunc;\n"
-                         "\n"
                          "namespace x {\n"
                          "namespace y {\n"
                          "void f() { Glob g; GFunc(); }\n"
@@ -893,7 +874,6 @@
                          "class Util {};\n"
                          "void func() {}\n"
                          "} // namespace util\n"
-                         "\n"
                          "namespace x {\n"
                          "namespace y {\n"
                          "namespace {\n"
@@ -921,7 +901,6 @@
                          "class Glob {};\n"
                          "}\n"
                          "using namespace glob;\n"
-                         "\n"
                          "namespace x {\n"
                          "namespace y {\n"
                          "void f() { Glob g; }\n"
@@ -950,7 +929,6 @@
       "namespace glob2 { class Glob2 {}; }\n"
       "namespace gl = glob;\n"
       "namespace gl2 = glob2;\n"
-      "\n"
       "namespace x {\n"
       "namespace y {\n"
       "void f() { gl::Glob g; gl2::Glob2 g2; }\n"
@@ -973,7 +951,6 @@
   std::string Expected = "namespace glob {\n"
                          "class Glob {};\n"
                          "}\n"
-                         "\n"
                          "namespace x {\n"
                          "namespace y {\n"
                          "namespace gl = glob;\n"
@@ -1002,7 +979,6 @@
                          "namespace other { namespace gl = glob; }\n"
                          "namespace na {\n"
                          "namespace ga = glob;\n"
-                         "\n"
                          "namespace nx {\n"
                          "void f() { ga::Glob g; }\n"
                          "} // namespace nx\n"
@@ -1028,7 +1004,6 @@
                          "namespace other { namespace gl = glob; }\n"
                          "namespace na {\n"
                          "namespace ga = glob;\n"
-                         "\n"
                          "} // namespace na\n"
                          "namespace x {\n"
                          "namespace y {\n"
@@ -1053,7 +1028,6 @@
   std::string Expected = "namespace glob {\n"
                          "class Glob {};\n"
                          "}\n"
-                         "\n"
                          "namespace x {\n"
                          "namespace y {\n"
                          "void f() { glob::Glob g; }\n"
@@ -1080,7 +1054,6 @@
                          "class Glob {};\n"
                          "}\n"
                          "namespace na {\n"
-                         "\n"
                          "namespace nc {\n"
                          "void f() { glob::Glob g; }\n"
                          "} // namespace nc\n"
@@ -1110,7 +1083,6 @@
                          "}\n"
                          "using glob1::glob2::Glob;\n"
                          "using namespace glob1;\n"
-                         "\n"
                          "namespace x {\n"
                          "namespace y {\n"
                          "void f() { Glob g; }\n"
@@ -1136,7 +1108,6 @@
                          "}\n"
                          "using GLB = glob::Glob;\n"
                          "using BLG = glob::Glob;\n"
-                         "\n"
                          "namespace x {\n"
                          "namespace y {\n"
                          "void f() { GLB g; BLG blg; }\n"
@@ -1158,7 +1129,6 @@
                      "} // namespace na\n";
   std::string Expected = "namespace na { class C_A {};\n }\n"
                          "using na::C_A;\n"
-                         "\n"
                          "namespace x {\n"
                          "namespace y {\n"
                          "class C_X {\n"
@@ -1183,7 +1153,6 @@
                      "} // namespace na\n";
   std::string Expected = "namespace na { class C_A {};\n }\n"
                          "using namespace na;\n"
-                         "\n"
                          "namespace x {\n"
                          "namespace y {\n"
                          "class C_X {\n"
@@ -1211,7 +1180,6 @@
   std::string Expected = "namespace glob {\n"
                          "class Glob {};\n"
                          "}\n"
-                         "\n"
                          "namespace x {\n"
                          "namespace y {\n"
                          "void f() {\n"
@@ -1234,7 +1202,6 @@
                      "} // namespace nb\n"
                      "} // namespace na\n";
   std::string Expected = "namespace na { class C_A {}; }\n"
-                         "\n"
                          "namespace x {\n"
                          "namespace y {\n"
                          "void f() {\n"
@@ -1258,7 +1225,6 @@
   std::string Expected = "namespace nx { void f(); }\n"
                          "namespace na {\n"
                          "using nx::f;\n"
-                         "\n"
                          "} // na\n"
                          "namespace x {\n"
                          "namespace y {\n"
@@ -1277,7 +1243,6 @@
                      "} // na\n";
 
   std::string Expected = "namespace nx { void f(); }\n"
-                         "\n"
                          "namespace x {\n"
                          "namespace y {\n"
                          "using ::nx::f;\n"
@@ -1309,7 +1274,6 @@
                          "using ::nx::f;\n"
                          "namespace c {\n"
                          "using ::nx::g;\n"
-                         "\n"
                          "} // c\n"
                          "namespace x {\n"
                          "namespace y {\n"
@@ -1335,7 +1299,6 @@
   std::string Expected = "namespace na { class A {}; }\n"
                          "namespace nb {\n"
                          "using na::A;\n"
-                         "\n"
                          "namespace nd {\n"
                          "void d() { A a; }\n"
                          "} // namespace nd\n"
@@ -1354,7 +1317,6 @@
                      "} // nb\n";
 
   std::string Expected = "namespace na { class A {}; }\n"
-                         "\n"
                          "namespace nc {\n"
                          "using ::na::A;\n"
                          "void d() { A a; }\n"
@@ -1379,7 +1341,6 @@
                          "template <typename T>\n"
                          "class A { T t; };\n"
                          "} // namespace na\n"
-                         "\n"
                          "namespace nc {\n"
                          "using ::na::A;\n"
                          "void d() { A<int> a; }\n"
@@ -1403,7 +1364,6 @@
                          "template <typename T>\n"
                          "void f() { T t; };\n"
                          "} // namespace na\n"
-                         "\n"
                          "namespace nc {\n"
                          "using ::na::f;\n"
                          "void d() { f<int>(); }\n"
@@ -1422,7 +1382,6 @@
       "} // namespace na\n";
 
   std::string Expected = "namespace nx { namespace ny { class X {}; } }\n"
-                         "\n"
                          "namespace x {\n"
                          "namespace y {\n"
                          "using Y = nx::ny::X;\n"
@@ -1444,7 +1403,6 @@
 
   std::string Expected = "namespace nx { namespace ny { class X {}; } }\n"
                          "using Y = nx::ny::X;\n"
-                         "\n"
                          "namespace x {\n"
                          "namespace y {\n"
                          "void f() { Y y; }\n"
@@ -1465,7 +1423,6 @@
       "} // namespace na\n";
 
   std::string Expected = "namespace nx { namespace ny { class X {}; } }\n"
-                         "\n"
                          "namespace x {\n"
                          "namespace y {\n"
                          "typedef nx::ny::X Y;\n"
@@ -1490,7 +1447,6 @@
       "} // namespace na\n";
   std::string Expected =
       "namespace nx { namespace ny { class X { public: X(int i) {} }; } }\n"
-      "\n\n"
       "namespace x {\n"
       "namespace y {\n"
       "class A : public nx::ny::X {\n"
@@ -1519,7 +1475,6 @@
       "} // namespace na\n";
   std::string Expected =
       "namespace nx { namespace ny { class X { public: X(int i) {} }; } }\n"
-      "\n\n"
       "namespace x {\n"
       "namespace y {\n"
       "class A : public nx::ny::X {\n"
@@ -1548,7 +1503,6 @@
       "} // namespace na\n";
   std::string Expected =
       "namespace nx { namespace ny { class X { public: X(int i) {} }; } }\n"
-      "\n\n"
       "namespace x {\n"
       "namespace y {\n"
       "class A : public nx::ny::X {\n"
@@ -1585,7 +1539,6 @@
                          "namespace nc {\n"
                          "class C_C {};"
                          "} // namespace nc\n"
-                         "\n"
                          "} // namespace na\n"
                          "class C_X {\n"
                          "public:\n"
@@ -1622,7 +1575,6 @@
                          "namespace nc {\n"
                          "class C_C {};"
                          "} // namespace nc\n"
-                         "\n"
                          "} // namespace na\n"
                          "namespace x {\n"
                          "namespace y {\n"
@@ -1753,7 +1705,6 @@
                          "namespace nb {\n"
                          "class B {};\n"
                          "} // namespace nb\n"
-                         "\n"
                          "namespace ny {\n"
                          "namespace na {\n"
                          "namespace nc {\n"
@@ -1793,7 +1744,6 @@
                          "namespace ny {\n"
                          "class Y {};\n"
                          "}\n"
-                         "\n"
                          "namespace ny {\n"
                          "namespace na {\n"
                          "namespace nc {\n"
@@ -1831,7 +1781,6 @@
                          "namespace nc { class C {}; } // namespace nc\n"
                          "}\n // namespace na\n"
                          "}\n // namespace ny\n"
-                         "\n"
                          "namespace ny {\n"
                          "namespace na {\n"
                          "class X {\n"
@@ -1868,7 +1817,6 @@
                          "namespace nc { class C {}; } // namespace nc\n"
                          "}\n // namespace na\n"
                          "}\n // namespace ny\n"
-                         "\n"
                          "namespace ny {\n"
                          "namespace na {\n"
                          "namespace {\n"
@@ -1888,7 +1836,7 @@
                      "enum Y { Y1, Y2 };\n"
                      "} // namespace nb\n"
                      "} // namespace na\n";
-  std::string Expected = "\n\nnamespace x {\n"
+  std::string Expected = "namespace x {\n"
                          "namespace y {\n"
                          "enum class X { X1, X2 };\n"
                          "enum Y { Y1, Y2 };\n"
@@ -1917,7 +1865,6 @@
                          "namespace na {\n"
                          "enum class X { X1 };\n"
                          "enum Y { Y1, Y2 };\n"
-                         "\n"
                          "} // namespace na\n"
                          "namespace x {\n"
                          "namespace y {\n"
@@ -1952,7 +1899,6 @@
                          "enum class X { X1 };\n"
                          "enum Y { Y1, Y2 };\n"
                          "} // namespace ns\n"
-                         "\n"
                          "namespace x {\n"
                          "namespace y {\n"
                          "void f() {\n"
@@ -1994,7 +1940,6 @@
                          "using ns::Y;\n"
                          "using ns::Y::Y2;\n"
                          "using ns::Y::Y3;\n"
-                         "\n"
                          "namespace x {\n"
                          "namespace y {\n"
                          "void f() {\n"
@@ -2038,7 +1983,6 @@
                          "typedef ns::Y TY;\n"
                          "using UX = ns::X;\n"
                          "using UY = ns::Y;\n"
-                         "\n"
                          "namespace x {\n"
                          "namespace y {\n"
                          "void f() {\n"
@@ -2067,7 +2011,6 @@
                      "} // namespace na\n";
   std::string Expected = "namespace na {\n"
                          "struct X { enum E { E1 }; };\n"
-                         "\n"
                          "} // namespace na\n"
                          "namespace x {\n"
                          "namespace y {\n"
@@ -2103,7 +2046,6 @@
                      "} // namespace na\n";
   std::string Expected = "namespace na {\n"
                          "struct X {};\n"
-                         "\n"
                          "} // namespace na\n"
                          "namespace x {\n"
                          "namespace y {\n"
@@ -2169,7 +2111,6 @@
                          "  int ref_;\n"
                          "};\n"
                          "} // namespace na\n"
-                         "\n"
                          "namespace x {\n"
                          "namespace y {\n"
                          "class A {\n"
@@ -2225,7 +2166,6 @@
                          "  }\n"
                          "};\n"
                          "}  // namespace a\n"
-                         "\n"
                          "namespace e {\n"
                          "class D : public a::Base<D> {\n"
                          " private:\n"
@@ -2263,7 +2203,6 @@
       "namespace x { namespace y {namespace base { class Base {}; } } }\n"
       "namespace util { class Status {}; }\n"
       "namespace base { class Base {}; }\n"
-      "\n"
       "namespace x {\n"
       "namespace y {\n"
       "void f() {\n"
Index: clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp
@@ -94,7 +94,6 @@
   i = 11;
   // CHECK-MESSAGES: :[[@LINE-4]]:7: warning: {{.*}} in if statement condition
   // CHECK-FIXES:      {{^  i = 8;$}}
-  // CHECK-FIXES-NEXT: {{^  $}}
   // CHECK-FIXES-NEXT: {{^  i = 11;$}}
 }
 
Index: clang-tools-extra/test/clang-tidy/checkers/readability-redundant-member-init.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/readability-redundant-member-init.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-redundant-member-init.cpp
@@ -120,6 +120,34 @@
   };
 };
 
+// Multiple members, removing them does not leave blank lines
+struct F10 {
+  F10() :
+    f(),
+    g(),
+    h()
+  {}
+  // CHECK-MESSAGES: :[[@LINE-4]]:5: warning: initializer for member 'f' is redundant
+  // CHECK-MESSAGES: :[[@LINE-4]]:5: warning: initializer for member 'g' is redundant
+  // CHECK-MESSAGES: :[[@LINE-4]]:5: warning: initializer for member 'h' is redundant
+  // CHECK-FIXES: F10()
+  // CHECK-FIXES-NEXT: {{^}}  {}{{$}}
+
+  S f, g, h;
+};
+
+// Constructor outside of class, remove redundant initializer leaving no blank line
+struct F11 {
+  F11();
+  S a;
+};
+F11::F11()
+: a()
+{}
+// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: initializer for member 'a' is redundant
+// CHECK-FIXES: {{^}}F11::F11(){{$}}
+// CHECK-FIXES-NEXT: {{^}}{}{{$}}
+
 // struct whose inline copy constructor default-initializes its base class
 struct WithCopyConstructor1 : public T {
   WithCopyConstructor1(const WithCopyConstructor1& other) : T(),
Index: clang-tools-extra/test/clang-tidy/checkers/readability-redundant-declaration.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/readability-redundant-declaration.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-redundant-declaration.cpp
@@ -94,3 +94,10 @@
 // CHECK-MESSAGES-NOMSCOMPAT: :[[@LINE-1]]:20: warning: redundant 'g' declaration
 // CHECK-FIXES-NOMSCOMPAT: {{^}}// extern g{{$}}
 #endif
+
+// Test that entire next line is removed.
+inline void g();
+// Test that entire previous line is removed.
+// CHECK-MESSAGES: :[[@LINE-2]]:13: warning: redundant 'g' declaration
+// CHECK-FIXES: {{^}}// Test that entire next line is removed.{{$}}
+// CHECK-FIXES-NEXT: {{^}}// Test that entire previous line is removed.{{$}}
Index: clang-tools-extra/test/clang-tidy/checkers/readability-redundant-control-flow.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/readability-redundant-control-flow.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-redundant-control-flow.cpp
@@ -10,6 +10,15 @@
 // CHECK-FIXES: {{^}}void f() {{{$}}
 // CHECK-FIXES-NEXT: {{^ *}$}}
 
+// Don't pull function closing brace up with comment as line is not blank.
+void f_with_note() {
+  /* NOTE */ return;
+}
+// CHECK-MESSAGES: :[[@LINE-2]]:14: warning: redundant return statement at the end of a function with a void return type [readability-redundant-control-flow]
+// CHECK-FIXES: {{^}}void f_with_note() {{{$}}
+// CHECK-FIXES-NEXT: {{^}}  /* NOTE */ {{$}}
+// CHECK-FIXES-NEXT: {{^}}}{{$}}
+
 void g() {
   f();
   return;
@@ -214,6 +223,18 @@
 // CHECK-FIXES: {{^}}  for (int i = 0; i < end; ++i) {{{$}}
 // CHECK-FIXES-NEXT: {{^ *}$}}
 
+// Don't pull loop closing brace up with comment as line is not blank.
+template <typename T>
+void template_loop_with_note(T end) {
+  for (T i = 0; i < end; ++i) {
+    /* NOTE */ continue;
+  }
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:16: warning: redundant continue statement
+// CHECK-FIXES: {{^}}  for (T i = 0; i < end; ++i) {{{$}}
+// CHECK-FIXES-NEXT: {{^}}    /* NOTE */ {{$}}
+// CHECK-FIXES-NEXT: {{^}}  }{{$}}
+
 void call_templates() {
   template_return(10);
   template_return(10.0f);
@@ -221,4 +242,5 @@
   template_loop(10);
   template_loop(10L);
   template_loop(10U);
+  template_loop_with_note(10U);
 }
Index: clang-tools-extra/test/clang-tidy/checkers/modernize-redundant-void-arg.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/modernize-redundant-void-arg.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-redundant-void-arg.cpp
@@ -195,7 +195,6 @@
 // CHECK-MESSAGES: :[[@LINE-2]]:9: warning: {{.*}} in typedef
 // CHECK-FIXES:      {{^typedef void \(function_ptr2\)$}}
 // CHECK-FIXES-NEXT: {{^    \($}}
-// CHECK-FIXES-NEXT: {{^        $}}
 // CHECK-FIXES-NEXT: {{^    \);$}}
 
 // intentionally not LLVM style to check preservation of whitesapce
@@ -262,7 +261,6 @@
 // CHECK-MESSAGES: :[[@LINE-2]]:9: warning: {{.*}} in typedef
 // CHECK-FIXES:      {{^typedef void \(gronk::\*member_function_ptr2\)$}}
 // CHECK-FIXES-NEXT: {{^    \($}}
-// CHECK-FIXES-NEXT: {{^        $}}
 // CHECK-FIXES-NEXT: {{^    \);$}}
 
 void gronk::foo() {
@@ -282,7 +280,6 @@
   // CHECK-MESSAGES: :[[@LINE-2]]:11: warning: {{.*}} in variable declaration
   // CHECK-FIXES:      {{^  }}void (*f3){{$}}
   // CHECK-FIXES-NEXT: {{^      \($}}
-  // CHECK-FIXES-NEXT: {{^          $}}
   // CHECK-FIXES-NEXT: {{^      \);$}}
 }
 
@@ -305,7 +302,6 @@
   // CHECK-MESSAGES: :[[@LINE-2]]:11: warning: {{.*}} in variable declaration
   // CHECK-FIXES:      {{^  }}void (gronk::*p5){{$}}
   // CHECK-FIXES-NEXT: {{^      \($}}
-  // CHECK-FIXES-NExT: {{^          $}}
   // CHECK-FIXES-NExT: {{^      \);$}}
 }
 
@@ -317,7 +313,6 @@
 // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: {{.*}} in function definition
 // CHECK-FIXES:      {{^void gronk::bar2$}}
 // CHECK-FIXES-NEXT: {{^  \($}}
-// CHECK-FIXES-NEXT: {{^  $}}
 // CHECK-FIXES-NEXT: {{^  \)$}}
 {
 }
@@ -366,7 +361,6 @@
   // CHECK-MESSAGES: :[[@LINE-3]]:11: warning: {{.*}} in named cast
   // CHECK-FIXES:      {{^  }}void (*f6)() = static_cast<void (*){{$}}
   // CHECK-FIXES-NEXT: {{^      \($}}
-  // CHECK-FIXES-NEXT: {{^          $}}
   // CHECK-FIXES-NEXT: {{^      }})>(0);{{$}}
 
   // intentionally not LLVM style to check preservation of whitesapce
@@ -378,7 +372,6 @@
   // CHECK-MESSAGES: :[[@LINE-3]]:11: warning: {{.*}} in cast expression
   // CHECK-FIXES:      {{^  }}void (*f7)() = (void (*){{$}}
   // CHECK-FIXES-NEXT: {{^      \($}}
-  // CHECK-FIXES-NEXT: {{^          $}}
   // CHECK-FIXES-NEXT: {{^      \)\) 0;$}}
 
   // intentionally not LLVM style to check preservation of whitesapce
@@ -390,7 +383,6 @@
   // CHECK-MESSAGES: :[[@LINE-3]]:11: warning: {{.*}} in named cast
   // CHECK-FIXES:      {{^  }}void (*f8)() = reinterpret_cast<void (*){{$}}
   // CHECK-FIXES-NEXT: {{^      \($}}
-  // CHECK-FIXES-NEXT: {{^          $}}
   // CHECK-FIXES-NEXT: {{^      \)>\(0\);$}}
 
   void (*o1)(int) = static_cast<void (*)(int)>(0);
Index: clang-tools-extra/clang-tidy/readability/RedundantControlFlowCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/RedundantControlFlowCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/RedundantControlFlowCheck.cpp
@@ -81,13 +81,13 @@
   if (Previous != Block->body_rend())
     Start = Lexer::findLocationAfterToken(
         dyn_cast<Stmt>(*Previous)->getEndLoc(), tok::semi, SM, getLangOpts(),
-        /*SkipTrailingWhitespaceAndNewLine=*/true);
+        /*SkipTrailingWhitespaceAndNewLine=*/false);
   if (!Start.isValid())
     Start = StmtRange.getBegin();
   auto RemovedRange = CharSourceRange::getCharRange(
       Start, Lexer::findLocationAfterToken(
                  StmtRange.getEnd(), tok::semi, SM, getLangOpts(),
-                 /*SkipTrailingWhitespaceAndNewLine=*/true));
+                 /*SkipTrailingWhitespaceAndNewLine=*/false));
 
   diag(StmtRange.getBegin(), Diag) << FixItHint::CreateRemoval(RemovedRange);
 }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to