[clang-tools-extra] [clang-tidy] Fix false positive in modernize-make-unique (PR #229802)
via cfe-commits
cfe-commits at lists.llvm.org
Sat Oct 17 07:38:43 PDT 2026
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 <sguelton at mozilla.com>
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 <sguelton at mozilla.com>
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());
}
More information about the cfe-commits
mailing list