https://github.com/serge-sans-paille updated https://github.com/llvm/llvm-project/pull/229802
>From 3846696c9d196b3b0200fde48bb98ded5ce7b330 Mon Sep 17 00:00:00 2001 From: serge-sans-paille <[email protected]> Date: Wed, 7 Oct 2026 17:16:44 +0200 Subject: [PATCH 1/6] [clang-tidy] Fix false positive in modernize-make-unique It is not valid to turn struct P {}; struct S : P { private: ~S(); }; std::unique_ptr<P> p; void foo() { p.reset(new S); } into void foo() { p = std::make_unique<S>(); } because the destructor of S is private, so it's illegal to create a temporary unique_ptr from it. --- .../modernize/MakeSmartPtrCheck.cpp | 36 +++++++++++-------- clang-tools-extra/docs/ReleaseNotes.md | 3 ++ .../make-unique-inaccessible-dtors.cpp | 31 ++++++++++++++++ 3 files changed, 56 insertions(+), 14 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/make-unique-inaccessible-dtors.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp index c05df597a28a4d..504769167f843f 100644 --- a/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp @@ -73,21 +73,28 @@ void MakeSmartPtrCheck::registerMatchers(ast_matchers::MatchFinder *Finder) { const auto CanCallCtor = unless(has(ignoringImpCasts( cxxConstructExpr(hasDeclaration(decl(unless(isPublic()))))))); + // A private destructor would require somehow making std::make_unique or some + // of its internal befriend the created type, so better be conservative in + // that case. + const auto CanCallDtor = unless(hasType(pointerType(pointee(recordType( + hasDeclaration(has(cxxDestructorDecl(unless(isPublic()))))))))); + auto IsPlacement = hasAnyPlacementArg(anything()); Finder->addMatcher( - traverse(TK_AsIs, - cxxConstructExpr( - anyOf(hasParent(cxxBindTemporaryExpr()), - hasParent(varDecl().bind(DirectVar))), - hasType(getSmartPointerTypeMatcher()), argumentCountIs(1), - hasArgument( - 0, cxxNewExpr(hasType(pointsTo(qualType(hasCanonicalType( - equalsBoundNode(PointerType))))), - CanCallCtor, unless(IsPlacement)) - .bind(NewExpression)), - unless(isInTemplateInstantiation())) - .bind(ConstructorCall)), + traverse( + TK_AsIs, + cxxConstructExpr( + anyOf(hasParent(cxxBindTemporaryExpr()), + hasParent(varDecl().bind(DirectVar))), + hasType(getSmartPointerTypeMatcher()), argumentCountIs(1), + hasArgument( + 0, cxxNewExpr(hasType(pointsTo(qualType(hasCanonicalType( + equalsBoundNode(PointerType))))), + CanCallCtor, CanCallDtor, unless(IsPlacement)) + .bind(NewExpression)), + unless(isInTemplateInstantiation())) + .bind(ConstructorCall)), this); Finder->addMatcher( @@ -95,8 +102,9 @@ void MakeSmartPtrCheck::registerMatchers(ast_matchers::MatchFinder *Finder) { TK_AsIs, cxxMemberCallExpr( unless(isInTemplateInstantiation()), - hasArgument(0, cxxNewExpr(CanCallCtor, unless(IsPlacement)) - .bind(NewExpression)), + hasArgument( + 0, cxxNewExpr(CanCallCtor, CanCallDtor, unless(IsPlacement)) + .bind(NewExpression)), callee(cxxMethodDecl(hasName("reset"))), anyOf(thisPointerType(getSmartPointerTypeMatcher()), on(ignoringImplicit(anyOf( diff --git a/clang-tools-extra/docs/ReleaseNotes.md b/clang-tools-extra/docs/ReleaseNotes.md index 69f3ecee49926b..9649921ee4981c 100644 --- a/clang-tools-extra/docs/ReleaseNotes.md +++ b/clang-tools-extra/docs/ReleaseNotes.md @@ -265,6 +265,9 @@ infrastructure are described first, followed by tool-specific sections. nested expressions involving different macros or a mix of macro and non-macro operands. +- Fixed false positive in {doc}`modernize-make-unique + <clang-tidy/checks/modernize/make-unique>` in presence of private destructor. + - Fixed a crash in {doc}`modernize-raw-string-literal <clang-tidy/checks/modernize/raw-string-literal>` on synthetic string literals created for raw user-defined literal operators, such as `12_w`. diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/make-unique-inaccessible-dtors.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/make-unique-inaccessible-dtors.cpp new file mode 100644 index 00000000000000..99754069ec3f64 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/make-unique-inaccessible-dtors.cpp @@ -0,0 +1,31 @@ +// RUN: %check_clang_tidy -std=c++14,c++17 %s modernize-make-unique %t -- -- -I %S/Inputs/smart-ptr -D CXX_14_17=1 + +#include "unique_ptr.h" +// CHECK-FIXES: #include <memory> + +struct Base { +}; + +struct ChildWithPublicDestructor: Base { +public: + ~ChildWithPublicDestructor(); +}; + +struct ChildWithProtectedDestructor: Base { +protected: + ~ChildWithProtectedDestructor(); +}; + +struct ChildWithPrivateDestructor: Base { +private: + ~ChildWithPrivateDestructor(); +}; + +void check_reset() { + std::unique_ptr<Base> p; + p.reset(new ChildWithPublicDestructor()); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use std::make_unique instead + // CHECK-FIXES: p = std::make_unique<ChildWithPublicDestructor>(); + p.reset(new ChildWithProtectedDestructor()); + p.reset(new ChildWithPrivateDestructor()); +} >From cdd3e5113d80a65815c069d91c0fb06fc85484d8 Mon Sep 17 00:00:00 2001 From: serge-sans-paille <[email protected]> Date: Sat, 17 Oct 2026 16:37:03 +0200 Subject: [PATCH 2/6] fixup! [clang-tidy] Fix false positive in modernize-make-unique --- .../clang-tidy/modernize/MakeSmartPtrCheck.cpp | 11 ++++++----- clang-tools-extra/docs/ReleaseNotes.md | 3 ++- .../modernize/make-unique-inaccessible-dtors.cpp | 5 +++++ 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp index 504769167f843f..c295028100b449 100644 --- a/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp @@ -73,11 +73,12 @@ void MakeSmartPtrCheck::registerMatchers(ast_matchers::MatchFinder *Finder) { const auto CanCallCtor = unless(has(ignoringImpCasts( cxxConstructExpr(hasDeclaration(decl(unless(isPublic()))))))); - // A private destructor would require somehow making std::make_unique or some - // of its internal befriend the created type, so better be conservative in - // that case. - const auto CanCallDtor = unless(hasType(pointerType(pointee(recordType( - hasDeclaration(has(cxxDestructorDecl(unless(isPublic()))))))))); + // A private/deleted destructor would require somehow making std::make_unique + // or some of its internal befriend the created type, so better be + // conservative in that case. + const auto CanCallDtor = + unless(hasType(pointerType(pointee(recordType(hasDeclaration( + has(cxxDestructorDecl(anyOf(isDeleted(), unless(isPublic())))))))))); auto IsPlacement = hasAnyPlacementArg(anything()); diff --git a/clang-tools-extra/docs/ReleaseNotes.md b/clang-tools-extra/docs/ReleaseNotes.md index 9649921ee4981c..faa9fcd280ad03 100644 --- a/clang-tools-extra/docs/ReleaseNotes.md +++ b/clang-tools-extra/docs/ReleaseNotes.md @@ -266,7 +266,8 @@ infrastructure are described first, followed by tool-specific sections. non-macro operands. - Fixed false positive in {doc}`modernize-make-unique - <clang-tidy/checks/modernize/make-unique>` in presence of private destructor. + <clang-tidy/checks/modernize/make-unique>` in presence of private, preotcted + or deleted destructor. - Fixed a crash in {doc}`modernize-raw-string-literal <clang-tidy/checks/modernize/raw-string-literal>` on synthetic string diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/make-unique-inaccessible-dtors.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/make-unique-inaccessible-dtors.cpp index 99754069ec3f64..b9d53f78e8ae53 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/make-unique-inaccessible-dtors.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/make-unique-inaccessible-dtors.cpp @@ -21,6 +21,10 @@ struct ChildWithPrivateDestructor: Base { ~ChildWithPrivateDestructor(); }; +struct ChildWithDeletedDestructor: Base { + ~ChildWithDeletedDestructor() = delete; +}; + void check_reset() { std::unique_ptr<Base> p; p.reset(new ChildWithPublicDestructor()); @@ -28,4 +32,5 @@ void check_reset() { // CHECK-FIXES: p = std::make_unique<ChildWithPublicDestructor>(); p.reset(new ChildWithProtectedDestructor()); p.reset(new ChildWithPrivateDestructor()); + p.reset(new ChildWithDeletedDestructor()); } >From 157b739816385a043fec2736b0e7e5f62a3f97b6 Mon Sep 17 00:00:00 2001 From: serge-sans-paille <[email protected]> Date: Mon, 19 Oct 2026 14:05:40 +0200 Subject: [PATCH 3/6] fixup! fixup! [clang-tidy] Fix false positive in modernize-make-unique --- clang-tools-extra/docs/ReleaseNotes.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/docs/ReleaseNotes.md b/clang-tools-extra/docs/ReleaseNotes.md index faa9fcd280ad03..2143501b4f1587 100644 --- a/clang-tools-extra/docs/ReleaseNotes.md +++ b/clang-tools-extra/docs/ReleaseNotes.md @@ -266,7 +266,7 @@ infrastructure are described first, followed by tool-specific sections. non-macro operands. - Fixed false positive in {doc}`modernize-make-unique - <clang-tidy/checks/modernize/make-unique>` in presence of private, preotcted + <clang-tidy/checks/modernize/make-unique>` in presence of private, protected or deleted destructor. - Fixed a crash in {doc}`modernize-raw-string-literal >From ba64e048b2afe031713740251127a31f1527fdf7 Mon Sep 17 00:00:00 2001 From: serge-sans-paille <[email protected]> Date: Mon, 19 Oct 2026 15:22:21 +0200 Subject: [PATCH 4/6] fixup! fixup! fixup! [clang-tidy] Fix false positive in modernize-make-unique --- .../modernize/MakeSmartPtrCheck.cpp | 9 ++++---- .../make-unique-inaccessible-dtors.cpp | 23 +++++++++++++++++++ 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp index c295028100b449..35d8d90b3d51e9 100644 --- a/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp @@ -76,9 +76,10 @@ void MakeSmartPtrCheck::registerMatchers(ast_matchers::MatchFinder *Finder) { // A private/deleted destructor would require somehow making std::make_unique // or some of its internal befriend the created type, so better be // conservative in that case. - const auto CanCallDtor = - unless(hasType(pointerType(pointee(recordType(hasDeclaration( - has(cxxDestructorDecl(anyOf(isDeleted(), unless(isPublic())))))))))); + const auto CanCallDtor = unless(hasType(hasUnqualifiedDesugaredType( + pointerType(pointee(hasUnqualifiedDesugaredType( + recordType(hasDeclaration(has(cxxDestructorDecl( + anyOf(isDeleted(), unless(isPublic())))))))))))); auto IsPlacement = hasAnyPlacementArg(anything()); @@ -92,7 +93,7 @@ void MakeSmartPtrCheck::registerMatchers(ast_matchers::MatchFinder *Finder) { hasArgument( 0, cxxNewExpr(hasType(pointsTo(qualType(hasCanonicalType( equalsBoundNode(PointerType))))), - CanCallCtor, CanCallDtor, unless(IsPlacement)) + CanCallCtor, unless(IsPlacement)) .bind(NewExpression)), unless(isInTemplateInstantiation())) .bind(ConstructorCall)), diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/make-unique-inaccessible-dtors.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/make-unique-inaccessible-dtors.cpp index b9d53f78e8ae53..c8c2d028a65762 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/make-unique-inaccessible-dtors.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/make-unique-inaccessible-dtors.cpp @@ -11,6 +11,14 @@ struct ChildWithPublicDestructor: Base { ~ChildWithPublicDestructor(); }; +using ChildWithPublicDestructorAlias = ChildWithPublicDestructor; + +template<class T> +struct TemplatedChildWithPublicDestructor: Base { +public: + ~TemplatedChildWithPublicDestructor(); +}; + struct ChildWithProtectedDestructor: Base { protected: ~ChildWithProtectedDestructor(); @@ -25,12 +33,27 @@ struct ChildWithDeletedDestructor: Base { ~ChildWithDeletedDestructor() = delete; }; +using ChildWithDeletedDestructorAlias = ChildWithDeletedDestructor; + +template<class T> +struct TemplatedChildWithDeletedDestructor: Base { + ~TemplatedChildWithDeletedDestructor() = delete; +}; + void check_reset() { std::unique_ptr<Base> p; p.reset(new ChildWithPublicDestructor()); // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use std::make_unique instead // CHECK-FIXES: p = std::make_unique<ChildWithPublicDestructor>(); + p.reset(new ChildWithPublicDestructorAlias()); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use std::make_unique instead + // CHECK-FIXES: p = std::make_unique<ChildWithPublicDestructorAlias>(); + p.reset(new TemplatedChildWithPublicDestructor<int>()); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use std::make_unique instead + // CHECK-FIXES: p = std::make_unique<TemplatedChildWithPublicDestructor<int>>(); p.reset(new ChildWithProtectedDestructor()); p.reset(new ChildWithPrivateDestructor()); p.reset(new ChildWithDeletedDestructor()); + p.reset(new ChildWithDeletedDestructorAlias()); + p.reset(new TemplatedChildWithDeletedDestructor<int>()); } >From e7c94a72b256e9e28b868b057e3ddf15100f0660 Mon Sep 17 00:00:00 2001 From: serge-sans-paille <[email protected]> Date: Mon, 19 Oct 2026 17:17:31 +0200 Subject: [PATCH 5/6] fixup! fixup! fixup! fixup! [clang-tidy] Fix false positive in modernize-make-unique --- .../modernize/MakeSmartPtrCheck.cpp | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp index 35d8d90b3d51e9..8b267fb4d93394 100644 --- a/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp @@ -84,19 +84,18 @@ void MakeSmartPtrCheck::registerMatchers(ast_matchers::MatchFinder *Finder) { auto IsPlacement = hasAnyPlacementArg(anything()); Finder->addMatcher( - traverse( - TK_AsIs, - cxxConstructExpr( - anyOf(hasParent(cxxBindTemporaryExpr()), - hasParent(varDecl().bind(DirectVar))), - hasType(getSmartPointerTypeMatcher()), argumentCountIs(1), - hasArgument( - 0, cxxNewExpr(hasType(pointsTo(qualType(hasCanonicalType( - equalsBoundNode(PointerType))))), - CanCallCtor, unless(IsPlacement)) - .bind(NewExpression)), - unless(isInTemplateInstantiation())) - .bind(ConstructorCall)), + traverse(TK_AsIs, + cxxConstructExpr( + anyOf(hasParent(cxxBindTemporaryExpr()), + hasParent(varDecl().bind(DirectVar))), + hasType(getSmartPointerTypeMatcher()), argumentCountIs(1), + hasArgument( + 0, cxxNewExpr(hasType(pointsTo(qualType(hasCanonicalType( + equalsBoundNode(PointerType))))), + CanCallCtor, unless(IsPlacement)) + .bind(NewExpression)), + unless(isInTemplateInstantiation())) + .bind(ConstructorCall)), this); Finder->addMatcher( >From 235b4fb2f62ce25918d22cbec681d86c4830de49 Mon Sep 17 00:00:00 2001 From: serge-sans-paille <[email protected]> Date: Thu, 22 Oct 2026 16:54:59 +0200 Subject: [PATCH 6/6] fixup! fixup! fixup! fixup! fixup! fixup! [clang-tidy] Fix false positive in modernize-make-unique --- .../clang-tidy/modernize/MakeSmartPtrCheck.cpp | 15 +++++++++++++-- .../make-unique-inaccessible-dtors.cpp | 17 ++++++++++++++++- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp index 8b267fb4d93394..3744066769d55b 100644 --- a/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp @@ -14,6 +14,18 @@ using namespace clang::ast_matchers; +namespace { +AST_MATCHER(clang::Decl, hasInaccessibleDtor) { + if (const auto *RD = llvm::dyn_cast<clang::CXXRecordDecl>(&Node)) { + if (const auto *Destructor = RD->getDestructor()) { + return Destructor->isDeleted() || + Destructor->getAccess() != clang::AS_public; + } + } + return false; +} +} // namespace + namespace clang::tidy::modernize { static constexpr char ConstructorCall[] = "constructorCall"; @@ -78,8 +90,7 @@ void MakeSmartPtrCheck::registerMatchers(ast_matchers::MatchFinder *Finder) { // conservative in that case. const auto CanCallDtor = unless(hasType(hasUnqualifiedDesugaredType( pointerType(pointee(hasUnqualifiedDesugaredType( - recordType(hasDeclaration(has(cxxDestructorDecl( - anyOf(isDeleted(), unless(isPublic())))))))))))); + recordType(hasDeclaration(hasInaccessibleDtor())))))))); auto IsPlacement = hasAnyPlacementArg(anything()); diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/make-unique-inaccessible-dtors.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/make-unique-inaccessible-dtors.cpp index c8c2d028a65762..7f767858f603fa 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/make-unique-inaccessible-dtors.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/make-unique-inaccessible-dtors.cpp @@ -1,4 +1,4 @@ -// RUN: %check_clang_tidy -std=c++14,c++17 %s modernize-make-unique %t -- -- -I %S/Inputs/smart-ptr -D CXX_14_17=1 +// RUN: %check_clang_tidy -std=c++20 %s modernize-make-unique %t -- -- -I %S/Inputs/smart-ptr -D CXX_14_17=1 #include "unique_ptr.h" // CHECK-FIXES: #include <memory> @@ -40,6 +40,13 @@ struct TemplatedChildWithDeletedDestructor: Base { ~TemplatedChildWithDeletedDestructor() = delete; }; +template <class T> +struct TemplatedWithMultipleDestructors { + ~TemplatedWithMultipleDestructors() requires(sizeof(T) > 1) = default; // selected for TemplatedWithMultipleDestructors<int> +private: + ~TemplatedWithMultipleDestructors() requires(sizeof(T) == 1) = default; // selected for TemplatedWithMultipleDestructors<char> +}; + void check_reset() { std::unique_ptr<Base> p; p.reset(new ChildWithPublicDestructor()); @@ -56,4 +63,12 @@ void check_reset() { p.reset(new ChildWithDeletedDestructor()); p.reset(new ChildWithDeletedDestructorAlias()); p.reset(new TemplatedChildWithDeletedDestructor<int>()); + + std::unique_ptr<TemplatedWithMultipleDestructors<int>> t0; + t0.reset(new TemplatedWithMultipleDestructors<int>()); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use std::make_unique instead + // CHECK-FIXES: t0 = std::make_unique<TemplatedWithMultipleDestructors<int>>(); + + std::unique_ptr<TemplatedWithMultipleDestructors<char>> t1; + t1.reset(new TemplatedWithMultipleDestructors<char>()); } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
