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/2] [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/2] 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()); } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
