This change benefits clang-format, clang-tidy, clang-change-namespace, and others. When a sequence of Removal Replacements (excluding Insertions or non-empty Replacements) leaves a previously non-blank line with nothing but whitespace, this patch adds a Removal to remove the entire line.
The initial impetus for this change was clang-tidy, which usually can safely ignore formatting concerns as formatting is clang-format's job. However, clang-format treats blank lines as intentional separators, so any clang-tidy fixes that remove all text from lines and leave newly blank lines can prevent clang-format from properly fixing formatting. For example, applying readability-redundant-member-init to:
struct F10 {
F10() :
f(),
g()
{}
S f, g;
};results in:
struct F10 {
F10()
{}
S f, g;
};which may get converted to:
struct F10 {
F10()
= default;
S f, g;
};The newly blank lines prevent clang-format from fixing this to the desired:
struct F10 {
F10() = default;
S f, g;
};A few individual clang-tidy fixes take steps to reduce some newly blank lines, e.g. RedundantControlFlowCheck.cpp calls Lexer::findLocationAfterToken(.../*SkipTrailingWhitespaceAndNewLine=*/true ) to chomp trailing newlines after redundant continue; or break; statements. But relying on each check to individually address the problem:
- does not check whether the whole line is blank before removing newlines, as that can only be known once all Replacements on the line are known
- can fail to notice when multiple removals combine to generate a blank line
- forces more work for each check writer
- results in inconsistency between checks regarding removals creating newly blank lines
For example:
F11::F11()
: a(), b()
{}is fixed by readability-redundant-member-init with independent removals to remove the "a()" and "b()" and the ":" is finally removed in cleanupAroundReplacements, so only there can the code know the result will be blank.
This patch adds a function removeNewlyBlankLines to Format.cpp that looks for sequences of Replacement removals that leave a previously non-blank line blank and, where found, adds a Replacement to remove the whole line. removeNewlyBlankLines is called from cleanupAroundReplacements().
The patch adds tests to clang/unittests/Format/CleanupTest.cpp and clang-tools-extra/test/checkers/readability-redundant-{member-init,control-flow,declaration}.cpp, plus updates numerous tests in ChangeNamespaceTests.cpp and elsewhere to reflect the improved line-removing behavior.
Two previously private/static helper functions were moved to clang/base/CharInfo.h for use in the implementation: AST/CommentLexer's skipNewline and AST/CommentParser's isWhitespace(StringRef) (the former was also renamed to skipNewlineChars to avoid a name clash.)
I'd suggest to avoid overloading here. A name like isWhitespaceOnly would be less ambiguous.
As for implementation, it can be less verbose:
for (char C : S) if (!isWhitespace(*I)) return false; return true;or just