diff --git a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt @@ -13,6 +13,7 @@ NonCopyableObjects.cpp NonPrivateMemberVariablesInClassesCheck.cpp RedundantExpressionCheck.cpp + SharedPtrArrayMismatchCheck.cpp StaticAssertCheck.cpp ThrowByValueCatchByReferenceCheck.cpp UnconventionalAssignOperatorCheck.cpp diff --git a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp --- a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp @@ -17,6 +17,7 @@ #include "NonCopyableObjects.h" #include "NonPrivateMemberVariablesInClassesCheck.h" #include "RedundantExpressionCheck.h" +#include "SharedPtrArrayMismatchCheck.h" #include "StaticAssertCheck.h" #include "ThrowByValueCatchByReferenceCheck.h" #include "UnconventionalAssignOperatorCheck.h" @@ -46,6 +47,8 @@ "misc-non-private-member-variables-in-classes"); CheckFactories.registerCheck( "misc-redundant-expression"); + CheckFactories.registerCheck( + "misc-shared-ptr-array-mismatch"); CheckFactories.registerCheck("misc-static-assert"); CheckFactories.registerCheck( "misc-throw-by-value-catch-by-reference"); diff --git a/clang-tools-extra/clang-tidy/misc/SharedPtrArrayMismatchCheck.h b/clang-tools-extra/clang-tidy/misc/SharedPtrArrayMismatchCheck.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/misc/SharedPtrArrayMismatchCheck.h @@ -0,0 +1,42 @@ +//===--- SharedPtrArrayMismatchCheck.h - clang-tidy -------------*- C++ -*-===// +// +// 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_MISC_SHAREDPTRARRAYMISMATCHCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SHAREDPTRARRAYMISMATCHCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang { +namespace tidy { +namespace misc { + +/// Find `std::shared_ptr(new T[...])`, replace it (in some cases) with +/// `std::shared_ptr(new T[...])`. +/// +/// Example: +/// +/// \code +/// std::shared_ptr PtrArr{new int[10]}; +/// \endcode +class SharedPtrArrayMismatchCheck : public ClangTidyCheck { +public: + SharedPtrArrayMismatchCheck(StringRef Name, ClangTidyContext *Context); + + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus11; + } + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; +}; + +} // namespace misc +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_SHAREDPTRARRAYMISMATCHCHECK_H diff --git a/clang-tools-extra/clang-tidy/misc/SharedPtrArrayMismatchCheck.cpp b/clang-tools-extra/clang-tidy/misc/SharedPtrArrayMismatchCheck.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/misc/SharedPtrArrayMismatchCheck.cpp @@ -0,0 +1,120 @@ +//===--- SharedPtrArrayMismatchCheck.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 "SharedPtrArrayMismatchCheck.h" +#include "../utils/ASTUtils.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace misc { + +SharedPtrArrayMismatchCheck::SharedPtrArrayMismatchCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + +void SharedPtrArrayMismatchCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) {} + +void SharedPtrArrayMismatchCheck::registerMatchers(MatchFinder *Finder) { + auto UsedConstructor = + cxxConstructorDecl( + ofClass(classTemplateSpecializationDecl( + hasName("::std::shared_ptr"), templateArgumentCountIs(1), + hasTemplateArgument( + 0, templateArgument(refersToType(qualType().bind("T")))))), + parameterCountIs(1)) + .bind("used_constructor"); + auto ConstructExpr = + cxxConstructExpr( + hasDeclaration(UsedConstructor), argumentCountIs(1), + hasArgument(0, cxxNewExpr(isArray(), hasType(pointerType(pointee( + equalsBoundNode("T"))))) + .bind("new_expr"))) + .bind("construct_expr"); + Finder->addMatcher(ConstructExpr, this); +} + +namespace { + +bool isInSingleDeclStmt(const DeclaratorDecl *D) { + const DynTypedNodeList Parents = + D->getASTContext().getParentMapContext().getParents(*D); + for (const DynTypedNode &PNode : Parents) + if (const auto *PDecl = PNode.get()) + return PDecl->isSingleDecl(); + return false; +} + +const DeclaratorDecl *getConstructedVarOrField(const Expr *FoundConstructExpr, + ASTContext &Ctx) { + const DynTypedNodeList ConstructParents = + Ctx.getParentMapContext().getParents(*FoundConstructExpr); + if (ConstructParents.size() != 1) + return nullptr; + const Decl *ParentDecl = ConstructParents.begin()->get(); + if (!ParentDecl) + return nullptr; + if (const auto *ParentVar = dyn_cast(ParentDecl)) + return ParentVar; + if (const auto *ParentField = dyn_cast(ParentDecl)) + return ParentField; + + return nullptr; +} + +} // namespace + +void SharedPtrArrayMismatchCheck::check( + const MatchFinder::MatchResult &Result) { + const auto *FoundNewExpr = Result.Nodes.getNodeAs("new_expr"); + const auto *FoundConstructExpr = + Result.Nodes.getNodeAs("construct_expr"); + const auto *UsedConstructorDecl = + Result.Nodes.getNodeAs("used_constructor"); + + ASTContext &Ctx = UsedConstructorDecl->getASTContext(); + const DeclaratorDecl *VarOrField = + getConstructedVarOrField(FoundConstructExpr, Ctx); + + auto D = diag(FoundNewExpr->getBeginLoc(), + "shared pointer to non-array is initialized with array"); + D << FoundNewExpr->getSourceRange(); + + if (VarOrField) { + auto TSTypeLoc = VarOrField->getTypeSourceInfo() + ->getTypeLoc() + .getAsAdjusted(); + assert(TSTypeLoc.getNumArgs() == 1 && + "Matched type should have 1 template argument."); + + SourceRange TemplateArgumentRange = TSTypeLoc.getArgLoc(0) + .getTypeSourceInfo() + ->getTypeLoc() + .getLocalSourceRange(); + D << TemplateArgumentRange; + + if (isInSingleDeclStmt(VarOrField)) { + const SourceManager &SM = Ctx.getSourceManager(); + if (!utils::rangeCanBeFixed(TemplateArgumentRange, &SM)) + return; + + SourceLocation InsertLoc = Lexer::getLocForEndOfToken( + TemplateArgumentRange.getEnd(), 0, SM, Ctx.getLangOpts()); + D << FixItHint::CreateInsertion(InsertLoc, "[]"); + } + } +} + +} // namespace misc +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -108,6 +108,10 @@ Reports identifier with unicode right-to-left characters. +- New :doc:`misc-shared-ptr-array-mismatch ` check. + + Reports if a C++ shared pointer is initialized with an array but declared with non-array type. + - New :doc:`readability-container-data-pointer ` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -220,6 +220,7 @@ `misc-non-copyable-objects `_, `misc-non-private-member-variables-in-classes `_, `misc-redundant-expression `_, "Yes" + `misc-shared-ptr-array-mismatch `_, "Yes" `misc-static-assert `_, "Yes" `misc-throw-by-value-catch-by-reference `_, `misc-unconventional-assign-operator `_, diff --git a/clang-tools-extra/docs/clang-tidy/checks/misc-shared-ptr-array-mismatch.rst b/clang-tools-extra/docs/clang-tidy/checks/misc-shared-ptr-array-mismatch.rst new file mode 100644 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/misc-shared-ptr-array-mismatch.rst @@ -0,0 +1,25 @@ +.. title:: clang-tidy - misc-shared-ptr-array-mismatch + +misc-shared-ptr-array-mismatch +============================== + +Find construction of ``std::shared_ptr`` where it is initialized with a new-expression ``new T[]``. +The check offers replacement of ``shared_ptr`` to ``shared_ptr`` if it is used at a single variable declaration. + +Reason: A ``shared_ptr`` uses plain ``delete`` to deallocate the target memory, this is incorrect if an array is to be deallocated. + +Example: + +.. code-block:: c++ + + std::shared_ptr x(new Foo[10]); // -> std::shared_ptr x(new Foo[10]); + // ^ warning: shared pointer to non-array is initialized with array [misc-shared-ptr-array-mismatch] + std::shared_ptr x1(new Foo), x2(new Foo[10]); // no replacement + // ^ warning: shared pointer to non-array is initialized with array [misc-shared-ptr-array-mismatch] + + std::shared_ptr x3(new Foo[10], [](const Foo *ptr) { delete[] ptr; }); // no warning + + struct S { + std::shared_ptr x(new Foo[10]); // no replacement in this case + // ^ warning: shared pointer to non-array is initialized with array [misc-shared-ptr-array-mismatch] + }; diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc-shared-ptr-array-mismatch.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc-shared-ptr-array-mismatch.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc-shared-ptr-array-mismatch.cpp @@ -0,0 +1,96 @@ +// RUN: %check_clang_tidy %s misc-shared-ptr-array-mismatch %t + +namespace std { + +template +struct default_delete {}; + +template +struct shared_ptr { + template + explicit shared_ptr(Y *) {} + template + shared_ptr(Y *, Deleter) {} +}; + +} // namespace std + +struct A {}; + +void f1() { + std::shared_ptr P1{new int}; + std::shared_ptr P2{new int[10]}; + // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: shared pointer to non-array is initialized with array [misc-shared-ptr-array-mismatch] + // CHECK-FIXES: std::shared_ptr P2{new int[10]}; + std::shared_ptr< int > P3{new int[10]}; + // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: shared pointer to non-array is initialized with array [misc-shared-ptr-array-mismatch] + // CHECK-FIXES: std::shared_ptr< int[] > P3{new int[10]}; + std::shared_ptr P4(new int[10]); + // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: shared pointer to non-array is initialized with array [misc-shared-ptr-array-mismatch] + // CHECK-FIXES: std::shared_ptr P4(new int[10]); + new std::shared_ptr(new int[10]); + // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: shared pointer to non-array is initialized with array [misc-shared-ptr-array-mismatch] + std::shared_ptr P5(new int[10]); + std::shared_ptr P6(new int[10], [](const int *Ptr) {}); +} + +void f2() { + std::shared_ptr P1(new A); + std::shared_ptr P2(new A[10]); + // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: shared pointer to non-array is initialized with array [misc-shared-ptr-array-mismatch] + // CHECK-FIXES: std::shared_ptr P2(new A[10]); + std::shared_ptr P3(new A[10]); +} + +void f3() { + std::shared_ptr P1{new int}, P2{new int[10]}, P3{new int[10]}; + // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: shared pointer to non-array is initialized with array [misc-shared-ptr-array-mismatch] + // CHECK-MESSAGES: :[[@LINE-2]]:57: warning: shared pointer to non-array is initialized with array [misc-shared-ptr-array-mismatch] +} + +struct S { + std::shared_ptr P1; + std::shared_ptr P2{new int[10]}; + // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: shared pointer to non-array is initialized with array [misc-shared-ptr-array-mismatch] + std::shared_ptr P3{new int}, P4{new int[10]}; + // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: shared pointer to non-array is initialized with array [misc-shared-ptr-array-mismatch] + S() : P1{new int[10]} {} + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: shared pointer to non-array is initialized with array [misc-shared-ptr-array-mismatch] +}; + +void f_parm(std::shared_ptr); + +void f4() { + f_parm(std::shared_ptr{new int[10]}); + // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: shared pointer to non-array is initialized with array [misc-shared-ptr-array-mismatch] +} + +std::shared_ptr f_ret() { + return std::shared_ptr(new int[10]); + // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: shared pointer to non-array is initialized with array [misc-shared-ptr-array-mismatch] +} + +template +void f_tmpl() { + std::shared_ptr P1{new T[10]}; +} + +void f5() { + f_tmpl(); +} + +#define CHAR_PTR_TYPE std::shared_ptr +#define CHAR_PTR_VAR(X) \ + X { new char[10] } +#define CHAR_PTR_INIT(X, Y) \ + std::shared_ptr X { Y } + +void f6() { + CHAR_PTR_TYPE P1{new char[10]}; + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: shared pointer to non-array is initialized with array [misc-shared-ptr-array-mismatch] + std::shared_ptr CHAR_PTR_VAR(P2); + // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: shared pointer to non-array is initialized with array [misc-shared-ptr-array-mismatch] + // CHECK-FIXES: std::shared_ptr CHAR_PTR_VAR(P2); + CHAR_PTR_INIT(P3, new char[10]); + // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: shared pointer to non-array is initialized with array [misc-shared-ptr-array-mismatch] +}