poelmanc updated this revision to Diff 230375.
poelmanc marked 3 inline comments as done.
poelmanc retitled this revision from "Clang-tidy fix removals removing all
non-blank text from a line should remove the line" to
"format::cleanupAroundReplacements removes whole line when Removals leave
previously non-blank line blank".
poelmanc edited the summary of this revision.
poelmanc added a comment.
I addressed the latest code review comments, added tests to
`clang/unittests/Format/CleanupTest.cpp`, and updated numerous tests to reflect
improved removal of blank lines.
I now realize how widely used `cleanupAroundReplacements` is. That's great as
this fix improves `clang-change-namespace` as a side-effect. However, I have
little experience running tests beyond clang-tidy or on Linux (I'm testing with
MSVC) and would appreciate help identifying any test failures or regressions.
Also please check my change to `FixOnlyAffectedCodeAfterReplacements` in
clang/unittests/Format/CleanupTest.cpp.
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-query/QueryParser.cpp
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/AST/CommentSema.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"
@@ -2371,6 +2372,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 &&
+ isWhitespace(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 (isWhitespace(TrailingText) &&
+ !isWhitespace(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) {
@@ -2384,7 +2466,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/CommentSema.cpp
===================================================================
--- clang/lib/AST/CommentSema.cpp
+++ clang/lib/AST/CommentSema.cpp
@@ -233,9 +233,9 @@
if (Direction == -1) {
// Try again with whitespace removed.
- ArgLower.erase(
- std::remove_if(ArgLower.begin(), ArgLower.end(), clang::isWhitespace),
- ArgLower.end());
+ ArgLower.erase(std::remove_if(ArgLower.begin(), ArgLower.end(),
+ [](char c) { return isWhitespace(c); }),
+ ArgLower.end());
Direction = getParamPassDirection(ArgLower);
SourceRange ArgRange(ArgLocBegin, ArgLocEnd);
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.
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
@@ -2274,6 +2274,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,15 @@
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 isWhitespace(llvm::StringRef S) {
+ for (StringRef::const_iterator I = S.begin(), E = S.end(); I != E; ++I) {
+ if (!isWhitespace(*I))
+ return false;
+ }
+ return true;
+}
+
/// Return true if this character is an ASCII digit: [0-9]
LLVM_READONLY inline bool isDigit(unsigned char c) {
using namespace charinfo;
@@ -193,6 +202,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);
}
Index: clang-tools-extra/clang-query/QueryParser.cpp
===================================================================
--- clang-tools-extra/clang-query/QueryParser.cpp
+++ clang-tools-extra/clang-query/QueryParser.cpp
@@ -39,7 +39,7 @@
return StringRef();
}
- StringRef Word = Line.take_until(isWhitespace);
+ StringRef Word = Line.take_until([](char c) { return isWhitespace(c); });
Line = Line.drop_front(Word.size());
return Word;
}
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits