poelmanc updated this revision to Diff 228795.
poelmanc edited the summary of this revision.
poelmanc added a comment.
Move isWhitespace and skipNewlines to clang/Basic/CharInfo.h (renaming to
isWhitespaceStringRef and skipNewlinesChars to resolve name clashes) and add
double-quotes around "/n" and "/r" in comments.
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/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/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
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"
@@ -2356,6 +2357,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 &&
+ isWhitespaceStringRef(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 (isWhitespaceStringRef(TrailingText) &&
+ !isWhitespaceStringRef(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) {
@@ -2369,7 +2451,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) && isWhitespaceStringRef(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
@@ -2251,6 +2251,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
@@ -193,6 +193,34 @@
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;
+}
+
+/// Returns true if and only if S consists entirely of whitespace.
+LLVM_READONLY inline bool isWhitespaceStringRef(llvm::StringRef S) {
+ for (StringRef::const_iterator I = S.begin(), E = S.end(); I != E; ++I) {
+ if (!isWhitespace(*I))
+ return false;
+ }
+ return true;
+}
+
} // end namespace clang
#endif
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
@@ -116,6 +116,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: {{^}}{}{{$}}
+
// Initializer not written
struct NF1 {
NF1() {}
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/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
