Fixed: Duck-typing in readability-redundant-smartptr-get didn't catch MSVC STL smart pointers.

Differential Revision: https://reviews.llvm.org/D61209

llvm-svn: 359801

Author: Florian Gross

clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp

@@ -30,6 +30,10 @@ internal::Matcher<Expr> callToGet(const internal::Matcher<Decl> &OnClass) {
       .bind("redundant_get");
 }
 
+internal::Matcher<Decl> knownSmartptr() {
+  return recordDecl(hasAnyName("::std::unique_ptr", "::std::shared_ptr"));
+}
+
 void registerMatchersForGetArrowStart(MatchFinder *Finder,
                                       MatchFinder::MatchCallback *Callback) {
   const auto QuacksLikeASmartptr = recordDecl(
@@ -39,21 +43,23 @@ void registerMatchersForGetArrowStart(MatchFinder *Finder,
       has(cxxMethodDecl(hasName("operator*"), returns(qualType(references(
                                                   type().bind("op*Type")))))));
 
+  // Make sure we are not missing the known standard types.
+  const auto Smartptr = anyOf(knownSmartptr(), QuacksLikeASmartptr);
+
   // Catch 'ptr.get()->Foo()'
-  Finder->addMatcher(memberExpr(expr().bind("memberExpr"), isArrow(),
-                                hasObjectExpression(ignoringImpCasts(
-                                    callToGet(QuacksLikeASmartptr)))),
-                     Callback);
+  Finder->addMatcher(
+      memberExpr(expr().bind("memberExpr"), isArrow(),
+                 hasObjectExpression(ignoringImpCasts(callToGet(Smartptr)))),
+      Callback);
 
   // Catch '*ptr.get()' or '*ptr->get()'
   Finder->addMatcher(
-      unaryOperator(hasOperatorName("*"),
-                    hasUnaryOperand(callToGet(QuacksLikeASmartptr))),
+      unaryOperator(hasOperatorName("*"), hasUnaryOperand(callToGet(Smartptr))),
       Callback);
 
   // Catch '!ptr.get()'
-  const auto CallToGetAsBool = ignoringParenImpCasts(callToGet(recordDecl(
-      QuacksLikeASmartptr, has(cxxConversionDecl(returns(booleanType()))))));
+  const auto CallToGetAsBool = ignoringParenImpCasts(callToGet(
+      recordDecl(Smartptr, has(cxxConversionDecl(returns(booleanType()))))));
   Finder->addMatcher(
       unaryOperator(hasOperatorName("!"), hasUnaryOperand(CallToGetAsBool)),
       Callback);
@@ -71,18 +77,15 @@ void registerMatchersForGetEquals(MatchFinder *Finder,
   // This one is harder to do with duck typing.
   // The operator==/!= that we are looking for might be member or non-member,
   // might be on global namespace or found by ADL, might be a template, etc.
-  // For now, lets keep a list of known standard types.
-
-  const auto IsAKnownSmartptr =
-      recordDecl(hasAnyName("::std::unique_ptr", "::std::shared_ptr"));
+  // For now, lets keep it to the known standard types.
 
   // Matches against nullptr.
   Finder->addMatcher(
       binaryOperator(anyOf(hasOperatorName("=="), hasOperatorName("!=")),
                      hasEitherOperand(ignoringImpCasts(
                          anyOf(cxxNullPtrLiteralExpr(), gnuNullExpr(),
                                integerLiteral(equals(0))))),
-                     hasEitherOperand(callToGet(IsAKnownSmartptr))),
+                     hasEitherOperand(callToGet(knownSmartptr()))),
       Callback);
 
   // FIXME: Match and fix if (l.get() == r.get()).

clang-tools-extra/test/clang-tidy/readability-redundant-smartptr-get-msvc.cpp

@@ -0,0 +1,94 @@
+// RUN: %check_clang_tidy %s readability-redundant-smartptr-get %t
+
+#define NULL __null
+
+namespace std {
+
+// MSVC headers define operator templates instead of plain operators.
+
+template <typename T>
+struct unique_ptr {
+  template <typename T2 = T>
+  T2& operator*() const;
+  template <typename T2 = T>
+  T2* operator->() const;
+  T* get() const;
+  explicit operator bool() const noexcept;
+};
+
+template <typename T>
+struct shared_ptr {
+  template <typename T2 = T>
+  T2& operator*() const;
+  template <typename T2 = T>
+  T2* operator->() const;
+  T* get() const;
+  explicit operator bool() const noexcept;
+};
+
+}  // namespace std
+
+struct Bar {
+  void Do();
+  void ConstDo() const;
+};
+
+void Positive() {
+  std::unique_ptr<Bar>* up;
+  (*up->get()).Do();
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant get() call
+  // CHECK-MESSAGES: (*up->get()).Do();
+  // CHECK-FIXES: (**up).Do();
+
+  std::unique_ptr<int> uu;
+  std::shared_ptr<double> *ss;
+  bool bb = uu.get() == nullptr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: redundant get() call
+  // CHECK-MESSAGES: uu.get() == nullptr;
+  // CHECK-FIXES: bool bb = uu == nullptr;
+
+  if (up->get());
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant get() call
+  // CHECK-MESSAGES: if (up->get());
+  // CHECK-FIXES: if (*up);
+  if ((uu.get()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: redundant get() call
+  // CHECK-MESSAGES: if ((uu.get()));
+  // CHECK-FIXES: if ((uu));
+  bb = !ss->get();
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant get() call
+  // CHECK-MESSAGES: bb = !ss->get();
+  // CHECK-FIXES: bb = !*ss;
+
+  bb = nullptr != ss->get();
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: redundant get() call
+  // CHECK-MESSAGES: nullptr != ss->get();
+  // CHECK-FIXES: bb = nullptr != *ss;
+
+  bb = std::unique_ptr<int>().get() == NULL;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: redundant get() call
+  // CHECK-MESSAGES: bb = std::unique_ptr<int>().get() == NULL;
+  // CHECK-FIXES: bb = std::unique_ptr<int>() == NULL;
+  bb = ss->get() == NULL;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: redundant get() call
+  // CHECK-MESSAGES: bb = ss->get() == NULL;
+  // CHECK-FIXES: bb = *ss == NULL;
+
+  std::unique_ptr<int> x, y;
+  if (x.get() == nullptr);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant get() call
+  // CHECK-MESSAGES: if (x.get() == nullptr);
+  // CHECK-FIXES: if (x == nullptr);
+  if (nullptr == y.get());
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: redundant get() call
+  // CHECK-MESSAGES: if (nullptr == y.get());
+  // CHECK-FIXES: if (nullptr == y);
+  if (x.get() == NULL);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant get() call
+  // CHECK-MESSAGES: if (x.get() == NULL);
+  // CHECK-FIXES: if (x == NULL);
+  if (NULL == x.get());
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: redundant get() call
+  // CHECK-MESSAGES: if (NULL == x.get());
+  // CHECK-FIXES: if (NULL == x);
+}