[clang-tidy] readability-redundant-smartptr-get: disable for smart pointers to arrays

Author: FabianWolff

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()).

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

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