[clang] [clang-tools-extra] [RecursiveASTVisitor] Skip implicit instantiations. (PR #110899)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-tools-extra

Author: Harald van Dijk (hvdijk)

<details>
<summary>Changes</summary>

In DEF_TRAVERSE_TMPL_SPEC_DECL, we attempted to skip implicit instantiations by detecting that D->getTemplateArgsAsWritten() returns nullptr, but as this test shows, it is possible for that to return a non-null pointer even for implicit instantiations. Explicitly check for this case instead.

Fixes #<!-- -->110502

cc @<!-- -->sdkrystian, this fixes an issue introduced by 12028373020739b388eb2b8141742509f1764e3c, does this look okay or do you feel there should be another way of doing it?

---
Full diff: https://github.com/llvm/llvm-project/pull/110899.diff


2 Files Affected:

- (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/type-traits.cpp (+21) 
- (modified) clang/include/clang/AST/RecursiveASTVisitor.h (+9-7) 


``````````diff
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/type-traits.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/type-traits.cpp
index 72241846384bcf..854ffa7928635c 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/type-traits.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/type-traits.cpp
@@ -14,11 +14,22 @@ namespace std {
     static constexpr bool value = true;
   };
 
+  template <typename T, typename U>
+  static constexpr bool is_same_v = is_same<T, U>::value;  // NOLINT
+
   template<bool, typename T = void>
   struct enable_if {
     using type = T;
   };
 
+  template <bool B, typename T = void>
+  using enable_if_t = typename enable_if<B, T>::type;  // NOLINT
+
+  template <typename T>
+  struct remove_reference {
+    using type = T;
+  };
+
 inline namespace __std_lib_version1 {
   template<typename T>
   struct add_const {
@@ -117,3 +128,13 @@ namespace my_std = std;
 using Alias = my_std::add_const<bool>::type;
 // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: use c++14 style type templates
 // CHECK-FIXES: using Alias = my_std::add_const_t<bool>;
+
+template <typename T>
+struct ImplicitlyInstantiatedConstructor {
+  template <typename U, typename = std::enable_if_t<std::is_same_v<U, T>>>
+  ImplicitlyInstantiatedConstructor(U) {}
+};
+
+const ImplicitlyInstantiatedConstructor<int> ImplicitInstantiation(std::remove_reference<int>::type(123));
+// CHECK-MESSAGES: :[[@LINE-1]]:68: warning: use c++14 style type templates
+// CHECK-FIXES: const ImplicitlyInstantiatedConstructor<int> ImplicitInstantiation(std::remove_reference_t<int>(123));
diff --git a/clang/include/clang/AST/RecursiveASTVisitor.h b/clang/include/clang/AST/RecursiveASTVisitor.h
index cd9947f7ab9805..b563b89875f08b 100644
--- a/clang/include/clang/AST/RecursiveASTVisitor.h
+++ b/clang/include/clang/AST/RecursiveASTVisitor.h
@@ -2069,22 +2069,24 @@ bool RecursiveASTVisitor<Derived>::TraverseTemplateArgumentLocsHelper(
 
 #define DEF_TRAVERSE_TMPL_SPEC_DECL(TMPLDECLKIND, DECLKIND)                    \
   DEF_TRAVERSE_DECL(TMPLDECLKIND##TemplateSpecializationDecl, {                \
+    auto TSK = D->getTemplateSpecializationKind();                             \
     /* For implicit instantiations ("set<int> x;"), we don't want to           \
        recurse at all, since the instatiated template isn't written in         \
        the source code anywhere.  (Note the instatiated *type* --              \
        set<int> -- is written, and will still get a callback of                \
        TemplateSpecializationType).  For explicit instantiations               \
        ("template set<int>;"), we do need a callback, since this               \
-       is the only callback that's made for this instantiation.                \
-       We use getTemplateArgsAsWritten() to distinguish. */                    \
-    if (const auto *ArgsWritten = D->getTemplateArgsAsWritten()) {             \
-      /* The args that remains unspecialized. */                               \
-      TRY_TO(TraverseTemplateArgumentLocsHelper(                               \
-          ArgsWritten->getTemplateArgs(), ArgsWritten->NumTemplateArgs));      \
+       is the only callback that's made for this instantiation. */             \
+    if (TSK != TSK_ImplicitInstantiation) {                                    \
+      if (const auto *ArgsWritten = D->getTemplateArgsAsWritten()) {           \
+        /* The args that remains unspecialized. */                             \
+        TRY_TO(TraverseTemplateArgumentLocsHelper(                             \
+            ArgsWritten->getTemplateArgs(), ArgsWritten->NumTemplateArgs));    \
+      }                                                                        \
     }                                                                          \
                                                                                \
     if (getDerived().shouldVisitTemplateInstantiations() ||                    \
-        D->getTemplateSpecializationKind() == TSK_ExplicitSpecialization) {    \
+        TSK == TSK_ExplicitSpecialization) {                                   \
       /* Traverse base definition for explicit specializations */              \
       TRY_TO(Traverse##DECLKIND##Helper(D));                                   \
     } else {                                                                   \

``````````

</details>


https://github.com/llvm/llvm-project/pull/110899