njames93 updated this revision to Diff 238764. njames93 added a comment. - Updated to trunk, fix release notes
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 @@ -91,6 +91,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,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 cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits