This revision was automatically updated to reflect the committed changes.
Closed by commit rG7c90666d2c3c: [clang-tidy] readability-redundant-string-init
now flags redundant… (authored by njames93).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72448/new/
https://reviews.llvm.org/D72448
Files:
clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
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
@@ -34,6 +34,12 @@
std::string d(R"()");
// CHECK-MESSAGES: [[@LINE-1]]:15: warning: redundant string initialization
// CHECK-FIXES: std::string d;
+ std::string e{""};
+ // CHECK-MESSAGES: [[@LINE-1]]:15: warning: redundant string initialization
+ // CHECK-FIXES: std::string e;
+ std::string f = {""};
+ // CHECK-MESSAGES: [[@LINE-1]]:15: warning: redundant string initialization
+ // CHECK-FIXES: std::string f;
std::string u = "u";
std::string w("w");
@@ -227,3 +233,53 @@
other::wstring e = L"";
other::wstring f(L"");
}
+
+class Foo {
+ std::string A = "";
+ // CHECK-MESSAGES: [[@LINE-1]]:15: warning: redundant string initialization
+ // CHECK-FIXES: std::string A;
+ std::string B;
+ std::string C;
+ std::string D;
+ std::string E = "NotEmpty";
+
+public:
+ // Check redundant constructor where Field has a redundant initializer.
+ Foo() : A("") {}
+ // CHECK-MESSAGES: [[@LINE-1]]:11: warning: redundant string initialization
+ // CHECK-FIXES: Foo() {}
+
+ // Check redundant constructor where Field has no initializer.
+ Foo(char) : B("") {}
+ // CHECK-MESSAGES: [[@LINE-1]]:15: warning: redundant string initialization
+ // CHECK-FIXES: Foo(char) {}
+
+ // Check redundant constructor where Field has a valid initializer.
+ Foo(long) : E("") {}
+ // CHECK-MESSAGES: [[@LINE-1]]:15: warning: redundant string initialization
+ // CHECK-FIXES: Foo(long) : E() {}
+
+ // Check how it handles removing 1 initializer, and defaulting the other.
+ Foo(int) : B(""), E("") {}
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: redundant string initialization
+ // CHECK-MESSAGES: [[@LINE-2]]:21: warning: redundant string initialization
+ // CHECK-FIXES: Foo(int) : E() {}
+
+ Foo(short) : B{""} {}
+ // CHECK-MESSAGES: [[@LINE-1]]:16: warning: redundant string initialization
+ // CHECK-FIXES: Foo(short) {}
+
+ Foo(float) : A{""}, B{""} {}
+ // CHECK-MESSAGES: [[@LINE-1]]:16: warning: redundant string initialization
+ // CHECK-MESSAGES: [[@LINE-2]]:23: warning: redundant string initialization
+ // CHECK-FIXES: Foo(float) {}
+
+
+ // Check how it handles removing some redundant initializers while leaving
+ // valid initializers intact.
+ Foo(std::string Arg) : A(Arg), B(""), C("NonEmpty"), D(R"()"), E("") {}
+ // CHECK-MESSAGES: [[@LINE-1]]:34: warning: redundant string initialization
+ // CHECK-MESSAGES: [[@LINE-2]]:56: warning: redundant string initialization
+ // CHECK-MESSAGES: [[@LINE-3]]:66: warning: redundant string initialization
+ // CHECK-FIXES: Foo(std::string Arg) : A(Arg), C("NonEmpty"), E() {}
+};
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -104,6 +104,11 @@
Changes in existing checks
^^^^^^^^^^^^^^^^^^^^^^^^^^
+- Improved :doc:`readability-redundant-string-init
+ <clang-tidy/checks/readability-redundant-string-init>` check now supports a
+ `StringNames` option enabling its application to custom string classes. The
+ check now detects in class initializers and constructor initializers which
+ are deemed to be redundant.
Renamed checks
^^^^^^^^^^^^^^
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
@@ -20,6 +20,46 @@
const char DefaultStringNames[] = "::std::basic_string";
+static ast_matchers::internal::Matcher<NamedDecl>
+hasAnyNameStdString(std::vector<std::string> Names) {
+ return ast_matchers::internal::Matcher<NamedDecl>(
+ new ast_matchers::internal::HasNameMatcher(std::move(Names)));
+}
+
+static std::vector<std::string>
+removeNamespaces(const std::vector<std::string> &Names) {
+ std::vector<std::string> Result;
+ Result.reserve(Names.size());
+ for (const std::string &Name : Names) {
+ std::string::size_type ColonPos = Name.rfind(':');
+ Result.push_back(
+ Name.substr(ColonPos == std::string::npos ? 0 : ColonPos + 1));
+ }
+ return Result;
+}
+
+static const CXXConstructExpr *
+getConstructExpr(const CXXCtorInitializer &CtorInit) {
+ const Expr *InitExpr = CtorInit.getInit();
+ if (const auto *CleanUpExpr = dyn_cast<ExprWithCleanups>(InitExpr))
+ InitExpr = CleanUpExpr->getSubExpr();
+ return dyn_cast<CXXConstructExpr>(InitExpr);
+}
+
+static llvm::Optional<SourceRange>
+getConstructExprArgRange(const CXXConstructExpr &Construct) {
+ SourceLocation B, E;
+ for (const Expr *Arg : Construct.arguments()) {
+ if (B.isInvalid())
+ B = Arg->getBeginLoc();
+ if (Arg->getEndLoc().isValid())
+ E = Arg->getEndLoc();
+ }
+ if (B.isInvalid() || E.isInvalid())
+ return llvm::None;
+ return SourceRange(B, E);
+}
+
RedundantStringInitCheck::RedundantStringInitCheck(StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
@@ -33,18 +73,9 @@
void RedundantStringInitCheck::registerMatchers(MatchFinder *Finder) {
if (!getLangOpts().CPlusPlus)
return;
- const auto hasStringTypeName = hasAnyName(
- SmallVector<StringRef, 3>(StringNames.begin(), StringNames.end()));
-
- // Version of StringNames with namespaces removed
- std::vector<std::string> stringNamesNoNamespace;
- for (const std::string &name : StringNames) {
- std::string::size_type colonPos = name.rfind(':');
- stringNamesNoNamespace.push_back(
- name.substr(colonPos == std::string::npos ? 0 : colonPos + 1));
- }
- const auto hasStringCtorName = hasAnyName(SmallVector<StringRef, 3>(
- stringNamesNoNamespace.begin(), stringNamesNoNamespace.end()));
+ const auto hasStringTypeName = hasAnyNameStdString(StringNames);
+ const auto hasStringCtorName =
+ hasAnyNameStdString(removeNamespaces(StringNames));
// Match string constructor.
const auto StringConstructorExpr = expr(
@@ -65,29 +96,76 @@
cxxConstructExpr(StringConstructorExpr,
hasArgument(0, ignoringImplicit(EmptyStringCtorExpr)));
+ const auto StringType = hasType(hasUnqualifiedDesugaredType(
+ recordType(hasDeclaration(cxxRecordDecl(hasStringTypeName)))));
+ const auto EmptyStringInit = expr(ignoringImplicit(
+ anyOf(EmptyStringCtorExpr, EmptyStringCtorExprWithTemporaries)));
+
// Match a variable declaration with an empty string literal as initializer.
// Examples:
// string foo = "";
// string bar("");
Finder->addMatcher(
namedDecl(
- varDecl(
- hasType(hasUnqualifiedDesugaredType(recordType(
- hasDeclaration(cxxRecordDecl(hasStringTypeName))))),
- hasInitializer(expr(ignoringImplicit(anyOf(
- EmptyStringCtorExpr, EmptyStringCtorExprWithTemporaries)))))
- .bind("vardecl"),
+ varDecl(StringType, hasInitializer(EmptyStringInit)).bind("vardecl"),
unless(parmVarDecl())),
this);
+ // Match a field declaration with an empty string literal as initializer.
+ Finder->addMatcher(
+ namedDecl(fieldDecl(StringType, hasInClassInitializer(EmptyStringInit))
+ .bind("fieldDecl")),
+ this);
+ // Matches Constructor Initializers with an empty string literal as
+ // initializer.
+ // Examples:
+ // Foo() : SomeString("") {}
+ Finder->addMatcher(
+ cxxCtorInitializer(
+ isWritten(),
+ forField(allOf(StringType, optionally(hasInClassInitializer(
+ EmptyStringInit.bind("empty_init"))))),
+ withInitializer(EmptyStringInit))
+ .bind("ctorInit"),
+ this);
}
void RedundantStringInitCheck::check(const MatchFinder::MatchResult &Result) {
- const auto *VDecl = Result.Nodes.getNodeAs<VarDecl>("vardecl");
- // VarDecl's getSourceRange() spans 'string foo = ""' or 'string bar("")'.
- // So start at getLocation() to span just 'foo = ""' or 'bar("")'.
- SourceRange ReplaceRange(VDecl->getLocation(), VDecl->getEndLoc());
- diag(VDecl->getLocation(), "redundant string initialization")
- << FixItHint::CreateReplacement(ReplaceRange, VDecl->getName());
+ if (const auto *VDecl = Result.Nodes.getNodeAs<VarDecl>("vardecl")) {
+ // VarDecl's getSourceRange() spans 'string foo = ""' or 'string bar("")'.
+ // So start at getLocation() to span just 'foo = ""' or 'bar("")'.
+ SourceRange ReplaceRange(VDecl->getLocation(), VDecl->getEndLoc());
+ diag(VDecl->getLocation(), "redundant string initialization")
+ << FixItHint::CreateReplacement(ReplaceRange, VDecl->getName());
+ }
+ if (const auto *FDecl = Result.Nodes.getNodeAs<FieldDecl>("fieldDecl")) {
+ // FieldDecl's getSourceRange() spans 'string foo = ""'.
+ // So start at getLocation() to span just 'foo = ""'.
+ SourceRange ReplaceRange(FDecl->getLocation(), FDecl->getEndLoc());
+ diag(FDecl->getLocation(), "redundant string initialization")
+ << FixItHint::CreateReplacement(ReplaceRange, FDecl->getName());
+ }
+ if (const auto *CtorInit =
+ Result.Nodes.getNodeAs<CXXCtorInitializer>("ctorInit")) {
+ if (const FieldDecl *Member = CtorInit->getMember()) {
+ if (!Member->hasInClassInitializer() ||
+ Result.Nodes.getNodeAs<Expr>("empty_init")) {
+ // The String isn't declared in the class with an initializer or its
+ // declared with a redundant initializer, which will be removed. Either
+ // way the string will be default initialized, therefore we can remove
+ // the constructor initializer entirely.
+ diag(CtorInit->getMemberLocation(), "redundant string initialization")
+ << FixItHint::CreateRemoval(CtorInit->getSourceRange());
+ return;
+ }
+ }
+ const CXXConstructExpr *Construct = getConstructExpr(*CtorInit);
+ if (!Construct)
+ return;
+ if (llvm::Optional<SourceRange> RemovalRange =
+ getConstructExprArgRange(*Construct))
+ diag(CtorInit->getMemberLocation(), "redundant string initialization")
+ << FixItHint::CreateRemoval(*RemovalRange);
+ }
}
} // namespace readability
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
