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(*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/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 whitespace @@ -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(0);{{$}} // intentionally not LLVM style to check preservation of whitespace @@ -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 whitespace @@ -390,7 +383,6 @@ // CHECK-MESSAGES: :[[@LINE-3]]:11: warning: {{.*}} in named cast // CHECK-FIXES: {{^ }}void (*f8)() = reinterpret_cast\(0\);$}} void (*o1)(int) = static_cast(0); 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 +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/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-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-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;$}} } @@ -134,7 +133,6 @@ i = 20; // CHECK-MESSAGES: :[[@LINE-4]]:7: warning: {{.*}} in if statement condition // CHECK-FIXES: {{^ i = 18;$}} - // CHECK-FIXES-NEXT: {{^ $}} // CHECK-FIXES-NEXT: {{^ i = 20;$}} } 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 \n" "class A { T t; };\n" "} // namespace na\n" - "\n" "namespace nc {\n" "using ::na::A;\n" "void d() { A a; }\n" @@ -1403,7 +1364,6 @@ "template \n" "void f() { T t; };\n" "} // namespace na\n" - "\n" "namespace nc {\n" "using ::na::f;\n" "void d() { f(); }\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 {\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/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/include/clang/Format/Format.h =================================================================== --- clang/include/clang/Format/Format.h +++ clang/include/clang/Format/Format.h @@ -3279,6 +3279,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 +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/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/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/Format/Format.cpp =================================================================== --- clang/lib/Format/Format.cpp +++ clang/lib/Format/Format.cpp @@ -26,6 +26,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" @@ -2670,8 +2671,126 @@ return Result; } +// Find the end of the line (\0, \r, or \n) containing StartPos. +int findLineEnd(StringRef Code, int StartPos) { + // When next character is '\0', '\n', or '\r', StartPos is end of line. + assert(StartPos >= 0); + assert(StartPos <= Code.size()); + while (StartPos < Code.size() && !isVerticalWhitespace(Code[StartPos])) { + ++StartPos; + } + assert(StartPos <= Code.size()); + return StartPos; +} + +// Pre-requisites: +// - All Replacements on the line starting at LineStartPos have been processed. +// - All characters up to LineCheckedThroughPos so far are known to be blank. +// This function then checks whether (a) the rest of the line is blank, and (b) +// the original line was *not* blank. If so, it adds a (likely overlapping) +// Replacement to the existing NewReplaces to remove the entire line. +llvm::Error maybeRemoveBlankLine(tooling::Replacements &NewReplaces, + StringRef FilePath, StringRef Code, + int LineStartPos, int CheckedThroughPos) { + assert(CheckedThroughPos >= 0); + int LineEndPos = findLineEnd(Code, CheckedThroughPos); + assert(LineEndPos <= Code.size()); + assert(LineEndPos >= CheckedThroughPos); + assert(LineEndPos >= LineStartPos); + + assert(LineStartPos >= 0); + assert(LineStartPos < Code.size()); + assert(LineStartPos + LineEndPos - LineStartPos <= Code.size()); + assert(CheckedThroughPos <= Code.size()); + // If original is not all whitespace, but here to end is whitespace, remove. + if (!isAllWhitespace(Code.substr(LineStartPos, LineEndPos - LineStartPos)) && + isAllWhitespace( + Code.substr(CheckedThroughPos, LineEndPos - CheckedThroughPos))) { + // Make replacement cut entire line including the subsequent newline, + // unless replacement already removes a trailing newline. (E.g. given + // " foo\n\n..." and a replacement removing "foo\n", remove leading " ", + // but don't *also* remove the second "\n" as it's on a *subsequent* line.) + int CutCount = + (CheckedThroughPos > 0 && + isVerticalWhitespace(Code[CheckedThroughPos - 1])) + ? (LineEndPos - LineStartPos) + : (skipNewlineChars(Code.data() + LineEndPos, Code.end()) - + Code.data() - LineStartPos); + if (llvm::Error Err = NewReplaces.add( + tooling::Replacement(FilePath, LineStartPos, CutCount, ""))) + return std::move(Err); + } + return llvm::Error::success(); +} + +// Find the beginning of the line containing StartPos. +int findLineStart(StringRef Code, int StartPos) { + // When preceding character is '\n' or '\r', StartPos is start of line. + assert(StartPos >= 0); + assert(StartPos <= Code.size()); + while (StartPos > 0 && !isVerticalWhitespace(Code[StartPos - 1])) { + --StartPos; + } + assert(StartPos >= 0); + return StartPos; +} + } // anonymous namespace +llvm::Expected +removeNewlyBlankLines(StringRef Code, const tooling::Replacements &Replaces) { + tooling::Replacements NewReplaces(Replaces); + if (Replaces.empty()) + return NewReplaces; + + StringRef FilePath = Replaces.begin()->getFilePath(); + // These variables track how far we've checked on the current line and detect + // when we move to a new line. + int LineStartPos = 0; + int LineCheckedThroughPos = 0; + bool LineBlankSoFar = true; + for (const auto &R : Replaces) { + assert(FilePath == R.getFilePath()); + const int RStartPos = R.getOffset(); + + // Find the beginning of the line this replacement starts on. + assert(RStartPos >= 0); + int CurrentRLineStartPos = findLineStart(Code, RStartPos); + + assert(CurrentRLineStartPos >= LineStartPos); + if (CurrentRLineStartPos != LineStartPos) { + // R starts on a different line, so process prior line before moving on. + if (LineBlankSoFar) { + if (llvm::Error Err = + maybeRemoveBlankLine(NewReplaces, FilePath, Code, LineStartPos, + LineCheckedThroughPos)) + return std::move(Err); + } + LineCheckedThroughPos = CurrentRLineStartPos; + LineStartPos = CurrentRLineStartPos; + LineBlankSoFar = true; + } + + // Check to see if line from LineCheckedThroughPos to here is blank. + assert(LineCheckedThroughPos >= 0); + assert(RStartPos >= LineCheckedThroughPos); + StringRef PriorTextToCheck( + Code.substr(LineCheckedThroughPos, RStartPos - LineCheckedThroughPos)); + LineBlankSoFar = LineBlankSoFar && isAllWhitespace(PriorTextToCheck) && + R.getReplacementText().empty(); + LineCheckedThroughPos = RStartPos + R.getLength(); + } + + // Process the line of the last Replacement. + if (LineBlankSoFar) { + if (llvm::Error Err = maybeRemoveBlankLine( + NewReplaces, FilePath, Code, LineStartPos, LineCheckedThroughPos)) + return std::move(Err); + } + + return NewReplaces; +} + llvm::Expected cleanupAroundReplacements(StringRef Code, const tooling::Replacements &Replaces, const FormatStyle &Style) { @@ -2685,7 +2804,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 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/unittests/Format/CMakeLists.txt =================================================================== --- clang/unittests/Format/CMakeLists.txt +++ clang/unittests/Format/CMakeLists.txt @@ -1,5 +1,6 @@ set(LLVM_LINK_COMPONENTS Support + TestingSupport ) add_clang_unittest(FormatTests Index: clang/unittests/Format/CleanupTest.cpp =================================================================== --- clang/unittests/Format/CleanupTest.cpp +++ clang/unittests/Format/CleanupTest.cpp @@ -12,6 +12,7 @@ #include "clang/Tooling/Core/Replacement.h" #include "gtest/gtest.h" +#include "llvm/Testing/Support/Annotations.h" using clang::tooling::ReplacementTest; using clang::tooling::toReplacements; @@ -320,6 +321,11 @@ return tooling::Replacement(FileName, Offset, Length, Text); } + tooling::Replacement createReplacement(llvm::Annotations::Range Range, + StringRef Text) { + return createReplacement(Range.Begin, Range.End - Range.Begin, Text); + } + tooling::Replacement createInsertion(StringRef IncludeDirective) { return createReplacement(UINT_MAX, 0, IncludeDirective); } @@ -373,10 +379,12 @@ "namespace C {\n" "namespace D { int i; }\n" "inline namespace E { namespace { int y; } }\n" + "\n" "int x= 0;" "}"; - std::string Expected = "\n\nnamespace C {\n" - "namespace D { int i; }\n\n" + std::string Expected = "\nnamespace C {\n" + "namespace D { int i; }\n" + "\n" "int x= 0;" "}"; tooling::Replacements Replaces = @@ -386,6 +394,104 @@ EXPECT_EQ(Expected, formatAndApply(Code, Replaces)); } +TEST_F(CleanUpReplacementsTest, RemoveLineWhenAllNonWhitespaceRemoved) { + llvm::Annotations Code = "namespace A {$r1[[ // Useless comment]]\n" + " $r2[[int]] $r3[[x]]\t $r4[[=]] $r5[[0;]]\t\n" + " int y\t = 0;$r6[[\t]]\n" + "} // namespace A\n"; + std::string Expected = "namespace A {\n" + " int y\t = 0;\n" + "} // namespace A\n"; + tooling::Replacements Replaces = + toReplacements({createReplacement(Code.range("r1"), ""), + createReplacement(Code.range("r2"), ""), + createReplacement(Code.range("r3"), ""), + createReplacement(Code.range("r4"), ""), + createReplacement(Code.range("r5"), ""), + createReplacement(Code.range("r6"), "")}); + + EXPECT_EQ(Expected, apply(Code.code(), Replaces)); +} + +TEST_F(CleanUpReplacementsTest, RemoveLinesWhenAllNonWhitespaceRemoved) { + llvm::Annotations Code = R"cpp(struct A { + A() + $r3[[:]] $r1[[f()]]$r2[[,]] + $r4[[g()]] + {} + int f = 0; + int g = 0; +};)cpp"; + std::string Expected = R"cpp(struct A { + A() + {} + int f = 0; + int g = 0; +};)cpp"; + tooling::Replacements Replaces = + toReplacements({createReplacement(Code.range("r1"), ""), + createReplacement(Code.range("r2"), ""), + createReplacement(Code.range("r3"), ""), + createReplacement(Code.range("r4"), "")}); + + EXPECT_EQ(Expected, apply(Code.code(), Replaces)); +} + +TEST_F(CleanUpReplacementsTest, KeepLinesWithInsertsOrReplacesEvenIfBlank) { + // Not using raw string literals so newlines and spaces are clear and explicit + llvm::Annotations Code = "struct A {\n" + " A() {}\n" + "$r3[[]] $r1[[int]] $r2[[f;]]\n" // " int f;\n" + " \n" + "$r4[[ ]]\n" + "};"; + std::string Expected = "struct A {\n" + " A() {}\n" + "\n" + " \n" + " \n" + "\t\n" + "};"; + tooling::Replacements Replaces = + toReplacements({createReplacement(Code.range("r1"), " "), + createReplacement(Code.range("r2"), " "), + createReplacement(Code.range("r3"), "\n"), + createReplacement(Code.range("r4"), "\t")}); + + EXPECT_EQ(Expected, apply(Code.code(), Replaces)); +} + +TEST_F(CleanUpReplacementsTest, KeepPreviouslyBlankLinesAfterPartialRemoval) { + // Not using raw string literals so newlines and spaces are clear and explicit + llvm::Annotations Code = " $r1[[ ]] \n" + "\t$r2[[\t]]\n" + "$r3[[\n]]"; + std::string Expected = " \n" + "\t\n" + ""; + tooling::Replacements Replaces = + toReplacements({createReplacement(Code.range("r1"), ""), + createReplacement(Code.range("r2"), ""), + createReplacement(Code.range("r3"), "")}); + + EXPECT_EQ(Expected, apply(Code.code(), Replaces)); +} + +TEST_F(CleanUpReplacementsTest, RemoveLineAndNewlineLineButNotNext) { + // Not using raw string literals so newlines and spaces are clear and explicit + llvm::Annotations Code = + "$r1[[#include]] $r2[[\"a\".h\n]]" // Removal removes newline char + "\n" + "struct foo;\n"; + std::string Expected = "\n" + "struct foo;\n"; + tooling::Replacements Replaces = + toReplacements({createReplacement(Code.range("r1"), ""), + createReplacement(Code.range("r2"), "")}); + + EXPECT_EQ(Expected, apply(Code.code(), Replaces)); +} + TEST_F(CleanUpReplacementsTest, InsertMultipleIncludesLLVMStyle) { std::string Code = "#include \"x/fix.h\"\n" "#include \"a.h\"\n"