njames93 updated this revision to Diff 237297.
njames93 edited the summary of this revision.
njames93 added a comment.
Checks for string initialisations which are spelt with braces
Ensures the check will detect and fix brace initializations
Example:
std::string a{""};
std::string b = {""};
CxxConstructor : A{""} {}
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
@@ -218,9 +218,11 @@
`"base class 'Foo' should be explicitly initialized in the copy constructor"`
warnings or errors with ``gcc -Wextra`` or ``gcc -Werror=extra``.
-- The :doc:`readability-redundant-string-init
+- 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.
+ `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.
- Improved :doc:`modernize-avoid-bind
<clang-tidy/checks/modernize-avoid-bind>` check.
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,30 @@
const char DefaultStringNames[] = "::std::basic_string";
+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),
@@ -65,29 +89,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 (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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
