njames93 created this revision.
njames93 added reviewers: aaron.ballman, alexfh, hokein.
njames93 added projects: clang, clang-tools-extra.
Herald added a subscriber: xazax.hun.
njames93 marked an inline comment as done.
njames93 added inline comments.
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp:244
+ // CHECK-MESSAGES: [[@LINE-1]]:11: warning: redundant string initialization
+ // CHECK-FIXES: Foo() {}
+
----------------
I'm aware the extra space between the Foo() and the braces is unsightly, but
its something that clang-format would fix. Same goes for all other lines
The original behaviour of this check only looked at VarDecls with strings that
had an empty string initializer. This has been improved to check for FieldDecls
with an in class initializer as well as constructor initializers.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D72448
Files:
clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.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
@@ -227,3 +227,43 @@
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() {}
+
+ // 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/clang-tidy/readability/RedundantStringInitCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
@@ -20,6 +20,31 @@
const char DefaultStringNames[] = "::std::basic_string";
+namespace {
+const CXXConstructExpr *getConstructExpr(const CXXCtorInitializer &CtorInit) {
+ Expr *InitExpr = CtorInit.getInit();
+ if (auto *CleanUpExpr = dyn_cast<ExprWithCleanups>(InitExpr))
+ InitExpr = CleanUpExpr->getSubExpr();
+ return dyn_cast<CXXConstructExpr>(InitExpr);
+}
+
+llvm::Optional<SourceRange>
+getConstructExprArgRange(const CXXConstructExpr &Construct) {
+ SourceLocation B, E;
+ for (const auto *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);
+}
+} // namespace
+
RedundantStringInitCheck::RedundantStringInitCheck(StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
@@ -65,29 +90,77 @@
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 (auto *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 auto *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
