[clang-tools-extra] a5e3d87 - [clang-tidy] Handle uninstantiated templates in redundant get check

Stephen Kelly via cfe-commits cfe-commits at lists.llvm.org
Sat Feb 27 04:09:42 PST 2021


Author: Stephen Kelly
Date: 2021-02-27T12:08:41Z
New Revision: a5e3d87f66a1b833594f121fbd8c7334df5a1eeb

URL: https://github.com/llvm/llvm-project/commit/a5e3d87f66a1b833594f121fbd8c7334df5a1eeb
DIFF: https://github.com/llvm/llvm-project/commit/a5e3d87f66a1b833594f121fbd8c7334df5a1eeb.diff

LOG: [clang-tidy] Handle uninstantiated templates in redundant get check

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

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp
    clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.h
    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 bf78acdc5f68..e663f055f30d 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp
@@ -18,15 +18,30 @@ 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"),
-                 returns(qualType(pointsTo(type().bind("getType")))))))
+  return expr(
+             anyOf(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"),
+                                            returns(qualType(pointsTo(
+                                                type().bind("getType"))))))),
+                   cxxDependentScopeMemberExpr(
+                       hasMemberName("get"),
+                       hasObjectExpression(
+                           expr(hasType(qualType(hasCanonicalType(
+                                    templateSpecializationType(hasDeclaration(
+                                        classTemplateDecl(has(cxxRecordDecl(
+                                            OnClass,
+                                            hasMethod(cxxMethodDecl(
+                                                hasName("get"),
+                                                returns(qualType(
+                                                    pointsTo(type().bind(
+                                                        "getType")))))))))))))))
+                               .bind("smart_pointer")))))
       .bind("redundant_get");
 }
 
@@ -47,10 +62,9 @@ void registerMatchersForGetArrowStart(MatchFinder *Finder,
   const auto Smartptr = anyOf(knownSmartptr(), QuacksLikeASmartptr);
 
   // Catch 'ptr.get()->Foo()'
-  Finder->addMatcher(
-      memberExpr(expr().bind("memberExpr"), isArrow(),
-                 hasObjectExpression(ignoringImpCasts(callToGet(Smartptr)))),
-      Callback);
+  Finder->addMatcher(memberExpr(expr().bind("memberExpr"), isArrow(),
+                                hasObjectExpression(callToGet(Smartptr))),
+                     Callback);
 
   // Catch '*ptr.get()' or '*ptr->get()'
   Finder->addMatcher(
@@ -58,8 +72,8 @@ void registerMatchersForGetArrowStart(MatchFinder *Finder,
       Callback);
 
   // Catch '!ptr.get()'
-  const auto CallToGetAsBool = ignoringParenImpCasts(callToGet(
-      recordDecl(Smartptr, has(cxxConversionDecl(returns(booleanType()))))));
+  const auto CallToGetAsBool = callToGet(
+      recordDecl(Smartptr, has(cxxConversionDecl(returns(booleanType())))));
   Finder->addMatcher(
       unaryOperator(hasOperatorName("!"), hasUnaryOperand(CallToGetAsBool)),
       Callback);
@@ -70,6 +84,10 @@ void registerMatchersForGetArrowStart(MatchFinder *Finder,
   // Catch 'ptr.get() ? X : Y'
   Finder->addMatcher(conditionalOperator(hasCondition(CallToGetAsBool)),
                      Callback);
+
+  Finder->addMatcher(cxxDependentScopeMemberExpr(hasObjectExpression(
+                         callExpr(has(callToGet(Smartptr))).bind("obj"))),
+                     Callback);
 }
 
 void registerMatchersForGetEquals(MatchFinder *Finder,
@@ -82,9 +100,8 @@ void registerMatchersForGetEquals(MatchFinder *Finder,
   // Matches against nullptr.
   Finder->addMatcher(
       binaryOperator(hasAnyOperatorName("==", "!="),
-                     hasOperands(ignoringImpCasts(anyOf(
-                                     cxxNullPtrLiteralExpr(), gnuNullExpr(),
-                                     integerLiteral(equals(0)))),
+                     hasOperands(anyOf(cxxNullPtrLiteralExpr(), gnuNullExpr(),
+                                       integerLiteral(equals(0))),
                                  callToGet(knownSmartptr()))),
       Callback);
 
@@ -138,13 +155,21 @@ void RedundantSmartptrGetCheck::check(const MatchFinder::MatchResult &Result) {
     return;
   }
 
+  auto SR = GetCall->getSourceRange();
+  // CXXDependentScopeMemberExpr source range does not include parens
+  // Extend the source range of the get call to account for them.
+  if (isa<CXXDependentScopeMemberExpr>(GetCall))
+    SR.setEnd(Lexer::getLocForEndOfToken(SR.getEnd(), 0, *Result.SourceManager,
+                                         getLangOpts())
+                  .getLocWithOffset(1));
+
   StringRef SmartptrText = Lexer::getSourceText(
       CharSourceRange::getTokenRange(Smartptr->getSourceRange()),
       *Result.SourceManager, getLangOpts());
   // Replace foo->get() with *foo, and foo.get() with foo.
   std::string Replacement = Twine(IsPtrToPtr ? "*" : "", SmartptrText).str();
   diag(GetCall->getBeginLoc(), "redundant get() call on smart pointer")
-      << FixItHint::CreateReplacement(GetCall->getSourceRange(), Replacement);
+      << FixItHint::CreateReplacement(SR, Replacement);
 }
 
 } // namespace readability

diff  --git a/clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.h b/clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.h
index 3214863602ca..37c09b871d44 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.h
@@ -35,6 +35,9 @@ class RedundantSmartptrGetCheck : public ClangTidyCheck {
   void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  llvm::Optional<TraversalKind> getCheckTraversalKind() const override {
+    return TK_IgnoreUnlessSpelledInSource;
+  }
 
 private:
   const bool IgnoreMacros;

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 51d98796265a..01f12b6bfe6e 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
@@ -168,6 +168,42 @@ void Positive() {
   // CHECK-FIXES: if (NULL == x);
 }
 
+template <typename T>
+void testTemplate() {
+  T().get()->Do();
+}
+
+template <typename T>
+void testTemplate2() {
+  std::unique_ptr<T> up;
+  up.get()->Do();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: redundant get() call
+  // CHECK-FIXES: up->Do();
+}
+
+void instantiate() {
+  testTemplate<BarPtr>();
+  testTemplate<std::unique_ptr<Bar>>();
+  testTemplate<Fail2>();
+
+  testTemplate2<Bar>();
+}
+
+struct S {
+
+  void foo() {
+    m_up.get()->Do();
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant get() call
+    // CHECK-FIXES: m_up->Do();
+    m_bp.get()->Do();
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant get() call
+    // CHECK-FIXES: m_bp->Do();
+  }
+
+  std::unique_ptr<Bar> m_up;
+  BarPtr m_bp;
+};
+
 #define MACRO(p) p.get()
 
 void Negative() {


        


More information about the cfe-commits mailing list