poelmanc updated this revision to Diff 226695.
poelmanc added a comment.
Remove extra `const`, braces for one-line `if` statements, and `clang::`. Added
a comment explaining the need for a regex on a warning test line.
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69238/new/
https://reviews.llvm.org/D69238
Files:
clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init-msvc.cpp
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
@@ -1,5 +1,4 @@
-// RUN: %check_clang_tidy -std=c++11,c++14 %s readability-redundant-string-init %t
-// FIXME: Fix the checker to work in C++17 mode.
+// RUN: %check_clang_tidy %s readability-redundant-string-init %t
namespace std {
template <typename T>
@@ -104,7 +103,8 @@
// CHECK-MESSAGES: [[@LINE-1]]:10: warning: redundant string initialization
// CHECK-FIXES: STRING d;
DECL_STRING(e, "");
- // CHECK-MESSAGES: [[@LINE-1]]:15: warning: redundant string initialization
+ // Warning at position :15 with -std=c++11/14, position :18 for -std=c++17/2x
+ // CHECK-MESSAGES: [[@LINE-2]]:1{{[58]}}: warning: redundant string initialization
MyString f = "u";
STRING g = "u";
Index: clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init-msvc.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init-msvc.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init-msvc.cpp
@@ -1,5 +1,4 @@
-// RUN: %check_clang_tidy -std=c++11,c++14 %s readability-redundant-string-init %t
-// FIXME: Fix the checker to work in C++17 mode.
+// RUN: %check_clang_tidy %s readability-redundant-string-init %t
namespace std {
template <typename T>
Index: clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
@@ -57,12 +57,73 @@
this);
}
+// Returns the SourceRange for the string constructor expression and a bool
+// indicating whether to fix it (by replacing it with just the variable name)
+// or just issue a diagnostic warning. CtorExpr's SourceRange includes:
+// foo = ""
+// bar("")
+// "" (only in C++17 and later)
+//
+// With -std c++14 or earlier (!LangOps.CPlusPlus17), it was sufficient to
+// return CtorExpr->getSourceRange(). However, starting with c++17, parsing
+// the expression 'std::string Name = ""' results in a CtorExpr whose
+// SourceRange includes just '""' rather than the previous 'Name = ""'.
+//
+// So if the CtorExpr's SourceRange doesn't already start with 'Name', we
+// search leftward for 'Name\b*=\b*' (where \b* represents optional whitespace)
+// and if found, extend the SourceRange to start at 'Name'.
+std::pair<SourceRange, bool>
+CtorSourceRange(const MatchFinder::MatchResult &Result, const NamedDecl *Decl,
+ const Expr *CtorExpr) {
+ const SourceManager &SM = *Result.SourceManager;
+ StringRef Name = Decl->getName();
+ int NameLength = Name.size();
+ SourceLocation CtorStartLoc(CtorExpr->getSourceRange().getBegin());
+ std::pair<FileID, unsigned> CtorLocInfo = SM.getDecomposedLoc(
+ CtorStartLoc.isMacroID() ? SM.getImmediateMacroCallerLoc(CtorStartLoc)
+ : CtorStartLoc);
+ int CtorStartPos = CtorLocInfo.second;
+ StringRef File = SM.getBufferData(CtorLocInfo.first);
+ StringRef CtorCheckName = File.substr(CtorStartPos, NameLength);
+ if (CtorCheckName == Name)
+ // For 'std::string foo("");', CtorExpr is 'foo("")'
+ return std::make_pair(CtorExpr->getSourceRange(), true);
+ // Scan backwards from CtorStartPos.
+ // If find "Name\b*=\b*", extend CtorExpr SourceRange leftwards to include it.
+ bool FoundEquals = false;
+ for (int Pos = CtorStartPos - 1; Pos >= 0; Pos--) {
+ char Char = File.data()[Pos];
+ if (Char == '=') {
+ if (FoundEquals)
+ break; // Only allow one equals sign
+ FoundEquals = true;
+ } else if (!isWhitespace(Char)) {
+ // First non-whitespace/non-equals char. Check for Name ending with it.
+ int CheckNameStart = Pos - NameLength + 1;
+ StringRef CheckName(File.data() + CheckNameStart, NameLength);
+ if (FoundEquals && Name == CheckName) {
+ return std::make_pair(SourceRange(CtorStartLoc.getLocWithOffset(
+ CheckNameStart - CtorStartPos),
+ CtorExpr->getSourceRange().getEnd()),
+ true);
+ } else {
+ break; // Found something other than 'Name\b*=\b*'
+ }
+ }
+ }
+ // We can't find Name\b*=\b* so to be safe warn but don't fix.
+ return std::make_pair(CtorExpr->getSourceRange(), false);
+}
+
void RedundantStringInitCheck::check(const MatchFinder::MatchResult &Result) {
const auto *CtorExpr = Result.Nodes.getNodeAs<Expr>("expr");
const auto *Decl = Result.Nodes.getNodeAs<NamedDecl>("decl");
- diag(CtorExpr->getExprLoc(), "redundant string initialization")
- << FixItHint::CreateReplacement(CtorExpr->getSourceRange(),
- Decl->getName());
+ std::pair<SourceRange, bool> CtorExprRange =
+ CtorSourceRange(Result, Decl, CtorExpr);
+ DiagnosticBuilder Diag =
+ diag(CtorExprRange.first.getBegin(), "redundant string initialization");
+ if (CtorExprRange.second)
+ Diag << FixItHint::CreateReplacement(CtorExprRange.first, Decl->getName());
}
} // namespace readability
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
