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