On Thu, Apr 9, 2015 at 10:51 AM, Samuel Benzaquen <[email protected]> wrote:
> Author: sbenza > Date: Thu Apr 9 12:51:01 2015 > New Revision: 234512 > > URL: http://llvm.org/viewvc/llvm-project?rev=234512&view=rev > Log: > [clang-tidy] Ignore expressions with incompatible deleters. > > Summary: > Do not warn on .reset(.release()) expressions if the deleters are not > compatible. > Using plain assignment will probably not work. > Should we have a separate check for that, then? It seems error prone... > > Reviewers: klimek > > Subscribers: curdeius, cfe-commits > > Differential Revision: http://reviews.llvm.org/D8422 > > Modified: > clang-tools-extra/trunk/clang-tidy/misc/UniqueptrResetReleaseCheck.cpp > > clang-tools-extra/trunk/test/clang-tidy/misc-uniqueptr-reset-release.cpp > > Modified: > clang-tools-extra/trunk/clang-tidy/misc/UniqueptrResetReleaseCheck.cpp > URL: > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/UniqueptrResetReleaseCheck.cpp?rev=234512&r1=234511&r2=234512&view=diff > > ============================================================================== > --- clang-tools-extra/trunk/clang-tidy/misc/UniqueptrResetReleaseCheck.cpp > (original) > +++ clang-tools-extra/trunk/clang-tidy/misc/UniqueptrResetReleaseCheck.cpp > Thu Apr 9 12:51:01 2015 > @@ -22,17 +22,75 @@ void UniqueptrResetReleaseCheck::registe > 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<ClassTemplateSpecializationDecl>(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<ClassTemplateSpecializationDecl>(LeftDeleter); > + const auto *RightAsTemplate = > + dyn_cast<ClassTemplateSpecializationDecl>(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<Base> vs. > + // std::default_delete<Derived>. > + return true; > + } > + return false; > +} > + > +} // namespace > + > void UniqueptrResetReleaseCheck::check(const MatchFinder::MatchResult > &Result) { > + if (!areDeletersCompatible(Result)) > + return; > + > const auto *ResetMember = > Result.Nodes.getNodeAs<MemberExpr>("reset_member"); > const auto *ReleaseMember = > Result.Nodes.getNodeAs<MemberExpr>("release_member"); > > Modified: > clang-tools-extra/trunk/test/clang-tidy/misc-uniqueptr-reset-release.cpp > URL: > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-uniqueptr-reset-release.cpp?rev=234512&r1=234511&r2=234512&view=diff > > ============================================================================== > --- > clang-tools-extra/trunk/test/clang-tidy/misc-uniqueptr-reset-release.cpp > (original) > +++ > clang-tools-extra/trunk/test/clang-tidy/misc-uniqueptr-reset-release.cpp > Thu Apr 9 12:51:01 2015 > @@ -2,12 +2,16 @@ > // REQUIRES: shell > > namespace std { > + > template <typename T> > +struct default_delete {}; > + > +template <typename T, class Deleter = std::default_delete<T>> > struct unique_ptr { > - unique_ptr<T>(); > - explicit unique_ptr<T>(T *); > - template <typename U> > - unique_ptr<T>(unique_ptr<U> &&); > + unique_ptr(); > + explicit unique_ptr(T *); > + template <typename U, typename E> > + unique_ptr(unique_ptr<U, E> &&); > void reset(T *); > T *release(); > }; > @@ -20,6 +24,9 @@ std::unique_ptr<Foo> Create(); > std::unique_ptr<Foo> &Look(); > std::unique_ptr<Foo> *Get(); > > +using FooFunc = void (*)(Foo *); > +using BarFunc = void (*)(Bar *); > + > void f() { > std::unique_ptr<Foo> a, b; > std::unique_ptr<Bar> 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<Bar, FooFunc> 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<Foo> src; > + struct OtherDeleter {}; > + std::unique_ptr<Foo, OtherDeleter> dest; > + dest.reset(src.release()); > + > + std::unique_ptr<Bar, FooFunc> func_a; > + std::unique_ptr<Bar, BarFunc> func_b; > + func_a.reset(func_b.release()); > +} > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
