[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.

Author: FabianWolff

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);
 }
 

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;
+}

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;
+}