[clang-tidy] Expand readability-redundant-smartptr-get to understand implicit converions to bool in more contexts.

[sbenza]

Summary

Expand readability-redundant-smartptr-get to understand implicit converions to bool in more contexts.

[aaron.ballman]

Aside from a minor formatting nit, LGTM!

clang-tidy/readability/RedundantSmartptrGetCheck.cpp

21

Spurious formatting change?

[sbenza]

minor fix

clang-tidy/readability/RedundantSmartptrGetCheck.cpp

 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace readability {
 
 namespace {
 internal::Matcher<Expr> callToGet(const internal::Matcher<Decl> &OnClass) {
   return cxxMemberCallExpr(
              on(expr(anyOf(hasType(OnClass),
                            hasType(qualType(
                                pointsTo(decl(OnClass).bind("ptr_to_ptr"))))))
                     .bind("smart_pointer")),
              unless(callee(memberExpr(hasObjectExpression(cxxThisExpr())))),
              callee(cxxMethodDecl(
                  hasName("get"),
Finder->addMatcher(memberExpr(expr().bind("memberExpr"), isArrow(),
                                     callToGet(QuacksLikeASmartptr)))),
                      Callback);
 
   // Catch '*ptr.get()' or '*ptr->get()'
   Finder->addMatcher(
       unaryOperator(hasOperatorName("*"),
                     hasUnaryOperand(callToGet(QuacksLikeASmartptr))),
       Callback);
+
+  // Catch '!ptr.get()'
+  const auto CallToGetAsBool = ignoringParenImpCasts(callToGet(recordDecl(
+      QuacksLikeASmartptr, has(cxxConversionDecl(returns(booleanType()))))));
+  Finder->addMatcher(
+      unaryOperator(hasOperatorName("!"), hasUnaryOperand(CallToGetAsBool)),
+      Callback);
+
+  // Catch 'if(ptr.get())'
+  Finder->addMatcher(ifStmt(hasCondition(CallToGetAsBool)), Callback);
+
+  // Catch 'ptr.get() ? X : Y'
+  Finder->addMatcher(conditionalOperator(hasCondition(CallToGetAsBool)),
+                     Callback);
 }
 
 void registerMatchersForGetEquals(MatchFinder *Finder,
                                   MatchFinder::MatchCallback *Callback) {
   // 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"));
 
   // Matches against nullptr.
   Finder->addMatcher(
       binaryOperator(anyOf(hasOperatorName("=="), hasOperatorName("!=")),
                      hasEitherOperand(ignoringImpCasts(
                          anyOf(cxxNullPtrLiteralExpr(), gnuNullExpr(),
                                integerLiteral(equals(0))))),
                      hasEitherOperand(callToGet(IsAKnownSmartptr))),
       Callback);
 
-  // Matches against if(ptr.get())
-  Finder->addMatcher(
-      ifStmt(hasCondition(ignoringImpCasts(callToGet(IsAKnownSmartptr)))),
-      Callback);
-
   // FIXME: Match and fix if (l.get() == r.get()).
 }
 
 } // namespace
 
 void RedundantSmartptrGetCheck::registerMatchers(MatchFinder *Finder) {
   // Only register the matchers for C++; the functionality currently does not
   // provide any benefit to other languages, despite being benign.

test/clang-tidy/readability-redundant-smartptr-get.cpp

 // RUN: %check_clang_tidy %s readability-redundant-smartptr-get %t
 
 #define NULL __null
 
 namespace std {
 
 template <typename T>
 struct unique_ptr {
   T& operator*() const;
   T* operator->() const;
   T* get() const;
+  explicit operator bool() const noexcept;
 };
 
 template <typename T>
 struct shared_ptr {
   T& operator*() const;
   T* operator->() const;
   T* get() const;
+  explicit operator bool() const noexcept;
 };
 
 }  // namespace std
 
 struct Bar {
   void Do();
   void ConstDo() const;
 };
 struct BarPtr {
   Bar* operator->();
   Bar& operator*();
   Bar* get();
+  explicit operator bool() const;
 };
 struct int_ptr {
   int* get();
   int* operator->();
   int& operator*();
 };
 
 struct Fail1 {
void Positive() {
 
   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 = u.get() ? true : false;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: redundant get() call
+  // CHECK-MESSAGES: bb = u.get() ? true : false;
+  // CHECK-FIXES: bb = u ? true : false;
+
   bb = nullptr != ss->get();
   // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: redundant get() call
   // CHECK-MESSAGES: nullptr != ss->get();
   // CHECK-FIXES: bb = nullptr != *ss;
 
   i = *PointerWithOverloadedGet().get();
   // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: redundant get() call
   // CHECK-MESSAGES: i = *PointerWithOverloadedGet().get();
void Positive() {
   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);
-  if (x.get());
-  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant get() call
-  // CHECK-MESSAGES: if (x.get());
-  // CHECK-FIXES: if (x);
 }
 
 void Negative() {
   struct NegPtr {
     int* get();
     int* operator->() {
       return &*this->get();
     }
void Negative() {
 
   Fail1().get()->Do();
   Fail2().get()->Do();
   const Bar& b = *Fail1().get();
   (*Fail2().get()).Do();
 
   int_ptr ip;
   bool bb = ip.get() == nullptr;
+  bb = !ip.get();
+  bb = ip.get() ? true : false;
 }