Alexander Kornienko | 5b982e5 | 2015-03-09 11:48:54 +0000 | [diff] [blame] | 1 | //===--- UniqueptrResetReleaseCheck.cpp - clang-tidy ----------------------===// |
Alexander Kornienko | bc0c423 | 2014-12-05 11:59:05 +0000 | [diff] [blame] | 2 | // |
| 3 | // The LLVM Compiler Infrastructure |
| 4 | // |
| 5 | // This file is distributed under the University of Illinois Open Source |
| 6 | // License. See LICENSE.TXT for details. |
| 7 | // |
| 8 | //===----------------------------------------------------------------------===// |
| 9 | |
Alexander Kornienko | 5b982e5 | 2015-03-09 11:48:54 +0000 | [diff] [blame] | 10 | #include "UniqueptrResetReleaseCheck.h" |
Alexander Kornienko | bc0c423 | 2014-12-05 11:59:05 +0000 | [diff] [blame] | 11 | #include "clang/ASTMatchers/ASTMatchFinder.h" |
| 12 | #include "clang/Lex/Lexer.h" |
| 13 | |
| 14 | using namespace clang::ast_matchers; |
| 15 | |
| 16 | namespace clang { |
| 17 | namespace tidy { |
Alexander Kornienko | 2b312420 | 2015-03-02 12:25:03 +0000 | [diff] [blame] | 18 | namespace misc { |
Alexander Kornienko | bc0c423 | 2014-12-05 11:59:05 +0000 | [diff] [blame] | 19 | |
Alexander Kornienko | 5b982e5 | 2015-03-09 11:48:54 +0000 | [diff] [blame] | 20 | void UniqueptrResetReleaseCheck::registerMatchers(MatchFinder *Finder) { |
Aaron Ballman | 327e97b | 2015-08-28 19:27:19 +0000 | [diff] [blame] | 21 | // Only register the matchers for C++11; the functionality currently does not |
| 22 | // provide any benefit to other languages, despite being benign. |
Aaron Ballman | bf89109 | 2015-08-31 15:28:57 +0000 | [diff] [blame] | 23 | if (!getLangOpts().CPlusPlus11) |
| 24 | return; |
| 25 | |
| 26 | Finder->addMatcher( |
Aaron Ballman | b9ea09c | 2015-09-17 13:31:25 +0000 | [diff] [blame] | 27 | cxxMemberCallExpr( |
Aaron Ballman | bf89109 | 2015-08-31 15:28:57 +0000 | [diff] [blame] | 28 | on(expr().bind("left")), callee(memberExpr().bind("reset_member")), |
Aaron Ballman | b9ea09c | 2015-09-17 13:31:25 +0000 | [diff] [blame] | 29 | callee( |
| 30 | cxxMethodDecl(hasName("reset"), |
| 31 | ofClass(cxxRecordDecl(hasName("::std::unique_ptr"), |
| 32 | decl().bind("left_class"))))), |
Piotr Padlewski | e93a73f | 2016-05-31 15:26:56 +0000 | [diff] [blame] | 33 | has(ignoringParenImpCasts(cxxMemberCallExpr( |
Aaron Ballman | bf89109 | 2015-08-31 15:28:57 +0000 | [diff] [blame] | 34 | on(expr().bind("right")), |
| 35 | callee(memberExpr().bind("release_member")), |
Aaron Ballman | b9ea09c | 2015-09-17 13:31:25 +0000 | [diff] [blame] | 36 | callee(cxxMethodDecl( |
Aaron Ballman | bf89109 | 2015-08-31 15:28:57 +0000 | [diff] [blame] | 37 | hasName("release"), |
Aaron Ballman | b9ea09c | 2015-09-17 13:31:25 +0000 | [diff] [blame] | 38 | ofClass(cxxRecordDecl(hasName("::std::unique_ptr"), |
Piotr Padlewski | e93a73f | 2016-05-31 15:26:56 +0000 | [diff] [blame] | 39 | decl().bind("right_class"))))))))) |
Aaron Ballman | bf89109 | 2015-08-31 15:28:57 +0000 | [diff] [blame] | 40 | .bind("reset_call"), |
| 41 | this); |
Alexander Kornienko | bc0c423 | 2014-12-05 11:59:05 +0000 | [diff] [blame] | 42 | } |
| 43 | |
Samuel Benzaquen | 91d85dc | 2015-04-09 17:51:01 +0000 | [diff] [blame] | 44 | namespace { |
| 45 | const Type *getDeleterForUniquePtr(const MatchFinder::MatchResult &Result, |
| 46 | StringRef ID) { |
| 47 | const auto *Class = |
| 48 | Result.Nodes.getNodeAs<ClassTemplateSpecializationDecl>(ID); |
| 49 | if (!Class) |
| 50 | return nullptr; |
| 51 | auto DeleterArgument = Class->getTemplateArgs()[1]; |
| 52 | if (DeleterArgument.getKind() != TemplateArgument::Type) |
| 53 | return nullptr; |
| 54 | return DeleterArgument.getAsType().getTypePtr(); |
| 55 | } |
| 56 | |
| 57 | bool areDeletersCompatible(const MatchFinder::MatchResult &Result) { |
| 58 | const Type *LeftDeleterType = getDeleterForUniquePtr(Result, "left_class"); |
| 59 | const Type *RightDeleterType = getDeleterForUniquePtr(Result, "right_class"); |
| 60 | |
| 61 | if (LeftDeleterType->getUnqualifiedDesugaredType() == |
| 62 | RightDeleterType->getUnqualifiedDesugaredType()) { |
| 63 | // Same type. We assume they are compatible. |
| 64 | // This check handles the case where the deleters are function pointers. |
| 65 | return true; |
| 66 | } |
| 67 | |
| 68 | const CXXRecordDecl *LeftDeleter = LeftDeleterType->getAsCXXRecordDecl(); |
| 69 | const CXXRecordDecl *RightDeleter = RightDeleterType->getAsCXXRecordDecl(); |
| 70 | if (!LeftDeleter || !RightDeleter) |
| 71 | return false; |
| 72 | |
| 73 | if (LeftDeleter->getCanonicalDecl() == RightDeleter->getCanonicalDecl()) { |
| 74 | // Same class. We assume they are compatible. |
| 75 | return true; |
| 76 | } |
| 77 | |
| 78 | const auto *LeftAsTemplate = |
| 79 | dyn_cast<ClassTemplateSpecializationDecl>(LeftDeleter); |
| 80 | const auto *RightAsTemplate = |
| 81 | dyn_cast<ClassTemplateSpecializationDecl>(RightDeleter); |
| 82 | if (LeftAsTemplate && RightAsTemplate && |
| 83 | LeftAsTemplate->getSpecializedTemplate() == |
| 84 | RightAsTemplate->getSpecializedTemplate()) { |
| 85 | // They are different instantiations of the same template. We assume they |
| 86 | // are compatible. |
| 87 | // This handles things like std::default_delete<Base> vs. |
| 88 | // std::default_delete<Derived>. |
| 89 | return true; |
| 90 | } |
| 91 | return false; |
| 92 | } |
| 93 | |
Mandeep Singh Grang | 7c7ea7d | 2016-11-08 07:50:19 +0000 | [diff] [blame] | 94 | } // namespace |
Samuel Benzaquen | 91d85dc | 2015-04-09 17:51:01 +0000 | [diff] [blame] | 95 | |
Alexander Kornienko | 5b982e5 | 2015-03-09 11:48:54 +0000 | [diff] [blame] | 96 | void UniqueptrResetReleaseCheck::check(const MatchFinder::MatchResult &Result) { |
Samuel Benzaquen | 91d85dc | 2015-04-09 17:51:01 +0000 | [diff] [blame] | 97 | if (!areDeletersCompatible(Result)) |
| 98 | return; |
| 99 | |
Alexander Kornienko | bc0c423 | 2014-12-05 11:59:05 +0000 | [diff] [blame] | 100 | const auto *ResetMember = Result.Nodes.getNodeAs<MemberExpr>("reset_member"); |
| 101 | const auto *ReleaseMember = |
| 102 | Result.Nodes.getNodeAs<MemberExpr>("release_member"); |
| 103 | const auto *Right = Result.Nodes.getNodeAs<Expr>("right"); |
| 104 | const auto *Left = Result.Nodes.getNodeAs<Expr>("left"); |
| 105 | const auto *ResetCall = |
| 106 | Result.Nodes.getNodeAs<CXXMemberCallExpr>("reset_call"); |
| 107 | |
| 108 | std::string LeftText = clang::Lexer::getSourceText( |
| 109 | CharSourceRange::getTokenRange(Left->getSourceRange()), |
Gabor Horvath | afad84c | 2016-09-24 02:13:45 +0000 | [diff] [blame] | 110 | *Result.SourceManager, getLangOpts()); |
Alexander Kornienko | bc0c423 | 2014-12-05 11:59:05 +0000 | [diff] [blame] | 111 | std::string RightText = clang::Lexer::getSourceText( |
| 112 | CharSourceRange::getTokenRange(Right->getSourceRange()), |
Gabor Horvath | afad84c | 2016-09-24 02:13:45 +0000 | [diff] [blame] | 113 | *Result.SourceManager, getLangOpts()); |
Alexander Kornienko | bc0c423 | 2014-12-05 11:59:05 +0000 | [diff] [blame] | 114 | |
| 115 | if (ResetMember->isArrow()) |
| 116 | LeftText = "*" + LeftText; |
| 117 | if (ReleaseMember->isArrow()) |
| 118 | RightText = "*" + RightText; |
Alexander Kornienko | ed07a25 | 2015-03-05 13:53:21 +0000 | [diff] [blame] | 119 | std::string DiagText; |
Alexander Kornienko | bc0c423 | 2014-12-05 11:59:05 +0000 | [diff] [blame] | 120 | // Even if x was rvalue, *x is not rvalue anymore. |
Alexander Kornienko | ed07a25 | 2015-03-05 13:53:21 +0000 | [diff] [blame] | 121 | if (!Right->isRValue() || ReleaseMember->isArrow()) { |
Alexander Kornienko | bc0c423 | 2014-12-05 11:59:05 +0000 | [diff] [blame] | 122 | RightText = "std::move(" + RightText + ")"; |
Alexander Kornienko | ed07a25 | 2015-03-05 13:53:21 +0000 | [diff] [blame] | 123 | DiagText = "prefer ptr1 = std::move(ptr2) over ptr1.reset(ptr2.release())"; |
| 124 | } else { |
| 125 | DiagText = |
| 126 | "prefer ptr = ReturnUnique() over ptr.reset(ReturnUnique().release())"; |
| 127 | } |
Alexander Kornienko | bc0c423 | 2014-12-05 11:59:05 +0000 | [diff] [blame] | 128 | std::string NewText = LeftText + " = " + RightText; |
| 129 | |
Mandeep Singh Grang | 7c7ea7d | 2016-11-08 07:50:19 +0000 | [diff] [blame] | 130 | diag(ResetMember->getExprLoc(), DiagText) << FixItHint::CreateReplacement( |
| 131 | CharSourceRange::getTokenRange(ResetCall->getSourceRange()), NewText); |
Alexander Kornienko | bc0c423 | 2014-12-05 11:59:05 +0000 | [diff] [blame] | 132 | } |
| 133 | |
Alexander Kornienko | 2b312420 | 2015-03-02 12:25:03 +0000 | [diff] [blame] | 134 | } // namespace misc |
Alexander Kornienko | bc0c423 | 2014-12-05 11:59:05 +0000 | [diff] [blame] | 135 | } // namespace tidy |
| 136 | } // namespace clang |