Index: clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp +++ clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp @@ -30,6 +30,10 @@ .bind("redundant_get"); } +internal::Matcher knownSmartptr() { + return recordDecl(hasAnyName("::std::unique_ptr", "::std::shared_ptr")); +} + void registerMatchersForGetArrowStart(MatchFinder *Finder, MatchFinder::MatchCallback *Callback) { const auto QuacksLikeASmartptr = recordDecl( @@ -39,21 +43,21 @@ has(cxxMethodDecl(hasName("operator*"), returns(qualType(references( type().bind("op*Type"))))))); + // Make sure we are not missing the known standard types + const auto Smartptr = anyOf(knownSmartptr(), QuacksLikeASmartptr); + // Catch 'ptr.get()->Foo()' Finder->addMatcher(memberExpr(expr().bind("memberExpr"), isArrow(), - hasObjectExpression(ignoringImpCasts( - callToGet(QuacksLikeASmartptr)))), + hasObjectExpression(ignoringImpCasts(callToGet(Smartptr)))), Callback); // Catch '*ptr.get()' or '*ptr->get()' Finder->addMatcher( - unaryOperator(hasOperatorName("*"), - hasUnaryOperand(callToGet(QuacksLikeASmartptr))), + unaryOperator(hasOperatorName("*"), hasUnaryOperand(callToGet(Smartptr))), Callback); // Catch '!ptr.get()' - const auto CallToGetAsBool = ignoringParenImpCasts(callToGet(recordDecl( - QuacksLikeASmartptr, has(cxxConversionDecl(returns(booleanType())))))); + const auto CallToGetAsBool = ignoringParenImpCasts(callToGet(recordDecl(Smartptr, has(cxxConversionDecl(returns(booleanType())))))); Finder->addMatcher( unaryOperator(hasOperatorName("!"), hasUnaryOperand(CallToGetAsBool)), Callback); @@ -71,10 +75,7 @@ // This one is harder to do with duck typing. // The operator==/!= that we are looking for might be member or non-member, // 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(hasAnyName("::std::unique_ptr", "::std::shared_ptr")); + // For now, lets keep it to the known standard types. // Matches against nullptr. Finder->addMatcher( @@ -82,7 +83,7 @@ hasEitherOperand(ignoringImpCasts( anyOf(cxxNullPtrLiteralExpr(), gnuNullExpr(), integerLiteral(equals(0))))), - hasEitherOperand(callToGet(IsAKnownSmartptr))), + hasEitherOperand(callToGet(knownSmartptr()))), Callback); // FIXME: Match and fix if (l.get() == r.get()). Index: clang-tools-extra/test/clang-tidy/readability-redundant-smartptr-get-msvc.cpp =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/readability-redundant-smartptr-get-msvc.cpp @@ -0,0 +1,95 @@ +// RUN: %check_clang_tidy %s readability-redundant-smartptr-get %t + +#define NULL __null + +namespace std { + +// MSVC headers define operator templates instead of plain operators. + +template +struct unique_ptr { + template + T2& operator*() const; + template + T2* operator->() const; + T* get() const; + explicit operator bool() const noexcept; +}; + +template +struct shared_ptr { + template + T2& operator*() const; + template + T2* operator->() const; + T* get() const; + explicit operator bool() const noexcept; +}; + +} // namespace std + +struct Bar { + void Do(); + void ConstDo() const; +}; + +void Positive() { + + std::unique_ptr* up; + (*up->get()).Do(); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant get() call + // CHECK-MESSAGES: (*up->get()).Do(); + // CHECK-FIXES: (**up).Do(); + + std::unique_ptr uu; + std::shared_ptr *ss; + bool bb = uu.get() == nullptr; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: redundant get() call + // CHECK-MESSAGES: uu.get() == nullptr; + // CHECK-FIXES: bool bb = uu == nullptr; + + if (up->get()); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant get() call + // CHECK-MESSAGES: if (up->get()); + // CHECK-FIXES: if (*up); + if ((uu.get())); + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: redundant get() call + // CHECK-MESSAGES: if ((uu.get())); + // CHECK-FIXES: if ((uu)); + bb = !ss->get(); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant get() call + // CHECK-MESSAGES: bb = !ss->get(); + // CHECK-FIXES: bb = !*ss; + + bb = nullptr != ss->get(); + // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: redundant get() call + // CHECK-MESSAGES: nullptr != ss->get(); + // CHECK-FIXES: bb = nullptr != *ss; + + 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); +} \ No newline at end of file