diff --git a/clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp index be52af77ae0a5..baa977750d101 100644 --- a/clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp @@ -49,29 +49,41 @@ internal::Matcher knownSmartptr() { void registerMatchersForGetArrowStart(MatchFinder *Finder, MatchFinder::MatchCallback *Callback) { - const auto QuacksLikeASmartptr = recordDecl( - recordDecl().bind("duck_typing"), - has(cxxMethodDecl(hasName("operator->"), - returns(qualType(pointsTo(type().bind("op->Type")))))), - has(cxxMethodDecl(hasName("operator*"), returns(qualType(references( - type().bind("op*Type"))))))); + const auto MatchesOpArrow = + allOf(hasName("operator->"), + returns(qualType(pointsTo(type().bind("op->Type"))))); + const auto MatchesOpStar = + allOf(hasName("operator*"), + returns(qualType(references(type().bind("op*Type"))))); + const auto HasRelevantOps = + allOf(anyOf(hasMethod(MatchesOpArrow), + has(functionTemplateDecl(has(functionDecl(MatchesOpArrow))))), + anyOf(hasMethod(MatchesOpStar), + has(functionTemplateDecl(has(functionDecl(MatchesOpStar)))))); + + const auto QuacksLikeASmartptr = + cxxRecordDecl(cxxRecordDecl().bind("duck_typing"), HasRelevantOps); // Make sure we are not missing the known standard types. - const auto Smartptr = anyOf(knownSmartptr(), QuacksLikeASmartptr); + const auto SmartptrAny = anyOf(knownSmartptr(), QuacksLikeASmartptr); + const auto SmartptrWithDeref = + anyOf(cxxRecordDecl(knownSmartptr(), HasRelevantOps), QuacksLikeASmartptr); // Catch 'ptr.get()->Foo()' - Finder->addMatcher(memberExpr(expr().bind("memberExpr"), isArrow(), - hasObjectExpression(callToGet(Smartptr))), - Callback); + Finder->addMatcher( + memberExpr(expr().bind("memberExpr"), isArrow(), + hasObjectExpression(callToGet(SmartptrWithDeref))), + Callback); // Catch '*ptr.get()' or '*ptr->get()' Finder->addMatcher( - unaryOperator(hasOperatorName("*"), hasUnaryOperand(callToGet(Smartptr))), + unaryOperator(hasOperatorName("*"), + hasUnaryOperand(callToGet(SmartptrWithDeref))), Callback); // Catch '!ptr.get()' const auto CallToGetAsBool = callToGet( - recordDecl(Smartptr, has(cxxConversionDecl(returns(booleanType()))))); + recordDecl(SmartptrAny, has(cxxConversionDecl(returns(booleanType()))))); Finder->addMatcher( unaryOperator(hasOperatorName("!"), hasUnaryOperand(CallToGetAsBool)), Callback); @@ -84,7 +96,7 @@ void registerMatchersForGetArrowStart(MatchFinder *Finder, Callback); Finder->addMatcher(cxxDependentScopeMemberExpr(hasObjectExpression( - callExpr(has(callToGet(Smartptr))).bind("obj"))), + callExpr(has(callToGet(SmartptrAny))))), Callback); } diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-smartptr-get-msvc.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-smartptr-get-msvc.cpp index bd8990a27b263..b360ccbadfa5f 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-smartptr-get-msvc.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-smartptr-get-msvc.cpp @@ -16,6 +16,14 @@ struct unique_ptr { explicit operator bool() const noexcept; }; +template +struct unique_ptr { + template + T2* operator[](unsigned) const; + T* get() const; + explicit operator bool() const noexcept; +}; + template struct shared_ptr { template @@ -26,6 +34,14 @@ struct shared_ptr { explicit operator bool() const noexcept; }; +template +struct shared_ptr { + template + T2* operator[](unsigned) const; + T* get() const; + explicit operator bool() const noexcept; +}; + } // namespace std struct Bar { @@ -92,3 +108,31 @@ void Positive() { // CHECK-MESSAGES: if (NULL == x.get()); // CHECK-FIXES: if (NULL == x); } + +void test_smart_ptr_to_array() { + std::unique_ptr i; + // The array specialization does not have operator*(), so make sure + // we do not incorrectly suggest sizeof(*i) here. + // FIXME: alternatively, we could suggest sizeof(i[0]) + auto sz = sizeof(*i.get()); + + std::shared_ptr s; + // The array specialization does not have operator->() either + s.get()->Do(); + + bool b1 = !s.get(); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: redundant get() call + // CHECK-FIXES: bool b1 = !s; + + if (s.get()) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant get() call + // CHECK-FIXES: if (s) {} + + int x = s.get() ? 1 : 2; + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: redundant get() call + // CHECK-FIXES: int x = s ? 1 : 2; + + bool b2 = s.get() == nullptr; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: redundant get() call + // CHECK-FIXES: bool b2 = s == nullptr; +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-smartptr-get.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-smartptr-get.cpp index ec4ca4cb79484..42e948fcb2a7f 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-smartptr-get.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-smartptr-get.cpp @@ -12,6 +12,13 @@ struct unique_ptr { explicit operator bool() const noexcept; }; +template +struct unique_ptr { + T& operator[](unsigned) const; + T* get() const; + explicit operator bool() const noexcept; +}; + template struct shared_ptr { T& operator*() const; @@ -20,6 +27,13 @@ struct shared_ptr { explicit operator bool() const noexcept; }; +template +struct shared_ptr { + T& operator[](unsigned) const; + T* get() const; + explicit operator bool() const noexcept; +}; + template struct vector { vector(); @@ -283,3 +297,31 @@ void test_redundant_get_with_member() { // CHECK-FIXES: f(**i->get()->getValue()); } } + +void test_smart_ptr_to_array() { + std::unique_ptr i; + // The array specialization does not have operator*(), so make sure + // we do not incorrectly suggest sizeof(*i) here. + // FIXME: alternatively, we could suggest sizeof(i[0]) + auto sz = sizeof(*i.get()); + + std::shared_ptr s; + // The array specialization does not have operator->() either + s.get()->getValue(); + + bool b1 = !s.get(); + // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: redundant get() call + // CHECK-FIXES: bool b1 = !s; + + if (s.get()) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant get() call + // CHECK-FIXES: if (s) {} + + int x = s.get() ? 1 : 2; + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: redundant get() call + // CHECK-FIXES: int x = s ? 1 : 2; + + bool b2 = s.get() == nullptr; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: redundant get() call + // CHECK-FIXES: bool b2 = s == nullptr; +}