diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt index 2c5c44db587fe..ad21bfd798648 100644 --- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt @@ -46,6 +46,7 @@ add_clang_library(clangTidyModernizeModule STATIC UseOverrideCheck.cpp UseRangesCheck.cpp UseScopedLockCheck.cpp + UseSharedPtrArrayCheck.cpp UseStartsEndsWithCheck.cpp UseStdBitCheck.cpp UseStdFormatCheck.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp index cc13da7535bcb..ff83b865abaf8 100644 --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -46,6 +46,7 @@ #include "UseOverrideCheck.h" #include "UseRangesCheck.h" #include "UseScopedLockCheck.h" +#include "UseSharedPtrArrayCheck.h" #include "UseStartsEndsWithCheck.h" #include "UseStdBitCheck.h" #include "UseStdFormatCheck.h" @@ -95,6 +96,8 @@ class ModernizeModule : public ClangTidyModule { CheckFactories.registerCheck("modernize-use-ranges"); CheckFactories.registerCheck( "modernize-use-scoped-lock"); + CheckFactories.registerCheck( + "modernize-use-shared-ptr-array"); CheckFactories.registerCheck( "modernize-use-starts-ends-with"); CheckFactories.registerCheck("modernize-use-std-bit"); diff --git a/clang-tools-extra/clang-tidy/modernize/UseSharedPtrArrayCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseSharedPtrArrayCheck.cpp new file mode 100644 index 0000000000000..ab3ffe31053ca --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UseSharedPtrArrayCheck.cpp @@ -0,0 +1,475 @@ +//===--- UseSharedPtrArrayCheck.cpp - clang-tidy --------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "UseSharedPtrArrayCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" +#include "clang/AST/ExprCXX.h" +#include "clang/AST/Stmt.h" +#include "clang/AST/TypeLoc.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::modernize { + +void UseSharedPtrArrayCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + cxxConstructExpr( + + hasType(qualType(hasDeclaration( + classTemplateSpecializationDecl(hasName("::std::shared_ptr"))))), + + argumentCountIs(2), + + hasArgument( + 0, ignoringParenImpCasts(cxxNewExpr(isArray()).bind("newExpr"))), + + anyOf(hasArgument( + 1, ignoringImplicit( + cxxConstructExpr( + hasType(qualType(hasCanonicalType(hasDeclaration( + classTemplateSpecializationDecl( + hasName("::std::default_delete"))))))) + .bind("defaultDelete"))), + + hasArgument( + 1, ignoringImplicit(lambdaExpr().bind("lambdaDeleter"))), + + hasArgument(1, ignoringImplicit(declRefExpr(to( + functionDecl().bind("deleterFunction"))))))) + .bind("sharedPtrCtor"), + this); +} + +// Verifies that a callable body consists of exactly delete[] param; with only +// one parameter. Used for both lambdas and free functions. +static bool bodyIsArrayDeleteOfParam(const Stmt *Body, + const ParmVarDecl *Param) { + const auto *Compound = dyn_cast_or_null(Body); + if (!Compound || Compound->size() != 1) + return false; + + const auto *OnlyExpr = dyn_cast(Compound->body_front()); + if (!OnlyExpr) + return false; + + OnlyExpr = OnlyExpr->IgnoreParenImpCasts(); + + const auto *DelExpr = dyn_cast(OnlyExpr); + if (!DelExpr || !DelExpr->isArrayForm()) + return false; + + const auto *DRE = + dyn_cast(DelExpr->getArgument()->IgnoreParenImpCasts()); + if (!DRE) + return false; + + return DRE->getDecl() == Param; +} + +// Manual parent walk rather than a matcher because implicit +// wrappers obscure assignment contexts. +static const VarDecl *findParentVarDecl(ASTContext &Ctx, const Stmt *S) { + DynTypedNode Node = DynTypedNode::create(*S); + + for (;;) { + auto Parents = Ctx.getParents(Node); + if (Parents.empty()) + return nullptr; + + const Expr *NextExpr = nullptr; + const Stmt *NextStmt = nullptr; + + for (const DynTypedNode &P : Parents) { + if (const auto *VD = P.get()) + return VD; + + // Exprs first preserves semantic structure longer than bare Stmt. + if (!NextExpr) + NextExpr = P.get(); + if (!NextStmt) + NextStmt = P.get(); + } + + if (NextExpr) + Node = DynTypedNode::create(*NextExpr); + else if (NextStmt) + Node = DynTypedNode::create(*NextStmt); + else + return nullptr; + } +} + +// Returns a StringRef into the SourceManager-owned buffer; stable for lifetime +// of the ASTContext. +static StringRef extractWrittenElementType(const TypeSourceInfo *TSI, + SourceManager &SM, + const LangOptions &LO) { + if (!TSI) + return {}; + const TypeLoc TL = TSI->getTypeLoc().getUnqualifiedLoc(); + auto TSTL = TL.getAsAdjusted(); + if (!TSTL || TSTL.getNumArgs() < 1) + return {}; + const TypeSourceInfo *ArgTSI = TSTL.getArgLoc(0).getTypeSourceInfo(); + if (!ArgTSI) + return {}; + const TypeLoc ArgTL = ArgTSI->getTypeLoc(); + const CharSourceRange R = + CharSourceRange::getTokenRange(ArgTL.getBeginLoc(), ArgTL.getEndLoc()); + return Lexer::getSourceText(R, SM, LO); +} + +bool UseSharedPtrArrayCheck::lambdaBodyIsArrayDelete( + const LambdaExpr *Lambda) const { + if (Lambda->capture_size() != 0) + return false; + + const CXXMethodDecl *CallOp = Lambda->getCallOperator(); + if (!CallOp || CallOp->getNumParams() != 1) + return false; + + return bodyIsArrayDeleteOfParam(Lambda->getBody(), CallOp->getParamDecl(0)); +} + +// Returns the QualType of the deleter's pointee, or null if the +// deleter shape is not recognised. +static QualType getDeleterParamPointee(const Expr *DeleterArg) { + const Expr *E = DeleterArg->IgnoreParenImpCasts(); + + if (const auto *L = dyn_cast(E)) { + const CXXMethodDecl *CallOp = L->getCallOperator(); + if (!CallOp || CallOp->getNumParams() != 1) + return {}; + const QualType P = CallOp->getParamDecl(0)->getType(); + if (!P->isPointerType()) + return {}; + return P->getPointeeType(); + } + + if (const auto *DRE = dyn_cast(E)) { + if (const auto *FD = dyn_cast(DRE->getDecl())) { + if (FD->getNumParams() != 1) + return {}; + const QualType P = FD->getParamDecl(0)->getType(); + if (!P->isPointerType()) + return {}; + return P->getPointeeType(); + } + } + + // default_delete: the template argument is T[], extract T. + if (const auto *CE = dyn_cast(E)) { + const auto *CTSD = dyn_cast( + CE->getConstructor()->getParent()); + if (!CTSD || CTSD->getTemplateArgs().size() != 1) + return {}; + const TemplateArgument &Arg = CTSD->getTemplateArgs()[0]; + if (Arg.getKind() != TemplateArgument::Type) + return {}; + QualType T = Arg.getAsType(); + if (!T->isArrayType()) + return {}; + return cast(T.getTypePtr())->getElementType(); + } + + return {}; +} + +// Locates the closing '>' of the shared_ptr template-id, RAngleLoc is the +// '>' to patch in the constructor expression. Locates separately the '>' of the +// VarDecl's declared type when different (copy-init, pointer/ref declarator). +static void resolveRAngleLocs(const CXXConstructExpr *Ctor, + const VarDecl *ParentVD, SourceManager &SM, + const LangOptions &LO, SourceLocation &RAngleLoc, + SourceLocation &VDRAngleLoc) { + auto FindRAngleLoc = [&](const TypeSourceInfo *TSI) -> SourceLocation { + if (!TSI) + return {}; + TypeLoc TL = TSI->getTypeLoc(); + if (auto QTL = TL.getAs()) + TL = QTL.getUnqualifiedLoc(); + if (auto TSTL = TL.getAsAdjusted()) + return TSTL.getRAngleLoc(); + return {}; + }; + + if (ParentVD) { + // Pointer/reference declarators (e.g. shared_ptr *sp = ...) wrap the + // template-id in a way that prevents safely rewriting both the declared + // type and constructor expression independently. Warn only. + TypeLoc TL = ParentVD->getTypeSourceInfo()->getTypeLoc(); + if (!TL.getAs().isNull() || + !TL.getAs().isNull()) { + RAngleLoc = {}; + VDRAngleLoc = {}; + return; + } + + RAngleLoc = FindRAngleLoc(ParentVD->getTypeSourceInfo()); + + if (RAngleLoc.isInvalid()) { + // Auto-declared VarDecl carry no written template-id in TypeSourceInfo. + // Try constructor TSI first, then fall back to token scanning between the + // constructor start and its paren/brace range. + SourceLocation CtorRAngleLoc; + + if (const auto *TOE = dyn_cast(Ctor)) + CtorRAngleLoc = FindRAngleLoc(TOE->getTypeSourceInfo()); + + if (CtorRAngleLoc.isInvalid()) { + SourceLocation CtorLoc = Ctor->getBeginLoc(); + SourceLocation LParenLoc = Ctor->getParenOrBraceRange().getBegin(); + + if (CtorLoc.isValid() && LParenLoc.isValid() && !CtorLoc.isMacroID() && + !LParenLoc.isMacroID()) { + SourceLocation ScanLoc = CtorLoc; + + // Fallback: when the VarDecl is declared with 'auto', the + // TypeSourceInfo carries no written template-id; scan tokens between + // the constructor's start and its paren/brace to locate the last '>' + // before the argument list. + for (;;) { + std::optional MaybeTok = + Lexer::findNextToken(ScanLoc, SM, LO); + if (!MaybeTok) + break; + + const Token &T = *MaybeTok; + + if (T.is(tok::l_paren) || T.is(tok::l_brace)) + break; + + if (T.is(tok::greater) || T.is(tok::greatergreater)) + CtorRAngleLoc = T.getLocation(); + + SourceLocation Next = + Lexer::getLocForEndOfToken(T.getLocation(), 0, SM, LO); + + // Guard against Lexer::getLocForEndOfToken returning the same + // location on malformed tokens/infinite loop. + if (Next == ScanLoc) + break; + + ScanLoc = Next; + } + } + } + + RAngleLoc = CtorRAngleLoc; + + } else if (ParentVD->getInitStyle() == VarDecl::CInit) { + // Copy-init: VarDecl and constructor expression have separate + // template-ids in source, both require independent insertions. + VDRAngleLoc = RAngleLoc; + + if (const auto *TOE = dyn_cast(Ctor)) + RAngleLoc = FindRAngleLoc(TOE->getTypeSourceInfo()); + + // Inserting twice into the same token would produce T[][]. + if (RAngleLoc.isInvalid() || RAngleLoc == VDRAngleLoc) + RAngleLoc = {}; + } + + } else if (const auto *TOE = dyn_cast(Ctor)) { + // No VarDecl parent: standalone temporary or return expression. + RAngleLoc = FindRAngleLoc(TOE->getTypeSourceInfo()); + } +} + +// Manual parent walk rather than a matcher because implicit +// wrappers obscure assignment contexts. +static bool isInsideAssignment(ASTContext &Ctx, const CXXConstructExpr *Ctor) { + DynTypedNode Node = DynTypedNode::create(*Ctor); + for (;;) { + auto Parents = Ctx.getParents(Node); + if (Parents.empty()) + return false; + + bool Advanced = false; + for (const DynTypedNode &P : Parents) { + if (const auto *Op = P.get()) + if (Op->getOperator() == OO_Equal) + return true; + + // VarDecl indicates initialization rather than assignment. + if (P.get()) + return false; + + if (!Advanced && (P.get() || P.get() || + P.get())) { + Node = P; + Advanced = true; + } + } + + if (!Advanced) + return false; + } +} + +// FixIt 1: insert "[]" after the closing '>' of the shared_ptr template-id. +// ">>" tokens (merged with an enclosing template's '>') get a +1 offset to get +// ">[]>" rather than "[]>>". +static FixItHint makeArrayInsertionFix(SourceLocation Loc, SourceManager &SM, + const LangOptions &LO) { + Token AngleTok; + if (!Lexer::getRawToken(Loc, AngleTok, SM, LO)) + if (AngleTok.is(tok::greatergreater)) + Loc = Loc.getLocWithOffset(1); + + return FixItHint::CreateInsertion(Loc, "[]"); +} + +// FixIt 2: remove ", deleter" — from the end of arg 0 to the end of arg 1. +static std::optional +buildRemoveDeleterFix(const CXXConstructExpr *Ctor, SourceManager &SM, + const LangOptions &LO) { + const Expr *Arg0 = Ctor->getArg(0); + const Expr *Arg1 = Ctor->getArg(1); + + SourceLocation Arg0End = + Lexer::getLocForEndOfToken(Arg0->getEndLoc(), 0, SM, LO); + SourceLocation Arg1End = + Lexer::getLocForEndOfToken(Arg1->getEndLoc(), 0, SM, LO); + + if (Arg0End.isInvalid() || Arg1End.isInvalid()) + return std::nullopt; + + return FixItHint::CreateRemoval( + CharSourceRange::getCharRange(Arg0End, Arg1End)); +} + +void UseSharedPtrArrayCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Ctor = Result.Nodes.getNodeAs("sharedPtrCtor"); + if (!Ctor) + return; + const auto *NewExpr = Result.Nodes.getNodeAs("newExpr"); + if (!NewExpr) + return; + + const auto *CTSD = dyn_cast( + Ctor->getType()->getAsCXXRecordDecl()); + if (!CTSD || CTSD->getTemplateArgs().size() != 1) + return; + const TemplateArgument &TyArg = CTSD->getTemplateArgs()[0]; + if (TyArg.getKind() != TemplateArgument::Type) + return; + QualType ElemTy = TyArg.getAsType(); + if (ElemTy->isArrayType() || ElemTy->isDependentType()) + return; + + QualType AllocTy = NewExpr->getAllocatedType(); + if (AllocTy.isNull()) + return; + if (AllocTy->isArrayType()) + AllocTy = cast(AllocTy.getTypePtr())->getElementType(); + + ASTContext &Ctx = *Result.Context; + + // Unqualified canonical comparison: handles CV qualified arrays and typedefs. + const QualType SharedElem = ElemTy.getCanonicalType().getUnqualifiedType(); + if (!Ctx.hasSameType(AllocTy.getCanonicalType().getUnqualifiedType(), + SharedElem)) + return; + + QualType DelPointee = getDeleterParamPointee(Ctor->getArg(1)); + if (DelPointee.isNull()) + return; + if (!Ctx.hasSameType(DelPointee.getCanonicalType().getUnqualifiedType(), + SharedElem)) + return; + + if (const auto *Lambda = + Result.Nodes.getNodeAs("lambdaDeleter")) { + if (!lambdaBodyIsArrayDelete(Lambda)) + return; + } + if (const auto *Func = + Result.Nodes.getNodeAs("deleterFunction")) + if (Func->getNumParams() != 1 || !Func->hasBody() || + !bodyIsArrayDeleteOfParam(Func->getBody(), Func->getParamDecl(0))) + return; + + if (Ctor->getArg(0)->getBeginLoc().isMacroID() || + Ctor->getArg(1)->getEndLoc().isMacroID()) + return; + + SourceManager &SM = *Result.SourceManager; + const LangOptions &LO = Ctx.getLangOpts(); + + // CanonicalFallbackBuff anchors the StringRef only in the canonical fallback; + // extractWrittenElementType returns into SourceManager owned memory so needs + // no buffer. + const VarDecl *ParentVD = findParentVarDecl(Ctx, Ctor); + std::string CanonicalFallbackBuff; + StringRef WrittenType = [&]() -> StringRef { + if (ParentVD) + if (StringRef S = + extractWrittenElementType(ParentVD->getTypeSourceInfo(), SM, LO); + !S.empty()) + return S; + if (const auto *TOE = dyn_cast(Ctor)) + if (StringRef S = + extractWrittenElementType(TOE->getTypeSourceInfo(), SM, LO); + !S.empty()) + return S; + CanonicalFallbackBuff = ElemTy.getAsString(Ctx.getPrintingPolicy()); + return CanonicalFallbackBuff; + }(); + + auto Warn = [&]() -> DiagnosticBuilder { + return diag(Ctor->getBeginLoc(), + "use 'std::shared_ptr<%0[]>' instead of " + "'std::shared_ptr<%0>' with explicit array deleter") + << WrittenType.str(); + }; + + // Multi-declarator: one TypeLoc shared across all declarators. Warn only. + if (ParentVD) { + const auto &VDParents = Ctx.getParents(*ParentVD); + if (!VDParents.empty()) + if (const auto *DS = VDParents[0].get()) + if (!DS->isSingleDecl()) { + warn(); + return; + } + } + + // Assignment targets may not have a rewritable written type at the + // declaration site. Warn only. + if (isInsideAssignment(Ctx, Ctor)) { + warn(); + return; + } + + SourceLocation RAngleLoc; + SourceLocation VDRAngleLoc; + resolveRAngleLocs(Ctor, ParentVD, SM, LO, RAngleLoc, VDRAngleLoc); + + // FixIt 1: Insert [] into the rewritten shared_ptr type. + FixItHint InsertArrayVarDecl; + if (VDRAngleLoc.isValid() && !VDRAngleLoc.isMacroID()) + InsertArrayVarDecl = makeArrayInsertionFix(VDRAngleLoc, SM, LO); + + // FixIt 2: Remove the explicit deleter argument. + auto RemoveDeleter = buildRemoveDeleterFix(Ctor, SM, LO); + if (!RemoveDeleter) + return; + + auto Diag = warn(); + if (RAngleLoc.isInvalid() || RAngleLoc.isMacroID()) + return; + + Diag << makeArrayInsertionFix(RAngleLoc, SM, LO) << *RemoveDeleter + << InsertArrayVarDecl; +} + +} // namespace clang::tidy::modernize diff --git a/clang-tools-extra/clang-tidy/modernize/UseSharedPtrArrayCheck.h b/clang-tools-extra/clang-tidy/modernize/UseSharedPtrArrayCheck.h new file mode 100644 index 0000000000000..59785cf5075ae --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UseSharedPtrArrayCheck.h @@ -0,0 +1,35 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USESHAREDPTRARRAYCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USESHAREDPTRARRAYCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::modernize { + +/// For the user-facing documentation see: +/// https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-shared-ptr-array.html + +class UseSharedPtrArrayCheck : public ClangTidyCheck { +public: + UseSharedPtrArrayCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus17; + } + +private: + bool lambdaBodyIsArrayDelete(const LambdaExpr *Lambda) const; +}; + +} // namespace clang::tidy::modernize + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USESHAREDPTRARRAYCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index f1b49e2cb6056..21e7334ce211a 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -263,6 +263,12 @@ New checks Finds common idioms which can be replaced by standard functions from the ```` C++20 header. +- New :doc:`modernize-use-shared-ptr-array + ` check. + + Finds ``std::shared_ptr`` constructions managing arrays via explicit + array deleters and replaces them with ``std::shared_ptr``. + - New :doc:`modernize-use-string-view ` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-shared-ptr-array.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-shared-ptr-array.rst new file mode 100644 index 0000000000000..86c6984945464 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-shared-ptr-array.rst @@ -0,0 +1,82 @@ +.. title:: clang-tidy - modernize-use-shared-ptr-array + +modernize-use-shared-ptr-array +============================== + +Finds ``std::shared_ptr`` constructions that manage dynamically allocated +arrays using explicit array deleters and suggests replacing them with +``std::shared_ptr``, which has been available since C++17. + +Fix-its are provided when the transformation can be applied safely. Some +patterns are diagnosed without a fix-it (warn-only), and others produce +no diagnostic at all. + +The check requires C++17 or later. + +Recognized Deleter Forms +------------------------ + +The following deleter forms trigger the check: + +- ``std::default_delete()`` and ``std::default_delete{}`` +- Aliases of ``std::default_delete`` introduced via ``using`` or + ``typedef`` +- Qualified, namespace-aliased, and unqualified spellings of + ``std::default_delete`` +- Captureless lambdas whose body consists solely of ``delete[] p;`` +- Free functions whose body consists solely of ``delete[] p;`` where the + deleted pointer is the function parameter and the definition is visible + in the translation unit + +Warnings Without Fix-its +^^^^^^^^^^^^^^^^^^^^^^^^ + +The following patterns are diagnosed but not automatically rewritten. + +Multiple declarators sharing one type specifier, because the shared +``TypeLoc`` makes independent insertions unsafe. + +Pointer and reference declarators, because rewriting the declared type and +constructor expression independently is not safe. + +Assignment expressions, because the declaration site is not reachable for +transformation. + +Chained assignments for the same reason. + +No Diagnostic +^^^^^^^^^^^^^ + +The following patterns produce no warning. + +Constructions that are already correct. + +Mismatched new/delete combinations (``new T`` with an array deleter, +``new T[]`` with a non-array deleter, or ``new T[]`` with no deleter). + +Type mismatches between the ``shared_ptr`` element type, allocated type, +or deleter pointee type. + +Covariant arrays, where the allocated type is derived from the +``shared_ptr`` element type. + +Functor deleters, because the check cannot statically verify arbitrary +callable objects. + +Function pointer variables, because only direct ``FunctionDecl`` references +are recognized. + +Capturing lambdas. + +Lambda bodies that are not solely ``delete[] p;``, including +multi-statement, conditional, or indirect deletion forms. + +Free functions that are not solely ``delete[] p;``. + +Template-dependent contexts, where element types are not fully resolved. + +Calls to ``shared_ptr::reset()``, which are member function calls rather +than constructor expressions. + +Constructions where the managed pointer expression or deleter argument +originates inside a macro expansion. diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-shared-ptr-array.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-shared-ptr-array.cpp new file mode 100644 index 0000000000000..f0dc1c1c32d15 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-shared-ptr-array.cpp @@ -0,0 +1,690 @@ +// RUN: %check_clang_tidy -std=c++17-or-later %s modernize-use-shared-ptr-array %t + +namespace std { + +template +struct default_delete { + constexpr default_delete() noexcept = default; + void operator()(T *ptr) const; +}; + +template +struct default_delete { + constexpr default_delete() noexcept = default; + + template + default_delete(const default_delete &) noexcept {} + + void operator()(T *ptr) const; +}; + +template +class shared_ptr { +public: + constexpr shared_ptr() noexcept = default; + + template + explicit shared_ptr(Y *ptr) {} + + template + shared_ptr(Y *ptr, D d) {} + + template + void reset(Y *ptr, D d) {} + + shared_ptr &operator=(const shared_ptr &) { return *this; } +}; + +} // namespace std + + +// Test types and helpers + +struct A { int x; }; +struct B { int x; }; +struct Base {}; +struct Derived : Base {}; + +struct WithDtor { + ~WithDtor() {} +}; + +struct ArrayFunctorDeleter { + void operator()(A *p) const { delete[] p; } +}; + +template +struct PairLike {}; + +void destroy_array(A *p) { delete[] p; } +void destroy_single(A *p) { delete p; } + +void destroy_multi(A *p) { + int x = 0; + delete[] p; +} + +void destroy_conditional(A *p) { + if (p) + delete[] p; +} + +A *GlobalArray; +void delete_global(A *p) { delete[] GlobalArray; } +void delete_other(A *p) { A *q = nullptr; delete[] q; } + +using MyDelete = std::default_delete; +using MyA = A; +typedef A AliasA; +typedef std::default_delete TDDelete; + +constexpr int kBufSize = 128; + + +// Positive: default_delete deleter forms + +void positive_default_delete_basic() { + std::shared_ptr basicSp(new A[10], std::default_delete()); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: std::shared_ptr basicSp(new A[10]); +} + +void positive_default_delete_brace_init() { + std::shared_ptr braceSp(new A[10], std::default_delete{}); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: std::shared_ptr braceSp(new A[10]); +} + +void positive_deleter_using_alias() { + std::shared_ptr sp(new A[10], MyDelete()); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: std::shared_ptr sp(new A[10]); +} + +void positive_deleter_typedef_alias() { + std::shared_ptr sp(new A[10], TDDelete()); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: std::shared_ptr sp(new A[10]); +} + +void positive_fully_qualified() { + ::std::shared_ptr qualifiedSp( + new A[10], + ::std::default_delete()); + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: ::std::shared_ptr qualifiedSp( + // CHECK-FIXES-NEXT: new A[10]); +} + +namespace st = std; + +void positive_namespace_alias() { + st::shared_ptr sp( + new A[10], + st::default_delete()); + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: st::shared_ptr sp( + // CHECK-FIXES-NEXT: new A[10]); +} + +using std::shared_ptr; +using std::default_delete; + +void positive_using_declarations() { + shared_ptr sp( + new A[10], + default_delete()); + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: shared_ptr sp( + // CHECK-FIXES-NEXT: new A[10]); +} + + +// Positive: lambda deleter forms + +void positive_lambda_basic() { + std::shared_ptr sp(new A[10], [](A *p) { delete[] p; }); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: std::shared_ptr sp(new A[10]); +} + +void positive_lambda_noexcept() { + std::shared_ptr sp( + new A[10], + [](A *p) noexcept { delete[] p; }); + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: std::shared_ptr sp( + // CHECK-FIXES-NEXT: new A[10]); +} + +void positive_lambda_explicit_return_type() { + std::shared_ptr sp(new A[10], [](A *p) -> void { delete[] p; }); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: std::shared_ptr sp(new A[10]); +} + +void positive_lambda_paren_delete() { + std::shared_ptr sp( + new A[10], + [](A *p) { (delete[] p); }); + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: std::shared_ptr sp( + // CHECK-FIXES-NEXT: new A[10]); +} + + +// Positive: free function deleter + +void positive_function_deleter() { + std::shared_ptr sp(new A[10], destroy_array); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: std::shared_ptr sp(new A[10]); +} + + +// Positive: element type variants + +void positive_primitive() { + std::shared_ptr sp( + new int[32], + std::default_delete()); + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: std::shared_ptr sp( + // CHECK-FIXES-NEXT: new int[32]); +} + +void positive_lambda_primitive() { + std::shared_ptr sp( + new int[64], + [](int *p) { delete[] p; }); + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: std::shared_ptr sp( + // CHECK-FIXES-NEXT: new int[64]); +} + +void positive_using_alias() { + std::shared_ptr sp( + new MyA[10], + std::default_delete()); + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: std::shared_ptr sp( + // CHECK-FIXES-NEXT: new MyA[10]); +} + +void positive_typedef_alias() { + std::shared_ptr sp( + new AliasA[10], + std::default_delete()); + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: std::shared_ptr sp( + // CHECK-FIXES-NEXT: new AliasA[10]); +} + +void positive_nested_template_type() { + std::shared_ptr> + sp(new PairLike[4], + std::default_delete[]>()); + // CHECK-MESSAGES: :[[@LINE-2]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: std::shared_ptr[]> + // CHECK-FIXES-NEXT: sp(new PairLike[4]); +} + +void positive_const_element() { + std::shared_ptr sp( + new const A[10](), + std::default_delete()); + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: std::shared_ptr sp( + // CHECK-FIXES-NEXT: new const A[10]()); +} + +void positive_volatile_element() { + std::shared_ptr sp( + new volatile int[8], + std::default_delete()); + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: std::shared_ptr sp( + // CHECK-FIXES-NEXT: new volatile int[8]); +} + +void positive_const_volatile_element() { + std::shared_ptr sp( + new const volatile A[4](), + std::default_delete()); + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: std::shared_ptr sp( + // CHECK-FIXES-NEXT: new const volatile A[4]()); +} + +// Non-trivial destructor: default_delete would silently call the wrong destructor on all but the first element. +void positive_nontrivial_destructor_default_delete() { + std::shared_ptr sp( + new WithDtor[10], + std::default_delete()); + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: std::shared_ptr sp( + // CHECK-FIXES-NEXT: new WithDtor[10]); +} + +void positive_nontrivial_destructor_lambda() { + std::shared_ptr sp( + new WithDtor[5], + [](WithDtor *p) { delete[] p; }); + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: std::shared_ptr sp( + // CHECK-FIXES-NEXT: new WithDtor[5]); +} + +// Nested template context exercises >> token splitting for fix-it insertion (tok:greatergreater). +void positive_rangle_merge_context() { + std::shared_ptr> sp( + new std::shared_ptr[4], + std::default_delete[]>()); + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: use 'std::shared_ptr + // CHECK-FIXES: std::shared_ptr[]> sp( + // CHECK-FIXES-NEXT: new std::shared_ptr[4]); +} + + +// Positive: array size variants + +void positive_runtime_size() { + int n = 10; + std::shared_ptr sp( + new A[n], + std::default_delete()); + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: std::shared_ptr sp( + // CHECK-FIXES-NEXT: new A[n]); +} + +void positive_constexpr_size() { + std::shared_ptr sp( + new A[kBufSize], + std::default_delete()); + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: std::shared_ptr sp( + // CHECK-FIXES-NEXT: new A[kBufSize]); +} + +void positive_zero_size() { + std::shared_ptr sp( + new A[0], + std::default_delete()); + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: std::shared_ptr sp( + // CHECK-FIXES-NEXT: new A[0]); +} + + +// Positive: array initializer forms + +void positive_value_initialized_array() { + std::shared_ptr sp( + new A[10](), + std::default_delete()); + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: std::shared_ptr sp( + // CHECK-FIXES-NEXT: new A[10]()); +} + +void positive_brace_initialized_array() { + std::shared_ptr sp( + new int[3]{1, 2, 3}, + std::default_delete()); + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: std::shared_ptr sp( + // CHECK-FIXES-NEXT: new int[3]{1, 2, 3}); +} + +void positive_extra_parens() { + std::shared_ptr sp( + (new A[10]), + std::default_delete()); + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: std::shared_ptr sp( + // CHECK-FIXES-NEXT: (new A[10])); +} + + +// Positive: VarDecl constructor styles + +void positive_brace_init_ctor() { + std::shared_ptr sp{new A[10], std::default_delete()}; + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: std::shared_ptr sp{new A[10]}; +} + +void positive_static_decl() { + static std::shared_ptr sp( + new A[10], + std::default_delete()); + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: static std::shared_ptr sp( + // CHECK-FIXES-NEXT: new A[10]); +} + +// Copy-init: VarDecl declared type and the temporary both need '[]' inserted. +void positive_copy_init() { + std::shared_ptr sp = + std::shared_ptr( + new A[10], + std::default_delete()); + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: std::shared_ptr sp = + // CHECK-FIXES-NEXT: std::shared_ptr( + // CHECK-FIXES-NEXT: new A[10]); +} + + +// Positive: non-VarDecl contexts + +// auto direct-init: no VarDecl TypeLoc to patch; only the constructor is rewritten. +void positive_auto_deduction() { + auto sp = + std::shared_ptr( + new A[10], + std::default_delete()); + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: auto sp = + // CHECK-FIXES-NEXT: std::shared_ptr( + // CHECK-FIXES-NEXT: new A[10]); +} + +// auto copy-init: same as direct-init — auto VarDecl has no written template-id. +void positive_auto_copy_init() { + auto sp = std::shared_ptr(new A[10], std::default_delete()); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: auto sp = std::shared_ptr(new A[10]); +} + +// Return statement: constructor is rewritten; the function's declared return +// type is not updated and may require manual adjustment. +std::shared_ptr positive_return_stmt() { + return std::shared_ptr( + new A[10], + std::default_delete()); + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: return std::shared_ptr( + // CHECK-FIXES-NEXT: new A[10]); +} + +std::shared_ptr> positive_return_nested_template() { + return std::shared_ptr>( + new PairLike[4], + std::default_delete[]>()); + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: return std::shared_ptr[]>( + // CHECK-FIXES-NEXT: new PairLike[4]); +} + +void positive_unqualified_using_namespace() { + using namespace std; + shared_ptr sp(new A[10], default_delete()); + // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: use 'std::shared_ptr + // CHECK-FIXES: shared_ptr sp(new A[10]); +} + + +// Positive: formatting edge cases + +void positive_multiline_input() { + std::shared_ptr + sp( + new A[10], + std::default_delete()); + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: std::shared_ptr + // CHECK-FIXES-NEXT: sp( + // CHECK-FIXES-NEXT: new A[10]); +} + +void positive_inline_comment_preserve() { + std::shared_ptr /*type-comment*/ sp( + new A[10], /*deleter-comment*/ + std::default_delete()); + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: std::shared_ptr /*type-comment*/ sp( + // CHECK-FIXES-NEXT: new A[10]); +} + +void positive_template_comments() { + std::shared_ptr sp( + new A[10], + std::default_delete()); + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: std::shared_ptr sp( + // CHECK-FIXES-NEXT: new A[10]); +} + + + +// Warn only: diagnostic emitted, no fix-it + +// Multi-declarator: shared TypeLoc makes independent fix-its unsafe. +void warn_only_multi_declarator() { + std::shared_ptr sp1(new A[10], std::default_delete()), + sp2(new A[8], std::default_delete()); + // CHECK-MESSAGES: :[[@LINE-2]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-MESSAGES: :[[@LINE-2]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: std::shared_ptr sp1(new A[10], std::default_delete()), + // CHECK-FIXES-NEXT: sp2(new A[8], std::default_delete()); +} + +// Pointer/reference declarators: rewriting both the declared type and +// constructor expression independently is not safe. Warn only. +void warn_only_pointer_declarator() { + std::shared_ptr *sp = + new std::shared_ptr( + new A[10], + std::default_delete()); + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: use 'std::shared_ptr +} + +void warn_only_reference_declarator() { + std::shared_ptr &sp = + *new std::shared_ptr( + new A[10], + std::default_delete()); + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: use 'std::shared_ptr +} + + +// Assignment: declaration site is not reachable for transformation. +void warn_only_assignment_after_decl() { + std::shared_ptr sp; + sp = std::shared_ptr( + new A[10], + std::default_delete()); + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: sp = std::shared_ptr( +} + +// Chained assignment: same reasoning as single assignment. +void warn_only_chained_assignment() { + std::shared_ptr sp, sp2; + sp = sp2 = std::shared_ptr( + new A[10], + std::default_delete()); + // CHECK-MESSAGES: :[[@LINE-3]]:{{[0-9]+}}: warning: use 'std::shared_ptr< + // CHECK-FIXES: sp = sp2 = std::shared_ptr( +} + + +// Negative: already correct + +void negative_already_array_type() { + std::shared_ptr sp(new A[10]); +} + +void negative_already_array_with_default_delete() { + std::shared_ptr sp(new A[10], std::default_delete()); +} + + +// Negative: wrong new/delete combination + +void negative_single_object_no_deleter() { + std::shared_ptr sp(new A); +} + +void negative_single_object_default_delete() { + std::shared_ptr sp(new A, std::default_delete()); +} + +void negative_single_object_lambda() { + std::shared_ptr sp(new A, [](A *p) { delete p; }); +} + +void negative_array_new_no_deleter() { + std::shared_ptr sp(new A[10]); +} + +// Array new with non-array default_delete: would silently corrupt — not our +// job to warn here since it's a separate bug, and the pattern doesn't match. +void negative_array_new_single_deleter() { + std::shared_ptr sp(new A[10], std::default_delete()); +} + +void negative_array_new_lambda_single_delete() { + std::shared_ptr sp(new A[10], [](A *p) { delete p; }); +} + + +// Negative: type mismatches + +void negative_type_mismatch_allocated() { + std::shared_ptr sp(new B[10], std::default_delete()); +} + +void negative_type_mismatch_deleter() { + std::shared_ptr sp(new A[10], std::default_delete()); +} + +// Covariant array: Derived[] is not safely manageable via shared_ptr +// without a virtual destructor and matching sizes — leave it alone. +void negative_covariant_array() { + std::shared_ptr sp(new Derived[10], std::default_delete()); +} + +// Non-trivial destructor with the wrong (non-array) deleter: the existing code +// is already a bug, but it's not our pattern — the deleter pointee won't match. +void negative_nontrivial_destructor_wrong_deleter() { + std::shared_ptr sp(new WithDtor[5], std::default_delete()); +} + + +// Negative: unsupported deleter forms + +// Functor: operator() is not a directly inspectable function body from here. +void negative_functor_deleter() { + std::shared_ptr sp(new A[10], ArrayFunctorDeleter{}); +} + +// Template-dependent: element type is dependent; can't canonically compare. +template +void negative_template_context(int n) { + std::shared_ptr sp(new T[n], std::default_delete()); +} + +// Function pointer variable: declRefExpr targets a VarDecl, not a FunctionDecl. +// The matcher binds only to functionDecl() refs; this silently falls through. +void negative_function_pointer_variable() { + auto fn = destroy_array; + std::shared_ptr sp(new A[10], fn); +} + +void negative_lambda_capture() { + int x = 0; + std::shared_ptr sp(new A[10], [x](A *p) { delete[] p; }); +} + +void negative_lambda_multiple_statements() { + std::shared_ptr sp( + new A[10], + [](A *p) { int x = 0; delete[] p; }); +} + +void negative_lambda_delete_alias() { + std::shared_ptr sp( + new A[10], + [](A *p) { A *q = p; delete[] q; }); +} + +void negative_lambda_conditional_delete() { + std::shared_ptr sp(new A[10], [](A *p) { if (p) delete[] p; }); +} + +void negative_lambda_return_before_delete() { + std::shared_ptr sp( + new A[10], + [](A *p) { if (!p) return; delete[] p; }); +} + +void negative_lambda_no_delete() { + std::shared_ptr sp(new A[10], [](A *) {}); +} + +// Lambda casting to void* before delete[]: the DeclRefExpr inside the delete +// refers to the cast result, not the original parameter. +void negative_void_ptr_cast_lambda() { + std::shared_ptr sp( + new A[10], + [](void *p) { delete[] static_cast(p); }); +} + +void negative_function_single_delete() { + std::shared_ptr sp(new A[10], destroy_single); +} + +void negative_function_multi_stmt() { + std::shared_ptr sp(new A[10], destroy_multi); +} + +void negative_function_conditional_delete() { + std::shared_ptr sp(new A[10], destroy_conditional); +} + +// Function deletes a global rather than its parameter: the DeclRefExpr inside +// the delete doesn't refer to the param. +void negative_function_global_delete() { + std::shared_ptr sp(new A[10], delete_global); +} + +void negative_function_delete_other() { + std::shared_ptr sp(new A[10], delete_other); +} + + +// Negative: unsupported contexts + +// reset() takes the same two-argument form but is a member function call, not +// a constructor — the cxxConstructExpr matcher won't fire. +void negative_reset() { + std::shared_ptr sp; + sp.reset(new A[10], std::default_delete()); +} + +void negative_reset_lambda() { + std::shared_ptr sp; + sp.reset(new A[10], [](A *p) { delete[] p; }); +} + +// Negative: macros + +// Full expansion in macro: new-expression and deleter are both inside the macro +// body; both source ranges are macro IDs, bail silently. +#define MAKE_SHARED_PTR \ + std::shared_ptr(new A[10], std::default_delete()) + +void negative_macro_full_expansion() { + std::shared_ptr sp = MAKE_SHARED_PTR; +} + +// Deleter-only macro: the deleter arg's end location is a macro ID; the +// rewritten range would be unsafe to emit even though new A[10] is clean. +#define ARR_DEL std::default_delete() + +void negative_macro_deleter_only() { + std::shared_ptr sp(new A[10], ARR_DEL); +} \ No newline at end of file