diff --git a/clang-tools-extra/clang-tidy/misc/UniqueptrResetReleaseCheck.cpp b/clang-tools-extra/clang-tidy/misc/UniqueptrResetReleaseCheck.cpp index d7ba7e73ae5f..436cd1012167 100644 --- a/clang-tools-extra/clang-tidy/misc/UniqueptrResetReleaseCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/UniqueptrResetReleaseCheck.cpp @@ -22,17 +22,75 @@ void UniqueptrResetReleaseCheck::registerMatchers(MatchFinder *Finder) { memberCallExpr( on(expr().bind("left")), callee(memberExpr().bind("reset_member")), callee(methodDecl(hasName("reset"), - ofClass(hasName("::std::unique_ptr")))), + ofClass(recordDecl(hasName("::std::unique_ptr"), + decl().bind("left_class"))))), has(memberCallExpr( on(expr().bind("right")), callee(memberExpr().bind("release_member")), - callee(methodDecl(hasName("release"), - ofClass(hasName("::std::unique_ptr"))))))) + callee(methodDecl( + hasName("release"), + ofClass(recordDecl(hasName("::std::unique_ptr"), + decl().bind("right_class")))))))) .bind("reset_call"), this); } +namespace { +const Type *getDeleterForUniquePtr(const MatchFinder::MatchResult &Result, + StringRef ID) { + const auto *Class = + Result.Nodes.getNodeAs(ID); + if (!Class) + return nullptr; + auto DeleterArgument = Class->getTemplateArgs()[1]; + if (DeleterArgument.getKind() != TemplateArgument::Type) + return nullptr; + return DeleterArgument.getAsType().getTypePtr(); +} + +bool areDeletersCompatible(const MatchFinder::MatchResult &Result) { + const Type *LeftDeleterType = getDeleterForUniquePtr(Result, "left_class"); + const Type *RightDeleterType = getDeleterForUniquePtr(Result, "right_class"); + + if (LeftDeleterType->getUnqualifiedDesugaredType() == + RightDeleterType->getUnqualifiedDesugaredType()) { + // Same type. We assume they are compatible. + // This check handles the case where the deleters are function pointers. + return true; + } + + const CXXRecordDecl *LeftDeleter = LeftDeleterType->getAsCXXRecordDecl(); + const CXXRecordDecl *RightDeleter = RightDeleterType->getAsCXXRecordDecl(); + if (!LeftDeleter || !RightDeleter) + return false; + + if (LeftDeleter->getCanonicalDecl() == RightDeleter->getCanonicalDecl()) { + // Same class. We assume they are compatible. + return true; + } + + const auto *LeftAsTemplate = + dyn_cast(LeftDeleter); + const auto *RightAsTemplate = + dyn_cast(RightDeleter); + if (LeftAsTemplate && RightAsTemplate && + LeftAsTemplate->getSpecializedTemplate() == + RightAsTemplate->getSpecializedTemplate()) { + // They are different instantiations of the same template. We assume they + // are compatible. + // This handles things like std::default_delete vs. + // std::default_delete. + return true; + } + return false; +} + +} // namespace + void UniqueptrResetReleaseCheck::check(const MatchFinder::MatchResult &Result) { + if (!areDeletersCompatible(Result)) + return; + const auto *ResetMember = Result.Nodes.getNodeAs("reset_member"); const auto *ReleaseMember = Result.Nodes.getNodeAs("release_member"); diff --git a/clang-tools-extra/test/clang-tidy/misc-uniqueptr-reset-release.cpp b/clang-tools-extra/test/clang-tidy/misc-uniqueptr-reset-release.cpp index 2839f167b9f3..3d1183991f20 100644 --- a/clang-tools-extra/test/clang-tidy/misc-uniqueptr-reset-release.cpp +++ b/clang-tools-extra/test/clang-tidy/misc-uniqueptr-reset-release.cpp @@ -2,12 +2,16 @@ // REQUIRES: shell namespace std { + template +struct default_delete {}; + +template > struct unique_ptr { - unique_ptr(); - explicit unique_ptr(T *); - template - unique_ptr(unique_ptr &&); + unique_ptr(); + explicit unique_ptr(T *); + template + unique_ptr(unique_ptr &&); void reset(T *); T *release(); }; @@ -20,6 +24,9 @@ std::unique_ptr Create(); std::unique_ptr &Look(); std::unique_ptr *Get(); +using FooFunc = void (*)(Foo *); +using BarFunc = void (*)(Bar *); + void f() { std::unique_ptr a, b; std::unique_ptr c; @@ -44,5 +51,20 @@ void f() { Get()->reset(Get()->release()); // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: prefer ptr1 = std::move(ptr2) // CHECK-FIXES: *Get() = std::move(*Get()); + + std::unique_ptr func_a, func_b; + func_a.reset(func_b.release()); + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: prefer ptr1 = std::move(ptr2) + // CHECK-FIXES: func_a = std::move(func_b); } +void negatives() { + std::unique_ptr src; + struct OtherDeleter {}; + std::unique_ptr dest; + dest.reset(src.release()); + + std::unique_ptr func_a; + std::unique_ptr func_b; + func_a.reset(func_b.release()); +}