Author: omtcyfz Date: Mon Sep 26 02:22:37 2016 New Revision: 282386 URL: http://llvm.org/viewvc/llvm-project?rev=282386&view=rev Log: [clang-tidy] make readability-redundant-smartptr-get report get() usage in conditions
This patch extends clang-tidy's readability-redundant-smartptr-get to produce warnings for previously unsupported cases: ``` std::unique_ptr<void> ptr; if (ptr.get()) if (ptr.get() == NULL) if (ptr.get() != NULL) ``` This is intended to fix https://llvm.org/bugs/show_bug.cgi?id=25804, a bug report opened by @Eugene.Zelenko. However, there still are cases not detected by the check. They can be found in `void Negative()` function defined in test/clang-tidy/readability-redundant-smartptr-get.cpp. Reviewers: alexfh Differential Revision: https://reviews.llvm.org/D24893 Modified: clang-tools-extra/trunk/clang-tidy/readability/RedundantSmartptrGetCheck.cpp clang-tools-extra/trunk/test/clang-tidy/readability-redundant-smartptr-get.cpp Modified: clang-tools-extra/trunk/clang-tidy/readability/RedundantSmartptrGetCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/RedundantSmartptrGetCheck.cpp?rev=282386&r1=282385&r2=282386&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/readability/RedundantSmartptrGetCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/readability/RedundantSmartptrGetCheck.cpp Mon Sep 26 02:22:37 2016 @@ -60,21 +60,27 @@ void registerMatchersForGetEquals(MatchF // might be on global namespace or found by ADL, might be a template, etc. // For now, lets keep a list of known standard types. - const auto IsAKnownSmartptr = recordDecl( - anyOf(hasName("::std::unique_ptr"), hasName("::std::shared_ptr"))); + const auto IsAKnownSmartptr = + recordDecl(hasAnyName("::std::unique_ptr", "::std::shared_ptr")); // Matches against nullptr. Finder->addMatcher( - binaryOperator( - anyOf(hasOperatorName("=="), hasOperatorName("!=")), - hasEitherOperand(ignoringImpCasts(cxxNullPtrLiteralExpr())), - hasEitherOperand(callToGet(IsAKnownSmartptr))), + binaryOperator(anyOf(hasOperatorName("=="), hasOperatorName("!=")), + hasEitherOperand(ignoringImpCasts( + anyOf(cxxNullPtrLiteralExpr(), gnuNullExpr(), + integerLiteral(equals(0))))), + hasEitherOperand(callToGet(IsAKnownSmartptr))), + Callback); + + // Matches against if(ptr.get()) + Finder->addMatcher( + ifStmt(hasCondition(ignoringImpCasts(callToGet(IsAKnownSmartptr)))), Callback); - // TODO: Catch ptr.get() == other_ptr.get() -} + // FIXME: Match and fix if (l.get() == r.get()). +} -} // namespace +} // namespace void RedundantSmartptrGetCheck::registerMatchers(MatchFinder *Finder) { // Only register the matchers for C++; the functionality currently does not @@ -102,10 +108,11 @@ bool allReturnTypesMatch(const MatchFind Result.Nodes.getNodeAs<Type>("getType")->getUnqualifiedDesugaredType(); return OpArrowType == OpStarType && OpArrowType == GetType; } -} // namespace +} // namespace void RedundantSmartptrGetCheck::check(const MatchFinder::MatchResult &Result) { - if (!allReturnTypesMatch(Result)) return; + if (!allReturnTypesMatch(Result)) + return; bool IsPtrToPtr = Result.Nodes.getNodeAs<Decl>("ptr_to_ptr") != nullptr; bool IsMemberExpr = Result.Nodes.getNodeAs<Expr>("memberExpr") != nullptr; Modified: clang-tools-extra/trunk/test/clang-tidy/readability-redundant-smartptr-get.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-redundant-smartptr-get.cpp?rev=282386&r1=282385&r2=282386&view=diff ============================================================================== --- clang-tools-extra/trunk/test/clang-tidy/readability-redundant-smartptr-get.cpp (original) +++ clang-tools-extra/trunk/test/clang-tidy/readability-redundant-smartptr-get.cpp Mon Sep 26 02:22:37 2016 @@ -113,6 +113,37 @@ void Positive() { // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: redundant get() call // CHECK-MESSAGES: i = *PointerWithOverloadedGet().get(); // CHECK-FIXES: i = *PointerWithOverloadedGet(); + + bb = std::unique_ptr<int>().get() == NULL; + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: redundant get() call + // CHECK-MESSAGES: bb = std::unique_ptr<int>().get() == NULL; + // CHECK-FIXES: bb = std::unique_ptr<int>() == NULL; + bb = ss->get() == NULL; + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: redundant get() call + // CHECK-MESSAGES: bb = ss->get() == NULL; + // CHECK-FIXES: bb = *ss == NULL; + + std::unique_ptr<int> x, y; + if (x.get() == nullptr); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant get() call + // CHECK-MESSAGES: if (x.get() == nullptr); + // CHECK-FIXES: if (x == nullptr); + if (nullptr == y.get()); + // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: redundant get() call + // CHECK-MESSAGES: if (nullptr == y.get()); + // CHECK-FIXES: if (nullptr == y); + if (x.get() == NULL); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant get() call + // CHECK-MESSAGES: if (x.get() == NULL); + // CHECK-FIXES: if (x == NULL); + if (NULL == x.get()); + // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: redundant get() call + // CHECK-MESSAGES: if (NULL == x.get()); + // CHECK-FIXES: if (NULL == x); + if (x.get()); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant get() call + // CHECK-MESSAGES: if (x.get()); + // CHECK-FIXES: if (x); } void Negative() { @@ -137,7 +168,5 @@ void Negative() { (*Fail2().get()).Do(); int_ptr ip; - bool bb = std::unique_ptr<int>().get() == NULL; - bb = ip.get() == nullptr; - bb = u->get() == NULL; + bool bb = ip.get() == nullptr; } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
