Author: FabianWolff Date: 2025-05-27T12:06:08+02:00 New Revision: 47d5e94acb92ce4ec5040e95e167ba471d080244
URL: https://github.com/llvm/llvm-project/commit/47d5e94acb92ce4ec5040e95e167ba471d080244 DIFF: https://github.com/llvm/llvm-project/commit/47d5e94acb92ce4ec5040e95e167ba471d080244.diff LOG: [clang-tidy] readability-redundant-smartptr-get: disable for smart pointers to arrays (#141092) Currently we generate an incorrect suggestion for shared/unique pointers to arrays; for instance ([Godbolt](https://godbolt.org/z/Tens1reGP)): ```c++ #include <memory> void test_shared_ptr_to_array() { std::shared_ptr<int[]> i; auto s = sizeof(*i.get()); } ``` ``` <source>:5:20: warning: redundant get() call on smart pointer [readability-redundant-smartptr-get] 5 | auto s = sizeof(*i.get()); | ^~~~~~~ | i 1 warning generated. ``` `sizeof(*i)` is incorrect, though, because the array specialization of `std::shared/unique_ptr` does not have an `operator*()`. Therefore I have disabled this check for smart pointers to arrays for now; future work could, of course, improve on this by suggesting, say, `sizeof(i[0])` in the above example. Added: Modified: clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp clang-tools-extra/test/clang-tidy/checkers/readability/redundant-smartptr-get-msvc.cpp clang-tools-extra/test/clang-tidy/checkers/readability/redundant-smartptr-get.cpp Removed: ################################################################################ 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<Decl> 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 <typename T> +struct unique_ptr<T[]> { + template <typename T2 = T> + T2* operator[](unsigned) const; + T* get() const; + explicit operator bool() const noexcept; +}; + template <typename T> struct shared_ptr { template <typename T2 = T> @@ -26,6 +34,14 @@ struct shared_ptr { explicit operator bool() const noexcept; }; +template <typename T> +struct shared_ptr<T[]> { + template <typename T2 = T> + 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<int[]> 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<Bar[]> 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 <typename T> +struct unique_ptr<T[]> { + T& operator[](unsigned) const; + T* get() const; + explicit operator bool() const noexcept; +}; + template <typename T> struct shared_ptr { T& operator*() const; @@ -20,6 +27,13 @@ struct shared_ptr { explicit operator bool() const noexcept; }; +template <typename T> +struct shared_ptr<T[]> { + T& operator[](unsigned) const; + T* get() const; + explicit operator bool() const noexcept; +}; + template <typename T> 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<int[]> 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<Inner[]> 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; +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
