https://github.com/FabianWolff updated https://github.com/llvm/llvm-project/pull/141092
>From 3ed915b5a5e97e327b132a612a817ebcff42a6a5 Mon Sep 17 00:00:00 2001 From: Fabian Wolff <fwo...@google.com> Date: Mon, 26 May 2025 08:45:21 +0000 Subject: [PATCH] [clang-tidy] readability-redundant-smartptr-get: disable for smart pointers to arrays --- .../readability/RedundantSmartptrGetCheck.cpp | 30 +++++++++---- .../redundant-smartptr-get-msvc.cpp | 16 +++++++ .../readability/redundant-smartptr-get.cpp | 42 +++++++++++++++++++ 3 files changed, 79 insertions(+), 9 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp index be52af77ae0a5..6d119863c5336 100644 --- a/clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp @@ -43,10 +43,18 @@ internal::Matcher<Expr> callToGet(const internal::Matcher<Decl> &OnClass) { .bind("redundant_get"); } -internal::Matcher<Decl> knownSmartptr() { +internal::Matcher<Decl> knownSmartptrAny() { return recordDecl(hasAnyName("::std::unique_ptr", "::std::shared_ptr")); } +internal::Matcher<Decl> knownSmartptrNonArray() { + return recordDecl(hasAnyName("::std::unique_ptr", "::std::shared_ptr"), + anyOf(has(cxxMethodDecl(hasName("operator*"))), + has(functionTemplateDecl(hasName("operator*")))), + anyOf(has(cxxMethodDecl(hasName("operator->"))), + has(functionTemplateDecl(hasName("operator->"))))); +} + void registerMatchersForGetArrowStart(MatchFinder *Finder, MatchFinder::MatchCallback *Callback) { const auto QuacksLikeASmartptr = recordDecl( @@ -57,21 +65,25 @@ void registerMatchersForGetArrowStart(MatchFinder *Finder, type().bind("op*Type"))))))); // Make sure we are not missing the known standard types. - const auto Smartptr = anyOf(knownSmartptr(), QuacksLikeASmartptr); + const auto SmartptrAny = anyOf(knownSmartptrAny(), QuacksLikeASmartptr); + const auto SmartptrNonArray = + anyOf(knownSmartptrNonArray(), 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(SmartptrNonArray))), + Callback); // Catch '*ptr.get()' or '*ptr->get()' Finder->addMatcher( - unaryOperator(hasOperatorName("*"), hasUnaryOperand(callToGet(Smartptr))), + unaryOperator(hasOperatorName("*"), + hasUnaryOperand(callToGet(SmartptrNonArray))), 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); } @@ -100,7 +112,7 @@ void registerMatchersForGetEquals(MatchFinder *Finder, binaryOperator(hasAnyOperatorName("==", "!="), hasOperands(anyOf(cxxNullPtrLiteralExpr(), gnuNullExpr(), integerLiteral(equals(0))), - callToGet(knownSmartptr()))), + callToGet(knownSmartptrAny()))), Callback); // FIXME: Match and fix if (l.get() == r.get()). 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..3b8e474c6fe8f 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 @@ -26,6 +26,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 +100,11 @@ void Positive() { // CHECK-MESSAGES: if (NULL == x.get()); // CHECK-FIXES: if (NULL == x); } + +void test_shared_ptr_to_array() { + std::shared_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 s = sizeof(*i.get()); +} 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..613fd4e4d57b1 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_unique_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