diff --git a/clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp index 3d1c863843f6..5aad5b5213b8 100644 --- a/clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp @@ -60,21 +60,27 @@ void registerMatchersForGetEquals(MatchFinder *Finder, // 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); - // TODO: Catch ptr.get() == other_ptr.get() + + // Matches against if(ptr.get()) + Finder->addMatcher( + ifStmt(hasCondition(ignoringImpCasts(callToGet(IsAKnownSmartptr)))), + Callback); + + // 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 MatchFinder::MatchResult &Result) { Result.Nodes.getNodeAs("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("ptr_to_ptr") != nullptr; bool IsMemberExpr = Result.Nodes.getNodeAs("memberExpr") != nullptr; diff --git a/clang-tools-extra/test/clang-tidy/readability-redundant-smartptr-get.cpp b/clang-tools-extra/test/clang-tidy/readability-redundant-smartptr-get.cpp index 61a36f6dee3f..985efc58e421 100644 --- a/clang-tools-extra/test/clang-tidy/readability-redundant-smartptr-get.cpp +++ b/clang-tools-extra/test/clang-tidy/readability-redundant-smartptr-get.cpp @@ -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().get() == NULL; + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: redundant get() call + // CHECK-MESSAGES: bb = std::unique_ptr().get() == NULL; + // CHECK-FIXES: bb = std::unique_ptr() == 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 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().get() == NULL; - bb = ip.get() == nullptr; - bb = u->get() == NULL; + bool bb = ip.get() == nullptr; }