[clang-tools-extra] r234512 - [clang-tidy] Ignore expressions with incompatible deleters.
David Blaikie
dblaikie at gmail.com
Thu Apr 9 11:08:01 PDT 2015
On Thu, Apr 9, 2015 at 10:51 AM, Samuel Benzaquen <sbenza at google.com> 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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150409/a86c8ee6/attachment.html>
More information about the cfe-commits
mailing list