[clang-tools-extra] 47d5e94 - [clang-tidy] readability-redundant-smartptr-get: disable for smart pointers to arrays (#141092)
via cfe-commits
cfe-commits at lists.llvm.org
Tue May 27 03:06:11 PDT 2025
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;
+}
More information about the cfe-commits
mailing list